public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ublk: fix ublksrv pid handling for pid namespaces
@ 2026-01-11  0:00 Seamus Connor
  2026-01-12  5:40 ` Ming Lei
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Seamus Connor @ 2026-01-11  0:00 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block, Caleb Sander; +Cc: linux-kernel

When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
the stored init-ns tgid against the userspace pid (getpid vnr), so the
check failed and control ops could not proceed. Compare against the
caller’s init-ns tgid and store that value, then translate it back to
the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
shows a sensible pid.

Testing: start/recover in a pid namespace; `ublk list` shows
reasonable pid values in init, child, and sibling namespaces.

Fixes: d37a224fc119 ("ublk: validate ublk server pid")
Signed-off-by: Seamus Connor <sconnor@purestorage.com>
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
---
 drivers/block/ublk_drv.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 79847e0b9e88..9ef6432fef7c 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2858,7 +2858,6 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
  const struct ublksrv_ctrl_cmd *header)
 {
  const struct ublk_param_basic *p = &ub->params.basic;
- int ublksrv_pid = (int)header->data[0];
  struct queue_limits lim = {
  .logical_block_size = 1 << p->logical_bs_shift,
  .physical_block_size = 1 << p->physical_bs_shift,
@@ -2874,8 +2873,6 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
  struct gendisk *disk;
  int ret = -EINVAL;

- if (ublksrv_pid <= 0)
- return -EINVAL;
  if (!(ub->params.types & UBLK_PARAM_TYPE_BASIC))
  return -EINVAL;

@@ -2922,7 +2919,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
  if (wait_for_completion_interruptible(&ub->completion) != 0)
  return -EINTR;

- if (ub->ublksrv_tgid != ublksrv_pid)
+ if (ub->ublksrv_tgid != current->tgid)
  return -EINVAL;

  mutex_lock(&ub->mutex);
@@ -2941,7 +2938,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
  disk->fops = &ub_fops;
  disk->private_data = ub;

- ub->dev_info.ublksrv_pid = ublksrv_pid;
+ ub->dev_info.ublksrv_pid = current->tgid;
  ub->ub_disk = disk;

  ublk_apply_params(ub);
@@ -3276,12 +3273,32 @@ static int ublk_ctrl_stop_dev(struct ublk_device *ub)
 static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
  const struct ublksrv_ctrl_cmd *header)
 {
+ struct task_struct *p;
+ struct pid *pid;
+ struct ublksrv_ctrl_dev_info dev_info;
+ __s32 init_ublksrv_tgid = ub->dev_info.ublksrv_pid;
  void __user *argp = (void __user *)(unsigned long)header->addr;

  if (header->len < sizeof(struct ublksrv_ctrl_dev_info) || !header->addr)
  return -EINVAL;

- if (copy_to_user(argp, &ub->dev_info, sizeof(ub->dev_info)))
+ memcpy(&dev_info, &ub->dev_info, sizeof(dev_info));
+ dev_info.ublksrv_pid = -1;
+
+ if (init_ublksrv_tgid > 0) {
+ rcu_read_lock();
+ pid = find_pid_ns(init_ublksrv_tgid, &init_pid_ns);
+ p = pid_task(pid, PIDTYPE_TGID);
+ if (p) {
+ int vnr = task_tgid_vnr(p);
+
+ if (vnr)
+ dev_info.ublksrv_pid = vnr;
+ }
+ rcu_read_unlock();
+ }
+
+ if (copy_to_user(argp, &dev_info, sizeof(dev_info)))
  return -EFAULT;

  return 0;
@@ -3414,7 +3431,6 @@ static int ublk_ctrl_start_recovery(struct
ublk_device *ub,
 static int ublk_ctrl_end_recovery(struct ublk_device *ub,
  const struct ublksrv_ctrl_cmd *header)
 {
- int ublksrv_pid = (int)header->data[0];
  int ret = -EINVAL;

  pr_devel("%s: Waiting for all FETCH_REQs, dev id %d...\n", __func__,
@@ -3426,7 +3442,7 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
  pr_devel("%s: All FETCH_REQs received, dev id %d\n", __func__,
  header->dev_id);

- if (ub->ublksrv_tgid != ublksrv_pid)
+ if (ub->ublksrv_tgid != current->tgid)
  return -EINVAL;

  mutex_lock(&ub->mutex);
@@ -3437,10 +3453,10 @@ static int ublk_ctrl_end_recovery(struct
ublk_device *ub,
  ret = -EBUSY;
  goto out_unlock;
  }
- ub->dev_info.ublksrv_pid = ublksrv_pid;
+ ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
  ub->dev_info.state = UBLK_S_DEV_LIVE;
  pr_devel("%s: new ublksrv_pid %d, dev id %d\n",
- __func__, ublksrv_pid, header->dev_id);
+ __func__, ub->ublksrv_tgid, header->dev_id);
  blk_mq_kick_requeue_list(ub->ub_disk->queue);
  ret = 0;
  out_unlock:
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-11  0:00 [PATCH] ublk: fix ublksrv pid handling for pid namespaces Seamus Connor
@ 2026-01-12  5:40 ` Ming Lei
  2026-01-12 18:09   ` Seamus Connor
  2026-01-12 22:56 ` [PATCH v2] " Seamus Connor
  2026-01-15  2:59 ` [PATCH v4] " Seamus Connor
  2 siblings, 1 reply; 15+ messages in thread
From: Ming Lei @ 2026-01-12  5:40 UTC (permalink / raw)
  To: Seamus Connor; +Cc: Jens Axboe, linux-block, Caleb Sander, linux-kernel

On Sat, Jan 10, 2026 at 04:00:15PM -0800, Seamus Connor wrote:
> When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
> the stored init-ns tgid against the userspace pid (getpid vnr), so the
> check failed and control ops could not proceed. Compare against the
> caller’s init-ns tgid and store that value, then translate it back to
> the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
> shows a sensible pid.
> 
> Testing: start/recover in a pid namespace; `ublk list` shows
> reasonable pid values in init, child, and sibling namespaces.
> 
> Fixes: d37a224fc119 ("ublk: validate ublk server pid")
> Signed-off-by: Seamus Connor <sconnor@purestorage.com>
> Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  drivers/block/ublk_drv.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 79847e0b9e88..9ef6432fef7c 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2858,7 +2858,6 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>   const struct ublksrv_ctrl_cmd *header)
>  {
>   const struct ublk_param_basic *p = &ub->params.basic;
> - int ublksrv_pid = (int)header->data[0];
>   struct queue_limits lim = {
>   .logical_block_size = 1 << p->logical_bs_shift,
>   .physical_block_size = 1 << p->physical_bs_shift,
> @@ -2874,8 +2873,6 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>   struct gendisk *disk;
>   int ret = -EINVAL;
> 
> - if (ublksrv_pid <= 0)
> - return -EINVAL;
>   if (!(ub->params.types & UBLK_PARAM_TYPE_BASIC))
>   return -EINVAL;
> 
> @@ -2922,7 +2919,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>   if (wait_for_completion_interruptible(&ub->completion) != 0)
>   return -EINTR;
> 
> - if (ub->ublksrv_tgid != ublksrv_pid)
> + if (ub->ublksrv_tgid != current->tgid)

This way requires that START_DEV command can only be submitted from ublk server
daemon context, which may break implementation sending `START_DEV` command
from remote process context.

Can we fix it in the following way?

+       struct pid *pid = find_vpid(ublksrv_pid);
+
+       if (!pid || pid_nr(pid) != ub->ublksrv_tgid)
+               return -EINVAL;

Also your patch has patch style issue, please check it before posting out
by `./scripts/checkpatch.pl`. Or you may have to use `git send-email` to
send patch file.


Thanks,
Ming


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-12  5:40 ` Ming Lei
@ 2026-01-12 18:09   ` Seamus Connor
  0 siblings, 0 replies; 15+ messages in thread
From: Seamus Connor @ 2026-01-12 18:09 UTC (permalink / raw)
  To: Ming Lei; +Cc: Jens Axboe, linux-block, Caleb Sander, linux-kernel

> Can we fix it in the following way?
>
> +       struct pid *pid = find_vpid(ublksrv_pid);
> +
> +       if (!pid || pid_nr(pid) != ub->ublksrv_tgid)
> +               return -EINVAL;

Sure that makes sense. Let me try that out.

> Also your patch has patch style issue, please check it before posting out
> by `./scripts/checkpatch.pl`. Or you may have to use `git send-email` to
> send patch file.

Sorry about that.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-11  0:00 [PATCH] ublk: fix ublksrv pid handling for pid namespaces Seamus Connor
  2026-01-12  5:40 ` Ming Lei
