On 14/01/2021 15:23, Jan Beulich wrote:
> Forever since the fix for XSA-230 the 2nd of the comments ahead of
> fixup_status_for_copy_pin() has been stale - there's nothing specific to
> transitive grants there anymore.
>
> Move the function up, drop the "copy" part from its name again, add a
> "readonly" parameter, and use it also on other paths having decremented
> one (or not having got to increment any) of the pin counts.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -908,6 +908,25 @@ static int _set_status(const grant_entry
>          return _set_status_v2(shah, status, rd, act, readonly, mapflag, 
> ldomid);
>  }
>  
> +/*
> + * The status for a grant may indicate that we're taking more access than
> + * the pin requires.  Fix up the status to match the pin.

This sentence isn't correct.  It will only clear status flags to match a
reduction in the pinned references.  IOW it cannot be used in the case
that a grant goes from unpinned to pinned.

How about renaming to clear_status_for_pin() to make this clearer?  I
don't think it is worth trying to make the operation more generic.

If so (and with a suitable adjustment to the comment), Reviewed-by:
Andrew Cooper <andrew.coop...@citrix.com>

Reply via email to