From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751448AbcGPHm5 (ORCPT ); Sat, 16 Jul 2016 03:42:57 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:36813 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751017AbcGPHmz (ORCPT ); Sat, 16 Jul 2016 03:42:55 -0400 MIME-Version: 1.0 In-Reply-To: <1466760574-1916-2-git-send-email-mpa@pengutronix.de> References: <1466760574-1916-1-git-send-email-mpa@pengutronix.de> <1466760574-1916-2-git-send-email-mpa@pengutronix.de> From: Pranay Srivastava Date: Sat, 16 Jul 2016 13:12:53 +0530 Message-ID: Subject: Re: [PATCH 2/2] nbd: Disallow ioctls on disconnected block device To: Markus Pargmann Cc: nbd-general@lists.sourceforge.net, linux-kernel@vger.kernel.org, Wouter Verhelst Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Fri, Jun 24, 2016 at 2:59 PM, Markus Pargmann wrote: > After NBD_DO_IT exited the block device may still be used. Make sure > that we handle intended disconnects differently and do not allow any > changed of the nbd device. > > This patch should avoid that the nbd-client connects to a different server > and the users of the block device are suddenly reading/writing from a > different backend device. > > For timeouts it is still possible to setup a new socket so that the > connection may be refreshed without creating problems for all users. But Shouldn't time out be checked for last end point? For example, consider the following steps 1) Timeout occurs but server[nbd-s1] comes up again albeit with a different network address. 2) The previous network address of server [nbd-s1] has now been assigned to another new nbd server [nbd-s2] 3) A new nbd-client tries to setup the socket again, Negotiation would be done again [correct?]. If correct then wouldn't we be sending data to wrong device this time? So instead can't we put a mechanism in place for network address + mac to be same for allowing clients to reconnect? Do let me know if this is not of concern. 4) If 3) doesn't apply then let's disallow all ioctls until nbd device is reset. > > Signed-off-by: Markus Pargmann > --- > drivers/block/nbd.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 620660f3ff0f..39358efac73e 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -708,6 +708,18 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd); > static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, > unsigned int cmd, unsigned long arg) > { > + /* > + * After a disconnect was instructed, do not allow any further actions > + * on the block device that would lead to a new connected endpoint. > + * This condition stays until nbd_reset was called either because all > + * users closed the device or because of CLEAR_SOCK. > + */ > + if (nbd->disconnect && > + cmd != NBD_CLEAR_SOCK && cmd != NBD_PRINT_DEBUG) { > + dev_info(disk_to_dev(nbd->disk), "Device is still busy after instructing a disconnect\n"); > + return -EBUSY; > + } > + > switch (cmd) { > case NBD_DISCONNECT: { > struct request sreq; > @@ -733,11 +745,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, > } > > case NBD_CLEAR_SOCK: > - sock_shutdown(nbd); > - nbd_clear_que(nbd); > - BUG_ON(!list_empty(&nbd->queue_head)); > - BUG_ON(!list_empty(&nbd->waiting_queue)); > - kill_bdev(bdev); > + if (nbd->disconnect) { > + nbd_reset(nbd); > + } else { > + sock_shutdown(nbd); > + nbd_clear_que(nbd); > + BUG_ON(!list_empty(&nbd->queue_head)); > + BUG_ON(!list_empty(&nbd->waiting_queue)); > + kill_bdev(bdev); > + } > return 0; > > case NBD_SET_SOCK: { > @@ -812,8 +828,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, > mutex_lock(&nbd->tx_lock); > nbd->task_recv = NULL; > > - if (nbd->disconnect) /* user requested, ignore socket errors */ > + if (nbd->disconnect) { /* user requested, ignore socket errors */ > + sock_shutdown(nbd); > error = 0; > + } > if (nbd->timedout) > error = -ETIMEDOUT; > > -- > 2.1.4 > -- ---P.K.S