diff mbox series

[v3,2/2] nbd: add comments about double lock for config_lock confusion

Message ID 20201103065156.342756-3-xiubli@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/2] nbd: fix use-after-freed crash for nbd->recv_workq | expand

Commit Message

Xiubo Li Nov. 3, 2020, 6:51 a.m. UTC 
From: Xiubo Li <xiubli@redhat.com>

When calling the ioctl(), fget() will be called on this fd, and
nbd_release() is only called when the fd's refcount drops to zero.
With this we can make sure that the nbd_release() won't be called
before the ioctl() finished.

So there won't have the double lock issue for the "config_lock",
which has already been held by nbd_ioctl().

Reviewed-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 drivers/block/nbd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Josef Bacik Nov. 3, 2020, 5:01 p.m. UTC | #1 
On 11/3/20 1:51 AM, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> When calling the ioctl(), fget() will be called on this fd, and
> nbd_release() is only called when the fd's refcount drops to zero.
> With this we can make sure that the nbd_release() won't be called
> before the ioctl() finished.
> 
> So there won't have the double lock issue for the "config_lock",
> which has already been held by nbd_ioctl().
> 
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Signed-off-by: Xiubo Li <xiubli@redhat.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
diff mbox series

Patch

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index e4c37677f3df..55466534cde6 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1345,6 +1345,17 @@  static void nbd_clear_sock_ioctl(struct nbd_device *nbd,
 	sock_shutdown(nbd);
 	__invalidate_device(bdev, true);
 	nbd_bdev_reset(bdev);
+
+	/*
+	 * When calling the ioctl(), fget() will be called on this
+	 * fd, and nbd_release() is only called when the fd's refcount
+	 * drops to zero. With this we can make sure that the
+	 * nbd_release() won't be called before the ioctl() finished.
+	 *
+	 * So there won't have the double lock issue if it will
+	 * call the nbd_config_put() here for the "config_lock", which
+	 * has already been held by nbd_ioctl().
+	 */
 	if (test_and_clear_bit(NBD_RT_HAS_CONFIG_REF,
 			       &nbd->config->runtime_flags))
 		nbd_config_put(nbd);