From: "Chuck Lever" <cel@kernel.org>
To: "Christoph Hellwig" <hch@lst.de>,
"Chuck Lever" <chuck.lever@oracle.com>,
"Jeff Layton" <jlayton@kernel.org>,
"Amir Goldstein" <amir73il@gmail.com>
Cc: NeilBrown <neil@brown.name>,
"Olga Kornievskaia" <okorniev@redhat.com>,
"Dai Ngo" <Dai.Ngo@oracle.com>, "Tom Talpey" <tom@talpey.com>,
"Carlos Maiolino" <cem@kernel.org>,
linux-nfs@vger.kernel.org, linux-xfs@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 5/7] nfsd/blocklayout: support GETDEVICEINFO for multiple devices
Date: Mon, 23 Mar 2026 09:53:33 -0400 [thread overview]
Message-ID: <736ddc8a-7a28-4900-b3c1-0368cad24085@app.fastmail.com> (raw)
In-Reply-To: <20260323070746.2940140-6-hch@lst.de>
On Mon, Mar 23, 2026, at 3:07 AM, Christoph Hellwig wrote:
> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> index 8ca8fd8f70cb..fb13f86f8eb5 100644
> --- a/fs/nfsd/blocklayout.c
> +++ b/fs/nfsd/blocklayout.c
> @@ -303,15 +303,19 @@ nfsd4_block_get_unique_id(struct gendisk *disk,
> struct pnfs_block_volume *b)
> }
>
> static int
> -nfsd4_block_get_device_info_scsi(struct super_block *sb,
> - struct nfs4_client *clp,
> - struct nfsd4_getdeviceinfo *gdp)
> +nfsd4_block_get_device_info_scsi(struct block_device *bdev,
> + struct nfs4_client *clp, struct nfsd4_getdeviceinfo *gdp)
> {
> struct pnfs_block_deviceaddr *dev;
> struct pnfs_block_volume *b;
> const struct pr_ops *ops;
> int ret;
>
> + if (bdev_is_partition(bdev))
> + return -EINVAL;
> + if (!exportfs_bdev_supports_out_of_band_id(bdev))
> + return -EINVAL;
> +
> dev = kzalloc_flex(*dev, volumes, 1);
> if (!dev)
> return -ENOMEM;
> @@ -323,30 +327,28 @@ nfsd4_block_get_device_info_scsi(struct super_block *sb,
> b->type = PNFS_BLOCK_VOLUME_SCSI;
> b->scsi.pr_key = nfsd4_scsi_pr_key(clp);
>
> - ret = nfsd4_block_get_unique_id(sb->s_bdev->bd_disk, b);
> + ret = nfsd4_block_get_unique_id(bdev->bd_disk, b);
> if (ret < 0)
> goto out_free_dev;
>
> ret = -EINVAL;
> - ops = sb->s_bdev->bd_disk->fops->pr_ops;
> + ops = bdev->bd_disk->fops->pr_ops;
> if (!ops) {
> - pr_err("pNFS: device %s does not support PRs.\n",
> - sb->s_id);
> + pr_err("pNFS: device %pg does not support persistent reservations.\n",
> + bdev);
> goto out_free_dev;
> }
>
> - ret = ops->pr_register(sb->s_bdev, 0, NFSD_MDS_PR_KEY, true);
> + ret = ops->pr_register(bdev, 0, NFSD_MDS_PR_KEY, true);
> if (ret) {
> - pr_err("pNFS: failed to register key for device %s.\n",
> - sb->s_id);
> + pr_err("pNFS: failed to register key for device %pg.\n", bdev);
> goto out_free_dev;
> }
>
> - ret = ops->pr_reserve(sb->s_bdev, NFSD_MDS_PR_KEY,
> + ret = ops->pr_reserve(bdev, NFSD_MDS_PR_KEY,
> PR_EXCLUSIVE_ACCESS_REG_ONLY, 0);
> if (ret) {
> - pr_err("pNFS: failed to reserve device %s.\n",
> - sb->s_id);
> + pr_err("pNFS: failed to reserve device %pd.\n", bdev);
> goto out_free_dev;
> }
>
> @@ -364,9 +366,16 @@ nfsd4_scsi_proc_getdeviceinfo(struct super_block *sb,
> struct nfs4_client *clp,
> struct nfsd4_getdeviceinfo *gdp)
> {
> - if (bdev_is_partition(sb->s_bdev))
> - return nfserr_inval;
> - return nfserrno(nfsd4_block_get_device_info_scsi(sb, clp, gdp));
> + struct block_device *bdev = sb->s_bdev;
> +
> + if (sb->s_export_op->block_ops->devid_to_bdev) {
> + bdev = sb->s_export_op->block_ops->devid_to_bdev(sb,
> + gdp->gd_devid.dev_idx);
> + if (IS_ERR(bdev))
> + return nfserrno(PTR_ERR(bdev));
> + }
> +
> + return nfserrno(nfsd4_block_get_device_info_scsi(bdev, clp, gdp));
> }
> static __be32
> nfsd4_scsi_proc_layoutcommit(struct inode *inode, struct svc_rqst *rqstp,
LLM review identified this issue:
402 static void
403 nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
404 {
405 struct nfs4_client *clp = ls->ls_stid.sc_client;
406 struct block_device *bdev = file->nf_file->f_path.mnt->mnt_sb->s_bdev;
407 int status;
408
This always gets the data device. For an RT inode whose layout was
granted with dev_idx=1, PR registration happened on the RT bdev (in
nfsd4_block_get_device_info_scsi after devid_to_bdev), but the
preempt here fires against the data bdev.
The difficulty is that fence_client receives a nfs4_layout_stateid
and nfsd_file, neither of which carries the dev_idx that was encoded
in the device ID at layout grant time.
So the fix isn't just a one-liner — it requires threading the device
index (or the bdev) through to the fence path, likely by storing it
in the layout stateid when the layout is granted.
Pre-series invariant (broken by this series):
Before the series, nfsd4_setup_layout_type checked
sb->s_bdev->bd_disk->fops->pr_ops directly before enabling LAYOUT_SCSI.
That guaranteed fence_client could safely dereference sb->s_bdev's
pr_ops. After the series, the check delegates to
xfs_fs_layouts_supported, which uses:
if (exportfs_bdev_supports_out_of_band_id(sb->s_bdev) ||
(mp->m_rtdev_targp &&
exportfs_bdev_supports_out_of_band_id(mp->m_rtdev_targp->bt_bdev)))
supported |= EXPFS_BLOCK_OUT_OF_BAND_ID;
This means LAYOUT_SCSI can be enabled when only the RT device has
pr_ops and get_unique_id.
Concrete path to NULL deref:
1. XFS with RT: data device is plain SATA (no pr_ops), RT device is NVMe (has pr_ops + get_unique_id)
2. xfs_fs_layouts_supported → EXPFS_BLOCK_OUT_OF_BAND_ID set (via RT device)
3. nfsd4_setup_layout_type → enables LAYOUT_SCSI
4. Client does LAYOUTGET for RT inode → map_blocks sets dev_idx=1
5. Client does GETDEVICEINFO with dev_idx=1 → devid_to_bdev returns RT bdev → PR register/reserve on RT bdev succeeds
6. Recall fails → nfsd4_scsi_fence_client (line 406): bdev = ...->s_bdev (data device)
7. Line 409: bdev->bd_disk->fops->pr_ops->pr_preempt(...) — pr_ops is NULL → kernel oops
Even when both devices have pr_ops, the fence still targets the
wrong device: the client's PR registration is on the RT bdev (done
during GETDEVICEINFO), but the preempt fires on the data bdev,
leaving the RT device unfenced.
The issue is reachable on any XFS+RT configuration where the RT
device supports SCSI persistent reservations but the data device
does not.
--
Chuck Lever
next prev parent reply other threads:[~2026-03-23 13:53 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-23 7:07 support multiple block devices per file system for block-style layouts Christoph Hellwig
2026-03-23 7:07 ` [PATCH 1/7] exportfs: split out the ops for layout-based block device access Christoph Hellwig
2026-03-23 13:39 ` Chuck Lever
2026-03-26 5:33 ` Christoph Hellwig
2026-03-23 7:07 ` [PATCH 2/7] exportfs: don't pass struct iattr to ->commit_blocks Christoph Hellwig
2026-03-23 14:05 ` Chuck Lever
2026-03-26 5:36 ` Christoph Hellwig
2026-03-23 7:07 ` [PATCH 3/7] exportfs,nfsd: rework checking for layout-based block device access support Christoph Hellwig
2026-03-23 7:07 ` [PATCH 4/7] nfsd: support multiple pNFS device IDs Christoph Hellwig
2026-03-23 7:07 ` [PATCH 5/7] nfsd/blocklayout: support GETDEVICEINFO for multiple devices Christoph Hellwig
2026-03-23 13:53 ` Chuck Lever [this message]
2026-03-26 5:38 ` Christoph Hellwig
2026-03-26 12:37 ` Chuck Lever
2026-03-23 7:07 ` [PATCH 6/7] exportfs: return a device index from ->map_blocks Christoph Hellwig
2026-03-23 7:07 ` [PATCH 7/7] xfs: support layout-based block device access on the RT device Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=736ddc8a-7a28-4900-b3c1-0368cad24085@app.fastmail.com \
--to=cel@kernel.org \
--cc=Dai.Ngo@oracle.com \
--cc=amir73il@gmail.com \
--cc=cem@kernel.org \
--cc=chuck.lever@oracle.com \
--cc=hch@lst.de \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=neil@brown.name \
--cc=okorniev@redhat.com \
--cc=tom@talpey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).