On 14 Apr 2016, at 16:26, Eric Blake <ebl...@redhat.com> wrote: > [adding qemu-block in cc, since Paolo can't send pull request] > > On 04/07/2016 07:09 PM, Eric Blake wrote: >> The NBD Protocol states that NBD_REP_SERVER may set >> 'length > sizeof(namelen) + namelen'; in which case the rest >> of the packet is a UTF-8 description of the export. While we >> don't know of any NBD servers that send this description yet, >> we had better consume the data so we don't choke when we start >> to talk to such a server. >> >> Also, a (buggy/malicious) server that replies with length < >> sizeof(namelen) would cause us to block waiting for bytes that >> the server is not sending, and one that replies with super-huge >> lengths could cause us to temporarily allocate up to 4G memory. >> Sanity check things before blindly reading incorrectly. >> >> Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Alex Bligh <a...@alex.org.uk> >> --- >> >> Yet another case of code introduced in 2.6 that doesn't play >> nicely with spec-compliant servers... >> >> Hopefully I've squashed them all now? >> >> nbd/client.c | 23 +++++++++++++++++++++-- >> 1 file changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/nbd/client.c b/nbd/client.c >> index 6777e58..48f2a21 100644 >> --- a/nbd/client.c >> +++ b/nbd/client.c >> @@ -192,13 +192,18 @@ static int nbd_receive_list(QIOChannel *ioc, char >> **name, Error **errp) >> return -1; >> } >> } else if (type == NBD_REP_SERVER) { >> + if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) { >> + error_setg(errp, "incorrect option length"); >> + return -1; >> + } >> if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) { >> error_setg(errp, "failed to read option name length"); >> return -1; >> } >> namelen = be32_to_cpu(namelen); >> - if (len != (namelen + sizeof(namelen))) { >> - error_setg(errp, "incorrect option mame length"); >> + len -= sizeof(namelen); >> + if (len < namelen) { >> + error_setg(errp, "incorrect option name length"); >> return -1; >> } >> if (namelen > 255) { >> @@ -214,6 +219,20 @@ static int nbd_receive_list(QIOChannel *ioc, char >> **name, Error **errp) >> return -1; >> } >> (*name)[namelen] = '\0'; >> + len -= namelen; >> + if (len) { >> + char *buf = g_malloc(len + 1); >> + if (read_sync(ioc, buf, len) != len) { >> + error_setg(errp, "failed to read export description"); >> + g_free(*name); >> + g_free(buf); >> + *name = NULL; >> + return -1; >> + } >> + buf[len] = '\0'; >> + TRACE("Ignoring export description: %s", buf); >> + g_free(buf); >> + } >> } else { >> error_setg(errp, "Unexpected reply type %x expected %x", >> type, NBD_REP_SERVER); >> > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail