* [PATCH 1/2] nvme: fix memory corruption for passthrough metadata [not found] ` <20230811155906.15883-1-joshi.k@samsung.com> @ 2023-08-11 15:59 ` Kanchan Joshi 2023-08-11 16:57 ` Keith Busch 0 siblings, 1 reply; 3+ messages in thread From: Kanchan Joshi @ 2023-08-11 15:59 UTC (permalink / raw) To: hch, kbusch, axboe, sagi Cc: linux-nvme, vincentfu, ankit.kumar, joshiiitr, gost.dev, Kanchan Joshi, stable, Vincent Fu User can specify a smaller meta buffer than what the device is wired to update/access. This may lead to Device doing a larger DMA operation, overwriting unrelated kernel memory. Detect this situation for uring passthrough, and bail out. Fixes: 456cba386e94 ("nvme: wire-up uring-cmd support for io-passthru on char-device") Cc: stable@vger.kernel.org Reported-by: Vincent Fu <vincent.fu@samsung.com> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> --- drivers/nvme/host/ioctl.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 19a5177bc360..fb73fa95f090 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c @@ -320,6 +320,30 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio) meta_len, lower_32_bits(io.slba), NULL, 0, 0); } +static bool nvme_validate_passthru_meta(struct nvme_ctrl *ctrl, + struct nvme_ns *ns, + struct nvme_command *c, + __u64 meta, __u32 meta_len) +{ + /* + * User may specify smaller meta-buffer with a larger data-buffer. + * Driver allocated meta buffer will also be small. + * Device can do larger dma into that, overwriting unrelated kernel + * memory. + */ + if (ns && (meta_len || meta)) { + u16 nlb = lower_16_bits(le32_to_cpu(c->common.cdw12)); + + if (meta_len != (nlb + 1) * ns->ms) { + dev_err(ctrl->device, + "%s: metadata length does not match!\n", current->comm); + return false; + } + } + + return true; +} + static bool nvme_validate_passthru_nsid(struct nvme_ctrl *ctrl, struct nvme_ns *ns, __u32 nsid) { @@ -593,6 +617,10 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns, d.metadata_len = READ_ONCE(cmd->metadata_len); d.timeout_ms = READ_ONCE(cmd->timeout_ms); + if (!nvme_validate_passthru_meta(ctrl, ns, &c, d.metadata, + d.metadata_len)) + return -EINVAL; + if (issue_flags & IO_URING_F_NONBLOCK) { rq_flags |= REQ_NOWAIT; blk_flags = BLK_MQ_REQ_NOWAIT; -- 2.25.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] nvme: fix memory corruption for passthrough metadata 2023-08-11 15:59 ` [PATCH 1/2] nvme: fix memory corruption for passthrough metadata Kanchan Joshi @ 2023-08-11 16:57 ` Keith Busch 2023-08-14 6:41 ` Kanchan Joshi 0 siblings, 1 reply; 3+ messages in thread From: Keith Busch @ 2023-08-11 16:57 UTC (permalink / raw) To: Kanchan Joshi Cc: hch, axboe, sagi, linux-nvme, vincentfu, ankit.kumar, joshiiitr, gost.dev, stable, Vincent Fu On Fri, Aug 11, 2023 at 09:29:05PM +0530, Kanchan Joshi wrote: > +static bool nvme_validate_passthru_meta(struct nvme_ctrl *ctrl, > + struct nvme_ns *ns, > + struct nvme_command *c, > + __u64 meta, __u32 meta_len) > +{ > + /* > + * User may specify smaller meta-buffer with a larger data-buffer. > + * Driver allocated meta buffer will also be small. > + * Device can do larger dma into that, overwriting unrelated kernel > + * memory. > + */ > + if (ns && (meta_len || meta)) { > + u16 nlb = lower_16_bits(le32_to_cpu(c->common.cdw12)); > + > + if (meta_len != (nlb + 1) * ns->ms) { > + dev_err(ctrl->device, > + "%s: metadata length does not match!\n", current->comm); > + return false; > + } Don't you need to check the command PRINFO PRACT bit to know if metadata length is striped/generated on the controller side? ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] nvme: fix memory corruption for passthrough metadata 2023-08-11 16:57 ` Keith Busch @ 2023-08-14 6:41 ` Kanchan Joshi 0 siblings, 0 replies; 3+ messages in thread From: Kanchan Joshi @ 2023-08-14 6:41 UTC (permalink / raw) To: Keith Busch Cc: hch, axboe, sagi, linux-nvme, vincentfu, ankit.kumar, joshiiitr, gost.dev, stable, Vincent Fu [-- Attachment #1: Type: text/plain, Size: 971 bytes --] On Fri, Aug 11, 2023 at 10:57:36AM -0600, Keith Busch wrote: >On Fri, Aug 11, 2023 at 09:29:05PM +0530, Kanchan Joshi wrote: >> +static bool nvme_validate_passthru_meta(struct nvme_ctrl *ctrl, >> + struct nvme_ns *ns, >> + struct nvme_command *c, >> + __u64 meta, __u32 meta_len) >> +{ >> + /* >> + * User may specify smaller meta-buffer with a larger data-buffer. >> + * Driver allocated meta buffer will also be small. >> + * Device can do larger dma into that, overwriting unrelated kernel >> + * memory. >> + */ >> + if (ns && (meta_len || meta)) { >> + u16 nlb = lower_16_bits(le32_to_cpu(c->common.cdw12)); >> + >> + if (meta_len != (nlb + 1) * ns->ms) { >> + dev_err(ctrl->device, >> + "%s: metadata length does not match!\n", current->comm); >> + return false; >> + } > >Don't you need to check the command PRINFO PRACT bit to know if metadata >length is striped/generated on the controller side? Good point. Will add that check in v2. [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-14 6:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20230811160454epcas5p2635d208557749a2431b99c27b30a727f@epcas5p2.samsung.com>
[not found] ` <20230811155906.15883-1-joshi.k@samsung.com>
2023-08-11 15:59 ` [PATCH 1/2] nvme: fix memory corruption for passthrough metadata Kanchan Joshi
2023-08-11 16:57 ` Keith Busch
2023-08-14 6:41 ` Kanchan Joshi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).