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

Commit Message

Paolo Bonzini April 7, 2016, 4:50 p.m. UTC

From: Eric Blake <[e-mail]>

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 <[e-mail]>
Message-Id: <[e-mail]>
Signed-off-by: Paolo Bonzini <[e-mail]>
---
 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 <[e-mail]>
> 
> 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

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 {
             /*