1. Patchew
  2. linux
  3. View series

[PATCH] ublk: fix ublksrv pid handling for pid namespaces

Seamus Connor posted 1 patch 4 weeks, 1 day ago 
drivers/block/ublk_drv.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
[PATCH] ublk: fix ublksrv pid handling for pid namespaces
Posted by Seamus Connor 4 weeks, 1 day ago 
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
Re: [PATCH] ublk: fix ublksrv pid handling for pid namespaces
Posted by Ming Lei 4 weeks ago 
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
Re: [PATCH] ublk: fix ublksrv pid handling for pid namespaces
Posted by Seamus Connor 4 weeks ago 
> 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.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.