Commit c95b708d authored by Nick Bowler's avatar Nick Bowler Committed by Christoph Hellwig
Browse files

nvme: fix compat address handling in several ioctls 



On a 32-bit kernel, the upper bits of userspace addresses passed via
various ioctls are silently ignored by the nvme driver.

However on a 64-bit kernel running a compat task, these upper bits are
not ignored and are in fact required to be zero for the ioctls to work.

Unfortunately, this difference matters.  32-bit smartctl submits the
NVME_IOCTL_ADMIN_CMD ioctl with garbage in these upper bits because it
seems the pointer value it puts into the nvme_passthru_cmd structure is
sign extended.  This works fine on 32-bit kernels but fails on a 64-bit
one because (at least on my setup) the addresses smartctl uses are
consistently above 2G.  For example:

  # smartctl -x /dev/nvme0n1
  smartctl 7.1 2019-12-30 r5022 [x86_64-linux-5.5.11] (local build)
  Copyright (C) 2002-19, Bruce Allen, Christian Franke, www.smartmontools.org

  Read NVMe Identify Controller failed: NVME_IOCTL_ADMIN_CMD: Bad address

Since changing 32-bit kernels to actually check all of the submitted
address bits now would break existing userspace, this patch fixes the
compat problem by explicitly zeroing the upper bits in the compat case.
This enables 32-bit smartctl to work on a 64-bit kernel.

Signed-off-by: default avatarNick Bowler <nbowler@draconx.ca>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
parent 458ef2a2

drivers/nvme/host/core.c

+20 −7
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ 


#include <linux/blkdev.h>

#include <linux/blk-mq.h>

#include <linux/compat.h>

#include <linux/delay.h>

#include <linux/errno.h>

#include <linux/hdreg.h>
@@ -1252,6 +1253,18 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl) 
	queue_work(nvme_wq, &ctrl->async_event_work);

}



/*

 * Convert integer values from ioctl structures to user pointers, silently

 * ignoring the upper bits in the compat case to match behaviour of 32-bit

 * kernels.

 */

static void __user *nvme_to_user_ptr(uintptr_t ptrval)

{

	if (in_compat_syscall())

		ptrval = (compat_uptr_t)ptrval;

	return (void __user *)ptrval;

}



static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)

{

	struct nvme_user_io io;
@@ -1275,7 +1288,7 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio) 


	length = (io.nblocks + 1) << ns->lba_shift;

	meta_len = (io.nblocks + 1) * ns->ms;

	metadata = (void __user *)(uintptr_t)io.metadata;

	metadata = nvme_to_user_ptr(io.metadata);



	if (ns->ext) {

		length += meta_len;
@@ -1298,7 +1311,7 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio) 
	c.rw.appmask = cpu_to_le16(io.appmask);



	return nvme_submit_user_cmd(ns->queue, &c,

			(void __user *)(uintptr_t)io.addr, length,

			nvme_to_user_ptr(io.addr), length,

			metadata, meta_len, lower_32_bits(io.slba), NULL, 0);

}
@@ -1418,9 +1431,9 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns, 


	effects = nvme_passthru_start(ctrl, ns, cmd.opcode);

	status = nvme_submit_user_cmd(ns ? ns->queue : ctrl->admin_q, &c,

			(void __user *)(uintptr_t)cmd.addr, cmd.data_len,

			(void __user *)(uintptr_t)cmd.metadata,

			cmd.metadata_len, 0, &result, timeout);

			nvme_to_user_ptr(cmd.addr), cmd.data_len,

			nvme_to_user_ptr(cmd.metadata), cmd.metadata_len,

			0, &result, timeout);

	nvme_passthru_end(ctrl, effects);



	if (status >= 0) {
@@ -1465,8 +1478,8 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns, 


	effects = nvme_passthru_start(ctrl, ns, cmd.opcode);

	status = nvme_submit_user_cmd(ns ? ns->queue : ctrl->admin_q, &c,

			(void __user *)(uintptr_t)cmd.addr, cmd.data_len,

			(void __user *)(uintptr_t)cmd.metadata, cmd.metadata_len,

			nvme_to_user_ptr(cmd.addr), cmd.data_len,

			nvme_to_user_ptr(cmd.metadata), cmd.metadata_len,

			0, &cmd.result, timeout);

	nvme_passthru_end(ctrl, effects);

CRA Git | Maintained and supported by SUSTech CRA and CCSE