| Message ID | 1400669607-18205-1-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
Gerd Hoffmann <kraxel@redhat.com> writes: > 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. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> -----Original Message----- > From: qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org > [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On > Behalf Of Gerd Hoffmann > Sent: Wednesday, May 21, 2014 6:53 PM > To: qemu-devel@nongnu.org > Cc: Gerd Hoffmann > Subject: [Qemu-devel] [PATCH] inet_listen_opts: add error checking > > 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. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > util/qemu-sockets.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 8818d7c..22971e2 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -133,8 +133,19 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, > Error **errp) > ai.ai_family = PF_INET6; > > /* lookup */ > - if (port_offset) > - snprintf(port, sizeof(port), "%d", atoi(port) + port_offset); > + if (port_offset) { > + unsigned long long baseport; unsigned long long is not necessary, unsigned long is enough. > + if (parse_uint_full(port, &baseport, 10) < 0) { > + error_setg(errp, "can't convert to a number: %s", port); > + return -1; > + } > + if (base_port > 65535 || base_port is undeclared. > + baseport + port_offset > 65535) { > + error_setg(errp, "port %s out of range", port); > + return -1; > + } > + snprintf(port, sizeof(port), "%d", (int)baseport + port_offset); > + } > rc = getaddrinfo(strlen(addr) ? addr : NULL, port, &ai, &res); > if (rc != 0) { > error_setg(errp, "address resolution failed for %s:%s: %s", addr, > port, > -- > 1.8.3.1 > Best regards, -Gonglei
> > /* lookup */ > > - if (port_offset) > > - snprintf(port, sizeof(port), "%d", atoi(port) + port_offset); > > + if (port_offset) { > > + unsigned long long baseport; > > unsigned long long is not necessary, unsigned long is enough. > > > + if (parse_uint_full(port, &baseport, 10) < 0) { parse_uint_full prototype uses "unsigned long long". And on 32bit machines it actually is a difference (long long is 64bit integer whereas long is 32bit only). > > + error_setg(errp, "can't convert to a number: %s", port); > > + return -1; > > + } > > + if (base_port > 65535 || > > base_port is undeclared. Oops, I'll fix. cheers, Gerd
> -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: Thursday, May 22, 2014 1:44 PM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org > Subject: Re: [Qemu-devel] [PATCH] inet_listen_opts: add error checking > > > > /* lookup */ > > > - if (port_offset) > > > - snprintf(port, sizeof(port), "%d", atoi(port) + port_offset); > > > + if (port_offset) { > > > + unsigned long long baseport; > > > > unsigned long long is not necessary, unsigned long is enough. > > > > > + if (parse_uint_full(port, &baseport, 10) < 0) { > > parse_uint_full prototype uses "unsigned long long". And on 32bit > machines it actually is a difference (long long is 64bit integer whereas > long is 32bit only). > Okay, Thanks. Best regards, -Gonglei
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 8818d7c..22971e2 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -133,8 +133,19 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp) ai.ai_family = PF_INET6; /* lookup */ - if (port_offset) - snprintf(port, sizeof(port), "%d", atoi(port) + port_offset); + 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 (base_port > 65535 || + baseport + port_offset > 65535) { + error_setg(errp, "port %s out of range", port); + return -1; + } + snprintf(port, sizeof(port), "%d", (int)baseport + port_offset); + } rc = getaddrinfo(strlen(addr) ? addr : NULL, port, &ai, &res); if (rc != 0) { error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
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. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- util/qemu-sockets.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)