[PATCH] vhost-scsi: fix T10-PI lifecycle hard BUG after endpoint

[PATCH] vhost-scsi: fix T10-PI lifecycle hard BUG after endpoint

[Jia Jia]

vhost_scsi_setup_vq_cmds() runs only from VHOST_SCSI_SET_ENDPOINT and
allocates each command's protection scatterlist array (prot_sgl) only
when VIRTIO_SCSI_F_T10_PI is already set at that moment.  Command pools
are not rebuilt later.  vhost_scsi_set_features() may still flip that
bit after the endpoint is live: it updates acked_features and returns
success without reallocating prot_sgl.

That is a feature-versus-resource lifetime mismatch.  The broken order
is:

  VHOST_SET_FEATURES(0)
  VHOST_SCSI_SET_ENDPOINT
      setup_vq_cmds() sees no T10-PI -> prot_sgl stays NULL
  VHOST_SET_FEATURES(VIRTIO_SCSI_F_T10_PI)
      only acked_features changes; command resources stay as above
  submit a T10-PI WRITE with a 129-page protection payload

The I/O path then follows the new feature bit while using the old
command objects.  vhost_scsi_mapal() (static, inlined into
vhost_scsi_handle_vq() here) does:

  sg_alloc_table_chained(table, 129, first_chunk=NULL,
                         nents_first_chunk=inline_sg_cnt)

With first_chunk NULL, __sg_alloc_table() sets curr_max_ents from
nents_first_chunk and calls sg_pool_alloc(129).  sg_pool_index() then
hits:

  BUG_ON(nents > SG_CHUNK_SIZE);   /* 129 > 128 */

The kernel reported the following call trace and register state:

  Call Trace:
   <TASK>
   ? __sg_alloc_table+0x1d8/0x250
   ? __pfx_vhost_run_work_list+0x10/0x10 [vhost]
   sg_alloc_table_chained+0x59/0xf0
   ? __pfx_sg_pool_alloc+0x10/0x10
   ? vhost_scsi_calc_sgls.constprop.0+0x43/0x60 [vhost_scsi]
   vhost_scsi_handle_vq+0xf02/0x1700 [vhost_scsi]
   ? __pfx_vhost_scsi_handle_vq+0x10/0x10 [vhost_scsi]
   vhost_scsi_handle_kick+0x37/0x50 [vhost_scsi]
   vhost_run_work_list+0x8e/0xd0 [vhost]
   vhost_task_fn+0xe1/0x210
   ret_from_fork+0x348/0x540
   </TASK>

  RIP: 0010:0x4
  CR2: 0000000000000004
  RSP: 0018:ffffc90000dbf940 EFLAGS: 00010202
  RAX: ffffffff82396810 RBX: ffff88811dc28b80 RCX: 0000000000000000
  RDX: 0000000000000000 RSI: 0000000000000820 RDI: 0000000000000081

