linux-kernel - Re: [CFT][PATCH] signal/usb: Replace kill_pid_info_as_cred with kill_pid_usb

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]

Message-ID: <Pine.LNX.4.44L0.1902081652080.1482-200000@iolanthe.rowland.org>
Date:   Fri, 8 Feb 2019 16:57:27 -0500 (EST)
From:   Alan Stern <stern@...land.harvard.edu>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
cc:     linux-usb@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Oliver Neukum <oneukum@...e.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [CFT][PATCH] signal/usb: Replace kill_pid_info_as_cred with
 kill_pid_usb_asyncio

On Thu, 7 Feb 2019, Eric W. Biederman wrote:

> The usb support for asyncio encoded one of it's values in the wrong
> field.  It should have used si_value but instead used si_addr which is
> not present in the _rt union member of struct siginfo.
> 
> The result is a POSIX and glibc incompatible encoding of fields in
> struct siginfo with si_code of SI_ASYNCIO.  This makes it impossible
> to look at a struct siginfo with si_code set to SI_ASYNCIO and without
> context properly decode it.  Which unfortunately means that
> copy_siginfo_to_user32 can't handle the compat issues this unfortunate
> choice in encodings brings up.
> 
> Therefore replace kill_pid_info_as_cred with kill_pid_usb_asyncio a
> dedicated function for this one specific case.  There are no other
> users of kill_pid_info_as_cred so this specialization should have no
> impact on the amont of code in the kernel.  Have kill_pid_usb_asyncio
> take instead of a siginfo_t which is difficult error prone 3
> arguments, a signal number, an errno value, and an address enconded as
> a sigval_t.  The encoding as a sigval_t allows the caller to deal with
> the compat issues before calling kill_pid_info_as_cred.
> 
> Add BUILD_BUG_ONs in kernel/signal.c to ensure that we can now place
> the pointer value at the in si_pid (instead of si_addr) and get
> the same binary result when the structure is copied to user space
> and when the structure is copied field by field.
> 
> The usb code is updated to track if the values it passes into
> kill_pid_usb_asyncio were passed to it from a native userspace
> or from at compat user space.  To allow a proper conversion
> of the addresses.
> 
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: linux-usb@...r.kernel.org
> Cc: Alan Stern <stern@...land.harvard.edu>
> Cc: Oliver Neukum <oneukum@...e.com>
> Fixes: v2.3.39
> Cc: stable@...r.kernel.org
> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> ---
> 
> Can I get someone to test this code?  I just discovered that the
> usb code is filling in struct siginfo wrong and copy_siginfo_to_user32
> can't possibly get this correct without some help.
> 
> I think I have coded up a working fix.  But I don't have a setup
> where I can test this.

Eric:

You should be able to test this patch by running the attached 
program.  It takes one argument, the pathname to a USB device file.  
For example, on my system:

# ./usbsig /dev/bus/usb/001/001
Got signal 10, signo 10 errno 0 code -4

I don't know exactly what you're looking for, but it should be pretty 
easy to modify the test program however you want.

If you need to test the compatibility mode specifically, I can do that
here -- I'm running a 32-bit userspace on a 64-bit kernel.  But you'll
have to tell me exactly what test code to run.

Alan Stern

View attachment "usbsig.c" of type "TEXT/plain" (1791 bytes)