On Sat, 18 Apr 2020 at 20:24, Sasha Levin sashal@kernel.org wrote:

From: Nick Bowler nbowler@draconx.ca

[ Upstream commit c95b708d5fa65b4e51f088ee077d127fd5a57b70 ]

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: Nick Bowler nbowler@draconx.ca Signed-off-by: Christoph Hellwig hch@lst.de Signed-off-by: Sasha Levin sashal@kernel.org

---

drivers/nvme/host/core.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index b8fe42f4b3c5b..f97c48fd3edae 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -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> @@ -1244,6 +1245,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;
  

arm64 make modules failed while building with an extra kernel config.

CONFIG_ARM64_64K_PAGES=y

# make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTCC=gcc CC="sccache aarch64-linux-gnu-gcc" O=build modules 70 # 71 ../drivers/nvme/host/core.c: In function ‘nvme_to_user_ptr’: 72 ../drivers/nvme/host/core.c:1256:13: error: ‘compat_uptr_t’ undeclared (first use in this function); did you mean ‘compat_time_t’? 73 1256 | ptrval = (compat_uptr_t)ptrval; 74 | ^~~~~~~~~~~~~ 75 | compat_time_t 76 ../drivers/nvme/host/core.c:1256:13: note: each undeclared identifier is reported only once for each function it appears in 77 ../drivers/nvme/host/core.c:1256:27: error: expected ‘;’ before ‘ptrval’ 78 1256 | ptrval = (compat_uptr_t)ptrval; 79 | ^~~~~~ 80 | ; 81 make[4]: *** [../scripts/Makefile.build:266: drivers/nvme/host/core.o] Error 1

full build log, https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/528723851

Kernel config: https://builds.tuxbuild.com/DeL6EepmdRw6OaOGmg7F_g/kernel.config