On Sat, 18 Apr 2020 at 20:24, Sasha Levin <sashal@xxxxxxxxxx> wrote: > > From: Nick Bowler <nbowler@xxxxxxxxxx> > > [ 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@xxxxxxxxxx> > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> > --- > 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