linux-kernel - compat_alloc_user_space removal, was Re: [PATCH 3/3] scsi: megaraid_sas: simplify compat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 13 Sep 2020 07:50:51 +0100
From:   Christoph Hellwig <hch@...radead.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Christoph Hellwig <hch@...radead.org>,
        Kashyap Desai <kashyap.desai@...adcom.com>,
        Sumit Saxena <sumit.saxena@...adcom.com>,
        Shivasharan S <shivasharan.srikanteshwara@...adcom.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Anand Lodnoor <anand.lodnoor@...adcom.com>,
        Chandrakanth Patil <chandrakanth.patil@...adcom.com>,
        Hannes Reinecke <hare@...e.de>, megaraidlinux.pdl@...adcom.com,
        linux-scsi <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        viro@...iv.linux.org.uk
Subject: compat_alloc_user_space removal, was Re: [PATCH 3/3] scsi:
 megaraid_sas: simplify compat_ioctl handling

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.