Since commit d869da91cccb ("nfs/blocklayout: Fix premature PR key
unregistration") an unmount of a pNFS SCSI layout-enabled NFS may
dereference a NULL block_device in:

  bl_unregister_scsi+0x16/0xe0 [blocklayoutdriver]
  bl_free_device+0x70/0x80 [blocklayoutdriver]
  bl_free_deviceid_node+0x12/0x30 [blocklayoutdriver]
  nfs4_put_deviceid_node+0x60/0xc0 [nfsv4]
  nfs4_deviceid_purge_client+0x132/0x190 [nfsv4]
  unset_pnfs_layoutdriver+0x59/0x60 [nfsv4]
  nfs4_destroy_server+0x36/0x70 [nfsv4]
  nfs_free_server+0x23/0xe0 [nfs]
  deactivate_locked_super+0x30/0xb0
  cleanup_mnt+0xba/0x150
  task_work_run+0x59/0x90
  syscall_exit_to_user_mode+0x217/0x220
  do_syscall_64+0x8e/0x160

This happens because even though we were able to create the
nfs4_deviceid_node, the lookup for the device was unable to attach the
block device to the pnfs_block_dev.

If we never found a block device to register, we can avoid this case with
the PNFS_BDEV_REGISTERED flag.  Move the deref behind the test for the
flag.

Fixes: d869da91cccb ("nfs/blocklayout: Fix premature PR key unregistration")
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/blocklayout/dev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
index 6252f4447945..cab8809f0e0f 100644
--- a/fs/nfs/blocklayout/dev.c
+++ b/fs/nfs/blocklayout/dev.c
@@ -20,9 +20,6 @@ static void bl_unregister_scsi(struct pnfs_block_dev *dev)
 	const struct pr_ops *ops = bdev->bd_disk->fops->pr_ops;
 	int status;
 
-	if (!test_and_clear_bit(PNFS_BDEV_REGISTERED, &dev->flags))
-		return;
-
 	status = ops->pr_register(bdev, dev->pr_key, 0, false);
 	if (status)
 		trace_bl_pr_key_unreg_err(bdev, dev->pr_key, status);
@@ -58,7 +55,8 @@ static void bl_unregister_dev(struct pnfs_block_dev *dev)
 		return;
 	}
 
-	if (dev->type == PNFS_BLOCK_VOLUME_SCSI)
+	if (dev->type == PNFS_BLOCK_VOLUME_SCSI &&
+		test_and_clear_bit(PNFS_BDEV_REGISTERED, &dev->flags))
 		bl_unregister_scsi(dev);
 }
 
-- 
2.47.0

On Wed, Nov 20, 2024 at 09:09:34AM -0500, Benjamin Coddington wrote:
> Since commit d869da91cccb ("nfs/blocklayout: Fix premature PR key
> unregistration") an unmount of a pNFS SCSI layout-enabled NFS may
> dereference a NULL block_device in:
> 
>   bl_unregister_scsi+0x16/0xe0 [blocklayoutdriver]
>   bl_free_device+0x70/0x80 [blocklayoutdriver]
>   bl_free_deviceid_node+0x12/0x30 [blocklayoutdriver]
>   nfs4_put_deviceid_node+0x60/0xc0 [nfsv4]
>   nfs4_deviceid_purge_client+0x132/0x190 [nfsv4]
>   unset_pnfs_layoutdriver+0x59/0x60 [nfsv4]
>   nfs4_destroy_server+0x36/0x70 [nfsv4]
>   nfs_free_server+0x23/0xe0 [nfs]
>   deactivate_locked_super+0x30/0xb0
>   cleanup_mnt+0xba/0x150
>   task_work_run+0x59/0x90
>   syscall_exit_to_user_mode+0x217/0x220
>   do_syscall_64+0x8e/0x160
> 
> This happens because even though we were able to create the
> nfs4_deviceid_node, the lookup for the device was unable to attach the
> block device to the pnfs_block_dev.
> 
> If we never found a block device to register, we can avoid this case with
> the PNFS_BDEV_REGISTERED flag.  Move the deref behind the test for the
> flag.
> 
> Fixes: d869da91cccb ("nfs/blocklayout: Fix premature PR key unregistration")
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/nfs/blocklayout/dev.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
> index 6252f4447945..cab8809f0e0f 100644
> --- a/fs/nfs/blocklayout/dev.c
> +++ b/fs/nfs/blocklayout/dev.c
> @@ -20,9 +20,6 @@ static void bl_unregister_scsi(struct pnfs_block_dev *dev)
>  	const struct pr_ops *ops = bdev->bd_disk->fops->pr_ops;
>  	int status;
>  
> -	if (!test_and_clear_bit(PNFS_BDEV_REGISTERED, &dev->flags))
> -		return;
> -
>  	status = ops->pr_register(bdev, dev->pr_key, 0, false);
>  	if (status)
>  		trace_bl_pr_key_unreg_err(bdev, dev->pr_key, status);
> @@ -58,7 +55,8 @@ static void bl_unregister_dev(struct pnfs_block_dev *dev)
>  		return;
>  	}
>  
> -	if (dev->type == PNFS_BLOCK_VOLUME_SCSI)
> +	if (dev->type == PNFS_BLOCK_VOLUME_SCSI &&
> +		test_and_clear_bit(PNFS_BDEV_REGISTERED, &dev->flags))
>  		bl_unregister_scsi(dev);
>  }
>  
> -- 
> 2.47.0
> 

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>

-- 
Chuck Lever