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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.03.12 at 18:23, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> Jan Beulich writes ("[Xen-devel] [PATCH] libxl: don't accept negative disk or 
> partition indexes"):
>> 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 - the bounds checking done (now) guarantees that the values won't
>> run into the negative range of "int" values.
> 
> IMO "unsigned int" is not a type that should be used for things which
> are like mathematical integers, even if their range happens to include
> only non-negative integers.  In C unsigned types have some very
> surprising behaviours in comparisons and subtractions.

I would call this surprising only if you think purely mathematically
(which you shouldn't when programming in any language that uses
finite width data types). Instead I find it rather natural to actively
use those properties that you call surprising, and in particular to
use unsigned types for variables that can't (or shouldn't) have
negative values (not the least because this tends to produce better
code, as no sign-extension is necessary when using such variables
e.g. as array indexes). Plus I'd be curious where you would see fit
for unsigned types (or whether you consider them evil altogether).

> So I think the correct thing to do is to check that the values are
> within sensible limits after sscanf returns.

As I'm disagreeing, I won't submit a version of the patch that does
so. Either you apply the patch as is (at least in this respect - if there
are other problems with it I'm perfectly fine to address those), or
you roll your own, or you leave the problem unfixed.

Jan


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