[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] inet_listen_opts: add error checking
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v4] inet_listen_opts: add error checking |
Date: |
Fri, 13 Dec 2013 05:49:35 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 |
On 12/13/2013 05:42 AM, Eric Blake wrote:
> On 12/13/2013 03:12 AM, Gerd Hoffmann wrote:
>> Don't use atoi() function which doesn't detect errors, switch to
>> strtol and error out on failures. Also add a range check while
>> being at it.
>>
>> [ v4: didn't commit buildfix. -ENOCOFFEE. sorry for the spam ]
>> [ v3: oops, v2 didn't build ]
>> [ v2: use parse_uint_full instead of strtol ]
>
>
>> + if (port_offset) {
>> + unsigned long long baseport;
>> + if (parse_uint_full(port, &baseport, 10) < 0) {
>> + error_setg(errp, "can't convert to a number: %s", port);
>> + return -1;
>> + }
>> + if (baseport + port_offset > 65535) {
>> + error_setg(errp, "port %s out of range", port);
>
> error_setg() is still reporting on a bogus errno value at this point.
My bad, now it's my turn for early-morning confusion.
I was thinking of error_setg_errno, but you are using error_setg.
That said, you STILL have a problem:
> + if (baseport + port_offset > 65535) {
If baseport is 0xfffffffffffffffe, and port_offset is 5000, then their
sum is 4998 which is not > 65535, so you fall through:
> + error_setg(errp, "port %s out of range", port);
> + return -1;
> + }
> + snprintf(port, sizeof(port), "%d", (int)baseport + port_offset);
and happily use a value that is less than port_offset at this point. I
don't think you meant to do that.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature