Re: [Qemu-devel] [PATCH v5 2/2] sheepdog: support user-defined redundancy option

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]

On 11/05/2013 07:37 AM, Stefan Hajnoczi wrote:

>> +
>> +    copy = strtol(n1, NULL, 10);
>> +    if (copy > SD_MAX_COPIES) {
>> +        return -EINVAL;
>> +    }

> 
> The string manipulation can be simplified using sscanf(3) and
> is_numeric() can be dropped:
> 
> static int parse_redundancy(BDRVSheepdogState *s, const char *opt)
> {
>     struct SheepdogInode *inode = &s->inode;
>     uint8_t copy, parity;
>     int n;
> 
>     n = sscanf(opt, "%hhu:%hhu", &copy, &parity);

Personally, I detest the use of sscanf() to parse integers out of
strings, because POSIX says that behavior is undefined if overflow
occurs.  For internal strings, you can get away with it.  But for
untrusted input that did not originate in your process, a user can mess
you up by passing a string that parses larger than the integer you are
trying to store into, where the behavior is unspecified whether it wraps
around module 256, parses additional digits, or any other odd behavior.
 By the time you've added code to sanitize untrusted input, it's just as
fast to use strtol() anyways.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]