diff mbox

[PULL,09/15] nbd: Don't kill server when client requests unknown option

Message ID 1460047845-14488-10-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini April 7, 2016, 4:50 p.m. UTC 
From: Eric Blake <eblake@redhat.com>

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>
Message-Id: <1459982918-32229-1-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd/server.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Eric Blake April 7, 2016, 10:14 p.m. UTC | #1 
On 04/07/2016 10:50 AM, Paolo Bonzini wrote:
> From: Eric Blake <eblake@redhat.com>
> 
> 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.
> 

I found a couple more in our server's handling of NBD_OPT_STARTTLS; now
that you are probably going to resend the pull request, you may want to
include that one.
https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg01299.html
diff mbox

Patch

diff --git a/nbd/server.c b/nbd/server.c
index 6d9c15a..2a4dd10 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -483,9 +483,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 {
             /*