@ 2026-01-12 22:56 ` Seamus Connor
  2026-01-13  2:01   ` Ming Lei
  2026-01-14 20:47   ` [PATCH v3] " Seamus Connor
  2026-01-15  2:59 ` [PATCH v4] " Seamus Connor
  2 siblings, 2 replies; 15+ messages in thread
From: Seamus Connor @ 2026-01-12 22:56 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block; +Cc: Seamus Connor, Caleb Sander Mateos

When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
the stored init-ns tgid against the userspace pid (getpid vnr), so the
check failed and control ops could not proceed. Compare against the
caller’s init-ns tgid and store that value, then translate it back to
the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
shows a sensible pid.

Testing: start/recover in a pid namespace; `ublk list` shows
reasonable pid values in init, child, and sibling namespaces.

Fixes: d37a224fc119 ("ublk: validate ublk server pid")
Signed-off-by: Seamus Connor <sconnor@purestorage.com>
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
---
Changes since v1:
 - Updated start_dev and end_recovery to respect the user-supplied pid

 drivers/block/ublk_drv.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 79847e0b9e88..4a4673e64668 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2922,6 +2922,10 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 	if (wait_for_completion_interruptible(&ub->completion) != 0)
 		return -EINTR;
 
+	rcu_read_lock();
+	ublksrv_pid = pid_nr(find_vpid(ublksrv_pid));
+	rcu_read_unlock();
+
 	if (ub->ublksrv_tgid != ublksrv_pid)
 		return -EINVAL;
 
