[PATCH] ublk: don't allow user copy for unprivileged device

[PATCH] ublk: don't allow user copy for unprivileged device

[Ming Lei]

UBLK_F_USER_COPY requires userspace to call write() on ublk char
device for filling request buffer, and unprivileged device can't
be trusted.

So don't allow user copy for unprivileged device.

Fixes: 1172d5b8beca ("ublk: support user copy")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c      | 11 ++++++++++-
 include/uapi/linux/ublk_cmd.h |  8 +++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index cd509126e152..f812cd271573 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2519,10 +2519,19 @@ static int ublk_ctrl_add_dev(struct io_uring_cmd *cmd)
 	 * TODO: provide forward progress for RECOVERY handler, so that
 	 * unprivileged device can benefit from it
 	 */
-	if (info.flags & UBLK_F_UNPRIVILEGED_DEV)
+	if (info.flags & UBLK_F_UNPRIVILEGED_DEV) {
 		info.flags &= ~(UBLK_F_USER_RECOVERY_REISSUE |
 				UBLK_F_USER_RECOVERY);
 
+		/*
+		 * For USER_COPY, we depends on userspace to fill request
+		 * buffer by pwrite() to ublk char device, which can't be
+		 * used for unprivileged device
+		 */
+		if (info.flags & UBLK_F_USER_COPY)
+			return -EINVAL;
+	}
+
 	/* the created device is always owned by current user */
 	ublk_store_owner_uid_gid(&info.owner_uid, &info.owner_gid);
 
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index 897ace0794c2..cbe53c980cbc 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -174,7 +174,13 @@
 /* use ioctl encoding for uring command */
 #define UBLK_F_CMD_IOCTL_ENCODE	(1UL << 6)
 
-/* Copy between request and user buffer by pread()/pwrite() */
+/*
+ *  Copy between request and user buffer by pread()/pwrite()
+ *
+ *  Not available for UBLK_F_UNPRIVILEGED_DEV, otherwise userspace may
+ *  deceive us by not filling request buffer, then kernel uninitialized
+ *  data may be leaked.
+ */
 #define UBLK_F_USER_COPY	(1UL << 7)
 
 /*
-- 
2.46.0

Re: [PATCH] ublk: don't allow user copy for unprivileged device

[Jens Axboe]

On Wed, 16 Oct 2024 21:48:47 +0800, Ming Lei wrote:
> UBLK_F_USER_COPY requires userspace to call write() on ublk char
> device for filling request buffer, and unprivileged device can't
> be trusted.
> 
> So don't allow user copy for unprivileged device.
> 
> 
> [...]

Applied, thanks!

[1/1] ublk: don't allow user copy for unprivileged device
      commit: 42aafd8b48adac1c3b20fe5892b1b91b80c1a1e6

Best regards,
-- 
Jens Axboe

Re: [PATCH] ublk: don't allow user copy for unprivileged device

[Jens Axboe]

On 10/16/24 7:48 AM, Ming Lei wrote:
> UBLK_F_USER_COPY requires userspace to call write() on ublk char
> device for filling request buffer, and unprivileged device can't
> be trusted.
> 
> So don't allow user copy for unprivileged device.
> 
> Fixes: 1172d5b8beca ("ublk: support user copy")

I marked this one for stable as well.

-- 
Jens Axboe