Re: [PATCH AUTOSEL 5.4 38/78] nvme: fix compat address handling in several ioctls

[Naresh Kamboju <naresh.kamboju@linaro.org>]

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