@@ -3276,12 +3280,32 @@ static int ublk_ctrl_stop_dev(struct ublk_device *ub)
 static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
 		const struct ublksrv_ctrl_cmd *header)
 {
+	struct task_struct *p;
+	struct pid *pid;
+	struct ublksrv_ctrl_dev_info dev_info;
+	pid_t init_ublksrv_tgid = ub->dev_info.ublksrv_pid;
 	void __user *argp = (void __user *)(unsigned long)header->addr;
 
 	if (header->len < sizeof(struct ublksrv_ctrl_dev_info) || !header->addr)
 		return -EINVAL;
 
-	if (copy_to_user(argp, &ub->dev_info, sizeof(ub->dev_info)))
+	memcpy(&dev_info, &ub->dev_info, sizeof(dev_info));
+	dev_info.ublksrv_pid = -1;
+
+	if (init_ublksrv_tgid > 0) {
+		rcu_read_lock();
+		pid = find_pid_ns(init_ublksrv_tgid, &init_pid_ns);
+		p = pid_task(pid, PIDTYPE_TGID);
+		if (p) {
+			int vnr = task_tgid_vnr(p);
+
+			if (vnr)
+				dev_info.ublksrv_pid = vnr;
+		}
+		rcu_read_unlock();
+	}
+
+	if (copy_to_user(argp, &dev_info, sizeof(dev_info)))
 		return -EFAULT;
 
 	return 0;
@@ -3426,6 +3450,10 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 	pr_devel("%s: All FETCH_REQs received, dev id %d\n", __func__,
 		 header->dev_id);
 
+	rcu_read_lock();
+	ublksrv_pid = pid_nr(find_vpid(ublksrv_pid));
+	rcu_read_unlock();
+
 	if (ub->ublksrv_tgid != ublksrv_pid)
 		return -EINVAL;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-12 22:56 ` [PATCH v2] " Seamus Connor
@ 2026-01-13  2:01   ` Ming Lei
  2026-01-13  2:46     ` Seamus Connor
  2026-01-14 20:47   ` [PATCH v3] " Seamus Connor
  1 sibling, 1 reply; 15+ messages in thread
From: Ming Lei @ 2026-01-13  2:01 UTC (permalink / raw)
  To: Seamus Connor; +Cc: Jens Axboe, linux-block, Caleb Sander Mateos

On Mon, Jan 12, 2026 at 02:56:14PM -0800, Seamus Connor wrote:
> When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
> the stored init-ns tgid against the userspace pid (getpid vnr), so the
> check failed and control ops could not proceed. Compare against the
> caller’s init-ns tgid and store that value, then translate it back to
> the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
> shows a sensible pid.
> 
> Testing: start/recover in a pid namespace; `ublk list` shows
> reasonable pid values in init, child, and sibling namespaces.
> 
> Fixes: d37a224fc119 ("ublk: validate ublk server pid")
> Signed-off-by: Seamus Connor <sconnor@purestorage.com>
> Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
> Changes since v1:
>  - Updated start_dev and end_recovery to respect the user-supplied pid
> 
>  drivers/block/ublk_drv.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 79847e0b9e88..4a4673e64668 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2922,6 +2922,10 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>  	if (wait_for_completion_interruptible(&ub->completion) != 0)
>  		return -EINTR;
>  
> +	rcu_read_lock();
> +	ublksrv_pid = pid_nr(find_vpid(ublksrv_pid));
> +	rcu_read_unlock();
> +

`ublksrv_pid` is from userspace, so it may be invalid, then you may have to
check result of find_vpid().

>  	if (ub->ublksrv_tgid != ublksrv_pid)
>  		return -EINVAL;

Please add one helper of ublk_validate_ublksrv_pid() by moving all above
change into the helper, then two code paths can use the single helper.


Otherwise, this patch looks fine.

Thanks, 
Ming


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-13  2:01   ` Ming Lei
@ 2026-01-13  2:46     ` Seamus Connor
  2026-01-13  3:47       ` Ming Lei
  0 siblings, 1 reply; 15+ messages in thread
From: Seamus Connor @ 2026-01-13  2:46 UTC (permalink / raw)
  To: Ming Lei; +Cc: Jens Axboe, linux-block, Caleb Sander Mateos

> `ublksrv_pid` is from userspace, so it may be invalid, then you may have to
> check result of find_vpid().

find_vpid() returns either a valid struct pid* or NULL as far as I
understand, and pid_nr handles the case where the provided struct pid*
is NULL. Is there another case to handle that I am missing?

Thanks,
Seamus

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-13  2:46     ` Seamus Connor
@ 2026-01-13  3:47       ` Ming Lei
  2026-01-13 23:03         ` Seamus Connor
  0 siblings, 1 reply; 15+ messages in thread
From: Ming Lei @ 2026-01-13  3:47 UTC (permalink / raw)
  To: Seamus Connor; +Cc: Jens Axboe, linux-block, Caleb Sander Mateos

On Mon, Jan 12, 2026 at 06:46:06PM -0800, Seamus Connor wrote:
> > `ublksrv_pid` is from userspace, so it may be invalid, then you may have to
> > check result of find_vpid().
> 
> find_vpid() returns either a valid struct pid* or NULL as far as I
> understand, and pid_nr handles the case where the provided struct pid*
> is NULL. Is there another case to handle that I am missing?

pid_nr(NULL) returns 0, but the stored ->ublksrv_pid can't be zero, so this
bad condition is always covered?  If yes, looks it is fine to not check
NULL `pid*` explicitly.


Thanks,
Ming


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-13  3:47       ` Ming Lei
@ 2026-01-13 23:03         ` Seamus Connor
  2026-01-14  3:55           ` Ming Lei
  0 siblings, 1 reply; 15+ messages in thread
From: Seamus Connor @ 2026-01-13 23:03 UTC (permalink / raw)
  To: Ming Lei; +Cc: Jens Axboe, linux-block, Caleb Sander Mateos

Hi Ming,

I did the following test. I updated kublk.c so that getpid() could be
overridden with arbitrary values. I then added probes around the code
change. I tested the behavior of the change with arbitrary negative,
and positive pids, confirming that I covered both pids that do exist,
and pids that do not exist. The behavior of
`pid_nr(find_vpid(ublksrv_pid));` is correct under these
circumstances.

Of course, I am happy to add explicit checks, move to a helper, or add
the tests I mentioned to the suite. Let me know.

Thank you!

-Seamus

On Mon, Jan 12, 2026 at 7:48 PM Ming Lei <ming.lei@redhat.com> wrote:
>
> On Mon, Jan 12, 2026 at 06:46:06PM -0800, Seamus Connor wrote:
> > > `ublksrv_pid` is from userspace, so it may be invalid, then you may have to
> > > check result of find_vpid().
> >
> > find_vpid() returns either a valid struct pid* or NULL as far as I
> > understand, and pid_nr handles the case where the provided struct pid*
> > is NULL. Is there another case to handle that I am missing?
>
> pid_nr(NULL) returns 0, but the stored ->ublksrv_pid can't be zero, so this
> bad condition is always covered?  If yes, looks it is fine to not check
> NULL `pid*` explicitly.
>
>
> Thanks,
> Ming
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-13 23:03         ` Seamus Connor
@ 2026-01-14  3:55           ` Ming Lei
  0 siblings, 0 replies; 15+ messages in thread
From: Ming Lei @ 2026-01-14  3:55 UTC (permalink / raw)
  To: Seamus Connor; +Cc: Jens Axboe, linux-block, Caleb Sander Mateos

On Tue, Jan 13, 2026 at 03:03:04PM -0800, Seamus Connor wrote:
> Hi Ming,
> 
> I did the following test. I updated kublk.c so that getpid() could be
> overridden with arbitrary values. I then added probes around the code
> change. I tested the behavior of the change with arbitrary negative,
> and positive pids, confirming that I covered both pids that do exist,
> and pids that do not exist. The behavior of
> `pid_nr(find_vpid(ublksrv_pid));` is correct under these
> circumstances.
> 
> Of course, I am happy to add explicit checks, move to a helper, or add
> the tests I mentioned to the suite. Let me know.

Hi Seamus,

Please go ahead and post V3.

Thanks,
Ming


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-12 22:56 ` [PATCH v2] " Seamus Connor
  2026-01-13  2:01   ` Ming Lei
@ 2026-01-14 20:47   ` Seamus Connor
  2026-01-15  1:48     ` Ming Lei
  1 sibling, 1 reply; 15+ messages in thread
From: Seamus Connor @ 2026-01-14 20:47 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block; +Cc: Seamus Connor, Caleb Sander Mateos

When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
the stored init-ns tgid against the userspace pid (getpid vnr), so the
check failed and control ops could not proceed. Compare against the
caller’s init-ns tgid and store that value, then translate it back to
the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
shows a sensible pid.

Testing: start/recover in a pid namespace; `ublk list` shows
reasonable pid values in init, child, and sibling namespaces.

Fixes: d37a224fc119 ("ublk: validate ublk server pid")
Signed-off-by: Seamus Connor <sconnor@purestorage.com>
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
---
Changes since v1:
 - Updated start_dev and end_recovery to respect the user-supplied pid

Changes since v2:
 - Moved pid translation into a helper function

 drivers/block/ublk_drv.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 67d4a867aec4..01747d256ff5 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2852,6 +2852,15 @@ static struct ublk_device *ublk_get_device_from_id(int idx)
 	return ub;
 }
 
+static pid_t ublk_translate_user_pid(pid_t ublksrv_pid)
+{
+	rcu_read_lock();
+	ublksrv_pid = pid_nr(find_vpid(ublksrv_pid));
+	rcu_read_unlock();
+
+	return ublksrv_pid;
+}
+
 static int ublk_ctrl_start_dev(struct ublk_device *ub,
 		const struct ublksrv_ctrl_cmd *header)
 {
@@ -2920,6 +2929,8 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 	if (wait_for_completion_interruptible(&ub->completion) != 0)
 		return -EINTR;
 
+	ublksrv_pid = ublk_translate_user_pid(ublksrv_pid);
+
 	if (ub->ublksrv_tgid != ublksrv_pid)
 		return -EINVAL;
 
@@ -3274,12 +3285,32 @@ static int ublk_ctrl_stop_dev(struct ublk_device *ub)
 static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
 		const struct ublksrv_ctrl_cmd *header)
 {
+	struct task_struct *p;
+	struct pid *pid;
+	struct ublksrv_ctrl_dev_info dev_info;
+	pid_t init_ublksrv_tgid = ub->dev_info.ublksrv_pid;
 	void __user *argp = (void __user *)(unsigned long)header->addr;
 
 	if (header->len < sizeof(struct ublksrv_ctrl_dev_info) || !header->addr)
 		return -EINVAL;
 
-	if (copy_to_user(argp, &ub->dev_info, sizeof(ub->dev_info)))
+	memcpy(&dev_info, &ub->dev_info, sizeof(dev_info));
+	dev_info.ublksrv_pid = -1;
+
+	if (init_ublksrv_tgid > 0) {
+		rcu_read_lock();
+		pid = find_pid_ns(init_ublksrv_tgid, &init_pid_ns);
+		p = pid_task(pid, PIDTYPE_TGID);
+		if (p) {
+			int vnr = task_tgid_vnr(p);
+
+			if (vnr)
+				dev_info.ublksrv_pid = vnr;
+		}
+		rcu_read_unlock();
+	}
+
+	if (copy_to_user(argp, &dev_info, sizeof(dev_info)))
 		return -EFAULT;
 
 	return 0;
@@ -3424,6 +3455,8 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 	pr_devel("%s: All FETCH_REQs received, dev id %d\n", __func__,
 		 header->dev_id);
 
+	ublksrv_pid = ublk_translate_user_pid(ublksrv_pid);
+
 	if (ub->ublksrv_tgid != ublksrv_pid)
 		return -EINVAL;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-14 20:47   ` [PATCH v3] " Seamus Connor
@ 2026-01-15  1:48     ` Ming Lei
  0 siblings, 0 replies; 15+ messages in thread
From: Ming Lei @ 2026-01-15  1:48 UTC (permalink / raw)
  To: Seamus Connor; +Cc: Jens Axboe, linux-block, Caleb Sander Mateos

On Wed, Jan 14, 2026 at 12:47:04PM -0800, Seamus Connor wrote:
> When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
> the stored init-ns tgid against the userspace pid (getpid vnr), so the
> check failed and control ops could not proceed. Compare against the
> caller’s init-ns tgid and store that value, then translate it back to
> the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
> shows a sensible pid.
> 
> Testing: start/recover in a pid namespace; `ublk list` shows
> reasonable pid values in init, child, and sibling namespaces.
> 
> Fixes: d37a224fc119 ("ublk: validate ublk server pid")
> Signed-off-by: Seamus Connor <sconnor@purestorage.com>
> Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
> Changes since v1:
>  - Updated start_dev and end_recovery to respect the user-supplied pid
> 
> Changes since v2:
>  - Moved pid translation into a helper function
> 
>  drivers/block/ublk_drv.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 67d4a867aec4..01747d256ff5 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2852,6 +2852,15 @@ static struct ublk_device *ublk_get_device_from_id(int idx)
>  	return ub;
>  }
>  
> +static pid_t ublk_translate_user_pid(pid_t ublksrv_pid)
> +{
> +	rcu_read_lock();
> +	ublksrv_pid = pid_nr(find_vpid(ublksrv_pid));
> +	rcu_read_unlock();
> +
> +	return ublksrv_pid;
> +}
> +
>  static int ublk_ctrl_start_dev(struct ublk_device *ub,
>  		const struct ublksrv_ctrl_cmd *header)
>  {
> @@ -2920,6 +2929,8 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>  	if (wait_for_completion_interruptible(&ub->completion) != 0)
>  		return -EINTR;
>  
> +	ublksrv_pid = ublk_translate_user_pid(ublksrv_pid);
> +
>  	if (ub->ublksrv_tgid != ublksrv_pid)
>  		return -EINVAL;

The above two lines can be moved to the helper which can be renamed as ublk_validate_user_pid(),
otherwise this patch looks good:

Reviewed-by: Ming Lei <ming.lei@redhat.com>


Thanks, 
Ming


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-11  0:00 [PATCH] ublk: fix ublksrv pid handling for pid namespaces Seamus Connor
  2026-01-12  5:40 ` Ming Lei
  2026-01-12 22:56 ` [PATCH v2] " Seamus Connor
@ 2026-01-15  2:59 ` Seamus Connor
  2026-01-20 23:48   ` Seamus Connor
                     ` (2 more replies)
  2 siblings, 3 replies; 15+ messages in thread
From: Seamus Connor @ 2026-01-15  2:59 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block; +Cc: Seamus Connor, Caleb Sander Mateos

When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
the stored init-ns tgid against the userspace pid (getpid vnr), so the
check failed and control ops could not proceed. Compare against the
caller’s init-ns tgid and store that value, then translate it back to
the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
shows a sensible pid.

Testing: start/recover in a pid namespace; `ublk list` shows
reasonable pid values in init, child, and sibling namespaces.

Fixes: d37a224fc119 ("ublk: validate ublk server pid")
Signed-off-by: Seamus Connor <sconnor@purestorage.com>
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
---
Changes since v1:
 - Updated start_dev and end_recovery to respect the user-supplied pid

Changes since v2:
 - Moved pid translation into a helper function

Changes since v3:
 - Minor rework to the helper function

 drivers/block/ublk_drv.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 67d4a867aec4..898c17755135 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2852,6 +2852,15 @@ static struct ublk_device *ublk_get_device_from_id(int idx)
 	return ub;
 }
 