The host worker dies and the machine stops accepting network service
until reset.  The missing object is prot_sgl after a post-endpoint
T10-PI enable introduced by commit bf2d650391be ("vhost-scsi: Allocate
T10 PI structs only when enabled").

I also checked the other feature bits handled around endpoint setup.
Only T10-PI allocates this endpoint-time per-command resource and does
not rebuild it on a later SET_FEATURES.  LOG_ALL uses lazy tvc_log and
already tears log storage down when cleared; HOTPLUG does not allocate
prot_sgl.  So the chosen fix is to refuse T10-PI enable/disable while
an endpoint is active, rather than adding a second rebuild path in
set_features().

Return -EBUSY if vs->vs_tpg is set and the T10-PI bit would change.
Userspace must CLEAR_ENDPOINT, SET_FEATURES, then SET_ENDPOINT again so
setup_vq_cmds() can allocate protection SGLs when needed.  That matches
the existing "build command resources at endpoint" model.

Fixes: bf2d650391be ("vhost-scsi: Allocate T10 PI structs only when enabled")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 drivers/vhost/scsi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d8c5..000000000000 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2219,6 +2219,7 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
 {
 	struct vhost_virtqueue *vq;
 	bool is_log, was_log;
+	bool is_t10_pi, was_t10_pi;
 	int i;
 
 	if (features & ~VHOST_SCSI_FEATURES)
@@ -2234,6 +2235,13 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
 	if (!vs->dev.nvqs)
 		goto out;
 
+	is_t10_pi = features & (1ULL << VIRTIO_SCSI_F_T10_PI);
+	was_t10_pi = vhost_has_feature(&vs->vqs[0].vq, VIRTIO_SCSI_F_T10_PI);
+	if (vs->vs_tpg && is_t10_pi != was_t10_pi) {
+		mutex_unlock(&vs->dev.mutex);
+		return -EBUSY;
+	}
+
 	is_log = features & (1 << VHOST_F_LOG_ALL);
 	/*
 	 * All VQs should have same feature.
-- 
2.43.0

Re: [PATCH] vhost-scsi: fix T10-PI lifecycle hard BUG after endpoint

[Stefan Hajnoczi]

On Wed, Jul 22, 2026 at 2:40 AM Jia Jia <physicalmtea@gmail.com> wrote:
>
> vhost_scsi_setup_vq_cmds() runs only from VHOST_SCSI_SET_ENDPOINT and
> allocates each command's protection scatterlist array (prot_sgl) only
> when VIRTIO_SCSI_F_T10_PI is already set at that moment.  Command pools
> are not rebuilt later.  vhost_scsi_set_features() may still flip that
> bit after the endpoint is live: it updates acked_features and returns
> success without reallocating prot_sgl.
>
> That is a feature-versus-resource lifetime mismatch.  The broken order
> is:
>
>   VHOST_SET_FEATURES(0)
>   VHOST_SCSI_SET_ENDPOINT
>       setup_vq_cmds() sees no T10-PI -> prot_sgl stays NULL
>   VHOST_SET_FEATURES(VIRTIO_SCSI_F_T10_PI)
>       only acked_features changes; command resources stay as above

VIRTIO_SCSI_F_T10_PI is defined in the VIRTIO spec. VIRTIO feature
bits do not change once negotiated. The device must be reset in order
to set the feature bits again.

VHOST_F_LOG_ALL is a vhost-specific feature bit that is not covered by
the VIRTIO spec and it can be changed at runtime. It is an exception.

Please generalize this fix to only allow VHOST_F_LOG_ALL to be changed
at runtime. Rather than protecting VIRTIO_SCSI_F_T10_PI specifically,
attempts to change feature bits other than VHOST_F_LOG_ALL once the
device is operational must be rejected.

Perhaps other vhost device implementations have the same issue?

Thanks,
Stefan

>   submit a T10-PI WRITE with a 129-page protection payload
>
> The I/O path then follows the new feature bit while using the old
> command objects.  vhost_scsi_mapal() (static, inlined into
> vhost_scsi_handle_vq() here) does:
>
>   sg_alloc_table_chained(table, 129, first_chunk=NULL,
>                          nents_first_chunk=inline_sg_cnt)
>
> With first_chunk NULL, __sg_alloc_table() sets curr_max_ents from
> nents_first_chunk and calls sg_pool_alloc(129).  sg_pool_index() then
> hits:
>
>   BUG_ON(nents > SG_CHUNK_SIZE);   /* 129 > 128 */
>
> The kernel reported the following call trace and register state:
>
>   Call Trace:
>    <TASK>
>    ? __sg_alloc_table+0x1d8/0x250
>    ? __pfx_vhost_run_work_list+0x10/0x10 [vhost]
>    sg_alloc_table_chained+0x59/0xf0
>    ? __pfx_sg_pool_alloc+0x10/0x10
>    ? vhost_scsi_calc_sgls.constprop.0+0x43/0x60 [vhost_scsi]
>    vhost_scsi_handle_vq+0xf02/0x1700 [vhost_scsi]
>    ? __pfx_vhost_scsi_handle_vq+0x10/0x10 [vhost_scsi]
>    vhost_scsi_handle_kick+0x37/0x50 [vhost_scsi]
>    vhost_run_work_list+0x8e/0xd0 [vhost]
>    vhost_task_fn+0xe1/0x210
>    ret_from_fork+0x348/0x540
>    </TASK>
>
>   RIP: 0010:0x4
>   CR2: 0000000000000004
>   RSP: 0018:ffffc90000dbf940 EFLAGS: 00010202
>   RAX: ffffffff82396810 RBX: ffff88811dc28b80 RCX: 0000000000000000
>   RDX: 0000000000000000 RSI: 0000000000000820 RDI: 0000000000000081
>
> The host worker dies and the machine stops accepting network service
> until reset.  The missing object is prot_sgl after a post-endpoint
> T10-PI enable introduced by commit bf2d650391be ("vhost-scsi: Allocate
> T10 PI structs only when enabled").
>
> I also checked the other feature bits handled around endpoint setup.
> Only T10-PI allocates this endpoint-time per-command resource and does
> not rebuild it on a later SET_FEATURES.  LOG_ALL uses lazy tvc_log and
> already tears log storage down when cleared; HOTPLUG does not allocate
> prot_sgl.  So the chosen fix is to refuse T10-PI enable/disable while
> an endpoint is active, rather than adding a second rebuild path in
> set_features().
>
> Return -EBUSY if vs->vs_tpg is set and the T10-PI bit would change.
> Userspace must CLEAR_ENDPOINT, SET_FEATURES, then SET_ENDPOINT again so
> setup_vq_cmds() can allocate protection SGLs when needed.  That matches
> the existing "build command resources at endpoint" model.
>
> Fixes: bf2d650391be ("vhost-scsi: Allocate T10 PI structs only when enabled")
> Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> ---
>  drivers/vhost/scsi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 9a1253b9d8c5..000000000000 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -2219,6 +2219,7 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
>  {
>         struct vhost_virtqueue *vq;
>         bool is_log, was_log;
> +       bool is_t10_pi, was_t10_pi;
>         int i;
>
>         if (features & ~VHOST_SCSI_FEATURES)
> @@ -2234,6 +2235,13 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
>         if (!vs->dev.nvqs)
>                 goto out;
>
> +       is_t10_pi = features & (1ULL << VIRTIO_SCSI_F_T10_PI);
> +       was_t10_pi = vhost_has_feature(&vs->vqs[0].vq, VIRTIO_SCSI_F_T10_PI);
> +       if (vs->vs_tpg && is_t10_pi != was_t10_pi) {
> +               mutex_unlock(&vs->dev.mutex);
> +               return -EBUSY;
> +       }
> +
>         is_log = features & (1 << VHOST_F_LOG_ALL);
>         /*
>          * All VQs should have same feature.
> --
> 2.43.0
>

Re: [PATCH] vhost-scsi: fix T10-PI lifecycle hard BUG after endpoint

[Jia Jia]

Thanks for the guidance. I understand now that the fix should follow the
virtio specification rather than use VHOST_F_LOG_ALL as the model. I will
update v2 accordingly.

Following your suggestion, I also checked the other vhost devices. For the
specific T10-PI lifecycle issue, vhost-vdpa already rejects feature changes
after FEATURES_OK. vhost-net and vhost-vsock still accept runtime feature
changes, but I have not yet confirmed a corresponding runtime failure in
those paths. I will treat them as separate audit items.

[PATCH v2] vhost-scsi: reject feature changes after endpoint

[Jia Jia]

vhost_scsi_setup_vq_cmds() runs from VHOST_SCSI_SET_ENDPOINT and allocates
each command's protection scatterlist array (prot_sgl) according to the
acknowledged VIRTIO_SCSI_F_T10_PI bit.  The command pools are not rebuilt
when VHOST_SET_FEATURES changes that bit later.

Although virtio feature bits must not change after feature negotiation,
vhost_scsi_set_features() currently accepts such a request after the
endpoint is active and updates acked_features.  Enabling T10-PI after
endpoint setup therefore leaves prot_sgl NULL while the I/O path follows
the new feature bit.

For a 129-page protection payload, vhost_scsi_mapal() passes the missing
first chunk to sg_alloc_table_chained():

  sg_alloc_table_chained(table, 129, first_chunk=NULL,
                         nents_first_chunk=inline_sg_cnt)

sg_pool_index() then hits:

  BUG_ON(nents > SG_CHUNK_SIZE);   /* 129 > 128 */

The kernel reported the following call trace and register state:

  Call Trace:
   <TASK>
   ? __sg_alloc_table+0x1d8/0x250
   ? __pfx_vhost_run_work_list+0x10/0x10 [vhost]
   sg_alloc_table_chained+0x59/0xf0
   ? __pfx_sg_pool_alloc+0x10/0x10
   ? vhost_scsi_calc_sgls.constprop.0+0x43/0x60 [vhost_scsi]
   vhost_scsi_handle_vq+0xf02/0x1700 [vhost_scsi]
   ? __pfx_vhost_scsi_handle_vq+0x10/0x10 [vhost_scsi]
   vhost_scsi_handle_kick+0x37/0x50 [vhost_scsi]
   vhost_run_work_list+0x8e/0xd0 [vhost]
   vhost_task_fn+0xe1/0x210
   ret_from_fork+0x348/0x540
   </TASK>

  RIP: 0010:0x4
  CR2 = 0x4
  RSP: 0018:ffffc90000dbf940 EFLAGS: 00010202
  RAX: ffffffff82396810 RBX: ffff88811dc28b80 RCX: 0000000000000000
  RDX: 0000000000000000 RSI: 0000000000000820 RDI: 0000000000000081

VHOST_F_LOG_ALL is a vhost-specific runtime feature and remains the only
exception.

Reject changes to any feature other than VHOST_F_LOG_ALL while the
endpoint is active.  This preserves the existing runtime log toggle while
preventing feature-dependent command resources and data-path state from
becoming inconsistent.  Userspace must clear the endpoint before changing
any other negotiated feature and set the endpoint up again afterward.

Fixes: bf2d650391be ("vhost-scsi: Allocate T10 PI structs only when enabled")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
Changes in v2:
- Reject changes to every feature except VHOST_F_LOG_ALL after endpoint
  setup, following review feedback.
- Keep the existing runtime VHOST_F_LOG_ALL cleanup path.
 drivers/vhost/scsi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d8c5..000000000000 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2219,6 +2219,7 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
 {
 	struct vhost_virtqueue *vq;
 	bool is_log, was_log;
+	u64 old_features;
 	int i;
 
 	if (features & ~VHOST_SCSI_FEATURES)
@@ -2234,6 +2235,14 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
 	if (!vs->dev.nvqs)
 		goto out;
 
+	old_features = vs->vqs[0].vq.acked_features;
+	if (vs->vs_tpg &&
+	    ((features ^ old_features) &
+	     ~(1ULL << VHOST_F_LOG_ALL))) {
+		mutex_unlock(&vs->dev.mutex);
+		return -EBUSY;
+	}
+
 	is_log = features & (1 << VHOST_F_LOG_ALL);
 	/*
 	 * All VQs should have same feature.
-- 
2.43.0