Re: [PATCH] sysctl: Don't overwrite array size variable when it is set on error earlier

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 25/03/15 17:09, Boris Ostrovsky wrote:
> When querying CPU topology, if caller-provided array size is smaller than
> number of online CPUs then, in addition to returning -ENOBUFS, sysctl is
> expected to provide back this number. However, this value, stored in 'i',
> is overwritten in the subsequent loop's control statement.
>
> Make sure we don't do this by converting the loop to 'while'.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>   xen/common/sysctl.c |    6 +++++-
>   1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index a8c629f..b83d230 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -338,8 +338,10 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>                   ret = -ENOBUFS;
>                   i = num_cpus;
>               }
> +            else
> +                i = 0;
>   
> -            for ( i = 0; i < num_cpus; i++ )
> +            while ( i < num_cpus )

This would be fine to keep as "for ( ; i < num_cpus; i++)", and helps 
avoid an issue if someone introduces a continue; in the future.

As for the fix itself, Reviewed-by: Andrew Cooper 
<andrew.cooper3@citrix.com>

>               {
>                   xen_sysctl_cputopo_t cputopo;
>   
> @@ -363,6 +365,8 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>                       ret = -EFAULT;
>                       break;
>                   }
> +
> +                i++;
>               }
>           }
>           else