+static bool ublk_validate_user_pid(struct ublk_device *ub, pid_t ublksrv_pid)
+{
+	rcu_read_lock();
+	ublksrv_pid = pid_nr(find_vpid(ublksrv_pid));
+	rcu_read_unlock();
+
+	return ub->ublksrv_tgid == ublksrv_pid;
+}
+
 static int ublk_ctrl_start_dev(struct ublk_device *ub,
 		const struct ublksrv_ctrl_cmd *header)
 {
@@ -2920,7 +2929,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 	if (wait_for_completion_interruptible(&ub->completion) != 0)
 		return -EINTR;
 
-	if (ub->ublksrv_tgid != ublksrv_pid)
+	if (!ublk_validate_user_pid(ub, ublksrv_pid))
 		return -EINVAL;
 
 	mutex_lock(&ub->mutex);
@@ -2939,7 +2948,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 	disk->fops = &ub_fops;
 	disk->private_data = ub;
 
-	ub->dev_info.ublksrv_pid = ublksrv_pid;
+	ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
 	ub->ub_disk = disk;
 
 	ublk_apply_params(ub);
@@ -3274,12 +3283,32 @@ static int ublk_ctrl_stop_dev(struct ublk_device *ub)
 static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
 		const struct ublksrv_ctrl_cmd *header)
 {
+	struct task_struct *p;
+	struct pid *pid;
+	struct ublksrv_ctrl_dev_info dev_info;
+	pid_t init_ublksrv_tgid = ub->dev_info.ublksrv_pid;
 	void __user *argp = (void __user *)(unsigned long)header->addr;
 
 	if (header->len < sizeof(struct ublksrv_ctrl_dev_info) || !header->addr)
 		return -EINVAL;
 
-	if (copy_to_user(argp, &ub->dev_info, sizeof(ub->dev_info)))
+	memcpy(&dev_info, &ub->dev_info, sizeof(dev_info));
+	dev_info.ublksrv_pid = -1;
+
+	if (init_ublksrv_tgid > 0) {
+		rcu_read_lock();
+		pid = find_pid_ns(init_ublksrv_tgid, &init_pid_ns);
+		p = pid_task(pid, PIDTYPE_TGID);
+		if (p) {
+			int vnr = task_tgid_vnr(p);
+
+			if (vnr)
+				dev_info.ublksrv_pid = vnr;
+		}
+		rcu_read_unlock();
+	}
+
+	if (copy_to_user(argp, &dev_info, sizeof(dev_info)))
 		return -EFAULT;
 
 	return 0;
@@ -3424,7 +3453,7 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 	pr_devel("%s: All FETCH_REQs received, dev id %d\n", __func__,
 		 header->dev_id);
 
-	if (ub->ublksrv_tgid != ublksrv_pid)
+	if (!ublk_validate_user_pid(ub, ublksrv_pid))
 		return -EINVAL;
 
 	mutex_lock(&ub->mutex);
@@ -3435,7 +3464,7 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 		ret = -EBUSY;
 		goto out_unlock;
 	}
