Re: [Xen-devel] [PATCH] libxl: don't accept negative disk or partition indexes

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-03-07 at 06:01 -0500, Jan Beulich wrote:
> When obtained via sscanf(), they were checked against an upper bound
> only so far. By converting the local variables' types to "unsigned int"
> those bounds checks become sufficient (as a consequence the helper
> function's parameter types need to be adjusted too). It's not strictly
> necessary to also convert libxl__device_disk_dev_number()'s parameter
> types - 

Any reason not to do it though?

> the bounds checking done (now) guarantees that the values won't
> run into the negative range of "int" values.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> 
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -260,8 +260,10 @@ int libxl__device_physdisk_major_minor(c
>  }
>  
>  static int device_virtdisk_matches(const char *virtpath, const char *devtype,
> -                                   int *index_r, int max_index,
> -                                   int *partition_r, int max_partition) {
> +                                   unsigned int *index_r,
> +                                   unsigned int max_index,
> +                                   unsigned int *partition_r,
> +                                   unsigned int max_partition) {
>      const char *p;
>      char *ep;
>      int tl, c;
> @@ -310,13 +312,13 @@ static int device_virtdisk_matches(const
>  int libxl__device_disk_dev_number(const char *virtpath, int *pdisk,
>                                    int *ppartition)
>  {
> -    int disk, partition;
> +    unsigned int disk, partition;
>      char *ep;
>      unsigned long ul;
>      int chrused;
>  
>      chrused = -1;
> -    if ((sscanf(virtpath, "d%ip%i%n", &disk, &partition, &chrused)  >= 2
> +    if ((sscanf(virtpath, "d%up%u%n", &disk, &partition, &chrused) >= 2
>           && chrused == strlen(virtpath) && disk < (1<<20) && partition < 256)
>          ||
>          device_virtdisk_matches(virtpath, "xvd",
> 
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel