compat_alloc_user_space removal, was Re: [PATCH 3/3] scsi: megaraid_sas: simplify compat_ioctl handling

Christoph Hellwig
2020-09-13T06:51:11Z

On Sat, Sep 12, 2020 at 02:49:05PM +0200, Arnd Bergmann wrote:
> fs/quota/compat.c: dqblk = compat_alloc_user_space(sizeof(struct if_dqblk));
> fs/quota/compat.c: dqblk = compat_alloc_user_space(sizeof(struct if_dqblk));
> fs/quota/compat.c: fsqstat = compat_alloc_user_space(sizeof(struct
> fs_quota_stat));

I sent this out a while ago, an Al has it in a branch, but not in
linux-next:

https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/log/?h=work.quota-compat

> drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c: karg =
> compat_alloc_user_space(
>
> Had a brief look but did not investigate further, it's complicated.
>
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: args =
> compat_alloc_user_space(sizeof(*args));
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: args =
> compat_alloc_user_space(sizeof(*args) +
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: args =
> compat_alloc_user_space(sizeof(*args));
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: args =
> compat_alloc_user_space(sizeof(*args) +
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: args =
> compat_alloc_user_space(sizeof(*args));
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: args =
> compat_alloc_user_space(sizeof(*args));
>
> Should not be too hard, but I have not looked in detail.

We do not have to care about staging drivers when removing interfaces.
But to be nice you probably ping the maintainers to see what they can
do.

I also have the mount side handles in this branch which I need to rebase
and submit:

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/mount-cleanups

> I think you got the wrong one there, the code above is where the
> dma address gets stored in the in-kernel copy of the sense data
> based on the user space offset. Conceptually it does make sense
> though and would end up looking something like
>
> if (ioc->sense_len) {
> /*
> * sense_ptr points to the location that has the user
> * sense buffer address
> */
> sense_ptr = (void *)ioc->frame.raw + ioc->sense_off;
> if (in_compat_syscall())
> uptr = compat_ptr(get_unaligned(u32 *)sense_ptr);
> else
> uptr = get_unaligned((void __user **)sense_ptr);
>
> if (copy_to_user(uptr, sense, ioc->sense_len)) {

Indeed. As said, I had started on the change and gave up pretty quickly
:)

> I tried that, but there is still one difference because one of them uses
> MEGASAS_IOC_FIRMWARE while the other one uses
> MEGASAS_IOC_FIRMWARE32. It would be possible to have
> a common handler that always handles both command codes.
> I tried to avoid changing the behavior that way though.

Ok.