-	ub->dev_info.ublksrv_pid = ublksrv_pid;
+	ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
 	ub->dev_info.state = UBLK_S_DEV_LIVE;
 	pr_devel("%s: new ublksrv_pid %d, dev id %d\n",
 			__func__, ublksrv_pid, header->dev_id);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-15  2:59 ` [PATCH v4] " Seamus Connor
@ 2026-01-20 23:48   ` Seamus Connor
  2026-01-21 14:45   ` Jens Axboe
  2026-01-21 14:48   ` Jens Axboe
  2 siblings, 0 replies; 15+ messages in thread
From: Seamus Connor @ 2026-01-20 23:48 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block; +Cc: Caleb Sander Mateos

Hi Ming and Jens,

As far as I know this patch is ready to be pulled. Let me know if any
further changes are needed.

Thanks,
Seamus

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-15  2:59 ` [PATCH v4] " Seamus Connor
  2026-01-20 23:48   ` Seamus Connor
@ 2026-01-21 14:45   ` Jens Axboe
  2026-01-21 14:48   ` Jens Axboe
  2 siblings, 0 replies; 15+ messages in thread
From: Jens Axboe @ 2026-01-21 14:45 UTC (permalink / raw)
  To: Seamus Connor, Ming Lei, linux-block; +Cc: Caleb Sander Mateos

