All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5.10 1/2] nvme: restrict management ioctls to admin
@ 2022-11-22 10:22 Ovidiu Panait
  2022-11-22 10:22 ` [PATCH 5.10 2/2] nvme: ensure subsystem reset is single threaded Ovidiu Panait
  2022-11-24 14:20 ` [PATCH 5.10 1/2] nvme: restrict management ioctls to admin Harshit Mogalapalli
  0 siblings, 2 replies; 3+ messages in thread
From: Ovidiu Panait @ 2022-11-22 10:22 UTC (permalink / raw)
  To: stable; +Cc: Keith Busch, Christoph Hellwig, Ovidiu Panait

From: Keith Busch <kbusch@kernel.org>

commit 23e085b2dead13b51fe86d27069895b740f749c0 upstream.

The passthrough commands already have this restriction, but the other
operations do not. Require the same capabilities for all users as all of
these operations, which include resets and rescans, can be disruptive.

Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
These backports are for CVE-2022-3169.

 drivers/nvme/host/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 3f106771d15b..d9c78fe85cb3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3330,11 +3330,17 @@ static long nvme_dev_ioctl(struct file *file, unsigned int cmd,
 	case NVME_IOCTL_IO_CMD:
 		return nvme_dev_user_cmd(ctrl, argp);
 	case NVME_IOCTL_RESET:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EACCES;
 		dev_warn(ctrl->device, "resetting controller\n");
 		return nvme_reset_ctrl_sync(ctrl);
 	case NVME_IOCTL_SUBSYS_RESET:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EACCES;
 		return nvme_reset_subsystem(ctrl);
 	case NVME_IOCTL_RESCAN:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EACCES;
 		nvme_queue_scan(ctrl);
 		return 0;
 	default:
-- 
2.38.1


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

* [PATCH 5.10 2/2] nvme: ensure subsystem reset is single threaded
  2022-11-22 10:22 [PATCH 5.10 1/2] nvme: restrict management ioctls to admin Ovidiu Panait
@ 2022-11-22 10:22 ` Ovidiu Panait
  2022-11-24 14:20 ` [PATCH 5.10 1/2] nvme: restrict management ioctls to admin Harshit Mogalapalli
  1 sibling, 0 replies; 3+ messages in thread
From: Ovidiu Panait @ 2022-11-22 10:22 UTC (permalink / raw)
  To: stable; +Cc: Keith Busch, Christoph Hellwig, Ovidiu Panait

From: Keith Busch <kbusch@kernel.org>

commit 1e866afd4bcdd01a70a5eddb4371158d3035ce03 upstream.

The subsystem reset writes to a register, so we have to ensure the
device state is capable of handling that otherwise the driver may access
unmapped registers. Use the state machine to ensure the subsystem reset
doesn't try to write registers on a device already undergoing this type
of reset.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=214771
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 drivers/nvme/host/nvme.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index abae7ef2ac51..86336496c65c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -544,11 +544,23 @@ static inline void nvme_fault_inject_fini(struct nvme_fault_inject *fault_inj)
 static inline void nvme_should_fail(struct request *req) {}
 #endif
 
+bool nvme_wait_reset(struct nvme_ctrl *ctrl);
+int nvme_try_sched_reset(struct nvme_ctrl *ctrl);
+
 static inline int nvme_reset_subsystem(struct nvme_ctrl *ctrl)
 {
+	int ret;
+
 	if (!ctrl->subsystem)
 		return -ENOTTY;
-	return ctrl->ops->reg_write32(ctrl, NVME_REG_NSSR, 0x4E564D65);
+	if (!nvme_wait_reset(ctrl))
+		return -EBUSY;
+
+	ret = ctrl->ops->reg_write32(ctrl, NVME_REG_NSSR, 0x4E564D65);
+	if (ret)
+		return ret;
+
+	return nvme_try_sched_reset(ctrl);
 }
 
 /*
@@ -635,7 +647,6 @@ void nvme_cancel_tagset(struct nvme_ctrl *ctrl);
 void nvme_cancel_admin_tagset(struct nvme_ctrl *ctrl);
 bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 		enum nvme_ctrl_state new_state);
-bool nvme_wait_reset(struct nvme_ctrl *ctrl);
 int nvme_disable_ctrl(struct nvme_ctrl *ctrl);
 int nvme_enable_ctrl(struct nvme_ctrl *ctrl);
 int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl);
@@ -688,7 +699,6 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count);
 void nvme_stop_keep_alive(struct nvme_ctrl *ctrl);
 int nvme_reset_ctrl(struct nvme_ctrl *ctrl);
 int nvme_reset_ctrl_sync(struct nvme_ctrl *ctrl);
-int nvme_try_sched_reset(struct nvme_ctrl *ctrl);
 int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
 
 int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
-- 
2.38.1


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

* Re: [PATCH 5.10 1/2] nvme: restrict management ioctls to admin
  2022-11-22 10:22 [PATCH 5.10 1/2] nvme: restrict management ioctls to admin Ovidiu Panait
  2022-11-22 10:22 ` [PATCH 5.10 2/2] nvme: ensure subsystem reset is single threaded Ovidiu Panait
@ 2022-11-24 14:20 ` Harshit Mogalapalli
  1 sibling, 0 replies; 3+ messages in thread
From: Harshit Mogalapalli @ 2022-11-24 14:20 UTC (permalink / raw)
  To: Ovidiu Panait; +Cc: Keith Busch, Christoph Hellwig, stable


Hi Ovidiu,

On 22/11/22 3:52 pm, Ovidiu Panait wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> commit 23e085b2dead13b51fe86d27069895b740f749c0 upstream.
> 
> The passthrough commands already have this restriction, but the other
> operations do not. Require the same capabilities for all users as all of
> these operations, which include resets and rescans, can be disruptive.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
> ---
> These backports are for CVE-2022-3169.
> 
>   drivers/nvme/host/core.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 3f106771d15b..d9c78fe85cb3 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3330,11 +3330,17 @@ static long nvme_dev_ioctl(struct file *file, unsigned int cmd,
>   	case NVME_IOCTL_IO_CMD:
>   		return nvme_dev_user_cmd(ctrl, argp);
>   	case NVME_IOCTL_RESET:
> +		if (!capable(CAP_SYS_ADMIN))
> +			return -EACCES;
>   		dev_warn(ctrl->device, "resetting controller\n");
>   		return nvme_reset_ctrl_sync(ctrl);
>   	case NVME_IOCTL_SUBSYS_RESET:
> +		if (!capable(CAP_SYS_ADMIN))
> +			return -EACCES;
>   		return nvme_reset_subsystem(ctrl);
>   	case NVME_IOCTL_RESCAN:
> +		if (!capable(CAP_SYS_ADMIN))
> +			return -EACCES;
>   		nvme_queue_scan(ctrl);
>   		return 0;
>   	default:

Should we apply this patch(1/2) to other stable releases as well, i.e. 
5.4, 4.19, 4.14 ?

This first patch applies cleanly there as well(5.4,4.19,4.14).

Thanks,
Harshit

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

end of thread, other threads:[~2022-11-24 14:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-22 10:22 [PATCH 5.10 1/2] nvme: restrict management ioctls to admin Ovidiu Panait
2022-11-22 10:22 ` [PATCH 5.10 2/2] nvme: ensure subsystem reset is single threaded Ovidiu Panait
2022-11-24 14:20 ` [PATCH 5.10 1/2] nvme: restrict management ioctls to admin Harshit Mogalapalli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.