On 03.10.2023 17:24, Federico Serafini wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5901,17 +5901,17 @@ int destroy_xen_mappings(unsigned long s, unsigned 
> long e)
>   * a problem.
>   */
>  void init_or_livepatch modify_xen_mappings_lite(
> -    unsigned long s, unsigned long e, unsigned int _nf)
> +    unsigned long s, unsigned long e, unsigned int nf)
>  {
> -    unsigned long v = s, fm, nf;
> +    unsigned long v = s, fm, flags;

While it looks correct, I consider this an unacceptably dangerous
change: What if by the time this is to be committed some new use of
the local "nf" appears, without resulting in fuzz while applying the
patch? Imo this needs doing in two steps: First nf -> flags, then
_nf -> nf.

Furthermore since you alter the local variable, is there any reason
not to also change it to be "unsigned int", matching the function
argument it's set from?

Yet then - can't we just delete "nf" and rename "_nf" to "nf"? The
function parameter is only used in

    nf = put_pte_flags(_nf & FLAGS_MASK);

Jan

Reply via email to