On 1/14/26 7:59 PM, Seamus Connor wrote:
> When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
> the stored init-ns tgid against the userspace pid (getpid vnr), so the
> check failed and control ops could not proceed. Compare against the
> caller’s init-ns tgid and store that value, then translate it back to
> the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
> shows a sensible pid.
> 
> Testing: start/recover in a pid namespace; `ublk list` shows
> reasonable pid values in init, child, and sibling namespaces.
> 
> Fixes: d37a224fc119 ("ublk: validate ublk server pid")

Where is this sha from? Looks like this should be:

Fixes: c2c8089f325e ("ublk: validate ublk server pid")                          

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] ublk: fix ublksrv pid handling for pid namespaces
  2026-01-15  2:59 ` [PATCH v4] " Seamus Connor
  2026-01-20 23:48   ` Seamus Connor
  2026-01-21 14:45   ` Jens Axboe
@ 2026-01-21 14:48   ` Jens Axboe
  2 siblings, 0 replies; 15+ messages in thread
From: Jens Axboe @ 2026-01-21 14:48 UTC (permalink / raw)
  To: Ming Lei, linux-block, Seamus Connor; +Cc: Caleb Sander Mateos


On Wed, 14 Jan 2026 18:59:52 -0800, Seamus Connor wrote:
> When ublksrv runs inside a pid namespace, START/END_RECOVERY compared
> the stored init-ns tgid against the userspace pid (getpid vnr), so the
> check failed and control ops could not proceed. Compare against the
> caller’s init-ns tgid and store that value, then translate it back to
> the caller’s pid namespace when reporting GET_DEV_INFO so ublk list
> shows a sensible pid.
> 
> [...]

Applied, thanks!

[1/1] ublk: fix ublksrv pid handling for pid namespaces
      commit: 47bdf1d29caec7207b7f112230055db36602dfc0

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-01-21 14:48 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-11  0:00 [PATCH] ublk: fix ublksrv pid handling for pid namespaces Seamus Connor
2026-01-12  5:40 ` Ming Lei
2026-01-12 18:09   ` Seamus Connor
2026-01-12 22:56 ` [PATCH v2] " Seamus Connor
2026-01-13  2:01   ` Ming Lei
2026-01-13  2:46     ` Seamus Connor
2026-01-13  3:47       ` Ming Lei
2026-01-13 23:03         ` Seamus Connor
2026-01-14  3:55           ` Ming Lei
2026-01-14 20:47   ` [PATCH v3] " Seamus Connor
2026-01-15  1:48     ` Ming Lei
2026-01-15  2:59 ` [PATCH v4] " Seamus Connor
2026-01-20 23:48   ` Seamus Connor
2026-01-21 14:45   ` Jens Axboe
2026-01-21 14:48   ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox