| Message ID | 1459982918-32229-1-git-send-email-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/04/2016 00:48, Eric Blake wrote: > nbd-server.c currently fails to handle unsupported options properly. > If during option haggling the client sends an unknown request, the > server kills the connection instead of letting the client try to > fall back to something older. This is precisely what advertising > NBD_FLAG_FIXED_NEWSTYLE was supposed to fix. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > > Turns out our server has a very similar bug to the client. > If desired, I can spin a v3 that moves the hunk in nbd/client.c > to the previous patch. I'll do that myself. Thanks for the patches! Paolo
Adding qemu-stable; this fix needs to be backported to 2.5.x stable series (in the file ./nbd.c at the time), if we want to be able to allow a 2.7 client to connect to a 2.5 server. On 04/06/2016 04:48 PM, Eric Blake wrote: > nbd-server.c currently fails to handle unsupported options properly. > If during option haggling the client sends an unknown request, the > server kills the connection instead of letting the client try to > fall back to something older. This is precisely what advertising > NBD_FLAG_FIXED_NEWSTYLE was supposed to fix. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > > Turns out our server has a very similar bug to the client. > If desired, I can spin a v3 that moves the hunk in nbd/client.c > to the previous patch. > > nbd/client.c | 2 ++ > nbd/server.c | 5 ++++- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/nbd/client.c b/nbd/client.c > index 67116b9..fc382c5 100644 > --- a/nbd/client.c > +++ b/nbd/client.c > @@ -109,6 +109,8 @@ static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t opt, uint32_t type, > > switch (type) { > case NBD_REP_ERR_UNSUP: > + TRACE("server doesn't understand request %d, attempting fallback", > + opt); > result = 0; > goto cleanup; > > diff --git a/nbd/server.c b/nbd/server.c > index b95571b..7843584 100644 > --- a/nbd/server.c > +++ b/nbd/server.c > @@ -482,9 +482,12 @@ static int nbd_negotiate_options(NBDClient *client) > return -EINVAL; > default: > TRACE("Unsupported option 0x%x", clientflags); > + if (nbd_negotiate_drop_sync(client->ioc, length) != length) { > + return -EIO; > + } > nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP, > clientflags); > - return -EINVAL; > + break; > } > } else { > /* >
diff --git a/nbd/client.c b/nbd/client.c index 67116b9..fc382c5 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -109,6 +109,8 @@ static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t opt, uint32_t type, switch (type) { case NBD_REP_ERR_UNSUP: + TRACE("server doesn't understand request %d, attempting fallback", + opt); result = 0; goto cleanup; diff --git a/nbd/server.c b/nbd/server.c index b95571b..7843584 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -482,9 +482,12 @@ static int nbd_negotiate_options(NBDClient *client) return -EINVAL; default: TRACE("Unsupported option 0x%x", clientflags); + if (nbd_negotiate_drop_sync(client->ioc, length) != length) { + return -EIO; + } nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP, clientflags); - return -EINVAL; + break; } } else { /*
nbd-server.c currently fails to handle unsupported options properly. If during option haggling the client sends an unknown request, the server kills the connection instead of letting the client try to fall back to something older. This is precisely what advertising NBD_FLAG_FIXED_NEWSTYLE was supposed to fix. Signed-off-by: Eric Blake <eblake@redhat.com> --- Turns out our server has a very similar bug to the client. If desired, I can spin a v3 that moves the hunk in nbd/client.c to the previous patch. nbd/client.c | 2 ++ nbd/server.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-)