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 <gre...@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Cc: Alan Stern <st...@rowland.harvard.edu>
> Cc: Oliver Neukum <oneu...@suse.com>
> Fixes: v2.3.39
> Cc: sta...@vger.kernel.org
> Signed-off-by: "Eric W. Biederman" <ebied...@xmission.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
/* usbsig.c -- test USB async signal delivery */
#include <stdio.h>
#include <fcntl.h>
#include <signal.h>
#include <string.h>
#include <sys/ioctl.h>
#include <unistd.h>
#include <linux/usb/ch9.h>
#include <linux/usbdevice_fs.h>
void handler(int sig, siginfo_t *info , void *ucontext)
{
ucontext_t *con = (ucontext_t *) ucontext;
printf("Got signal %d, signo %d errno %d code %d\n",
sig, info->si_signo, info->si_errno, info->si_code);
}
int main(int argc, char **argv)
{
char *devfilename;
int fd;
int rc;
struct sigaction act;
struct usbdevfs_urb urb;
struct usb_ctrlrequest *req;
void *ptr;
char buf[80];
if (argc != 2) {
fprintf(stderr, "Usage: usbsig device-file-name\n");
return 1;
}
devfilename = argv[1];
fd = open(devfilename, O_RDWR);
if (fd == -1) {
perror("Error opening device file");
return 1;
}
act.sa_sigaction = handler;
sigemptyset(&act.sa_mask);
act.sa_flags = SA_SIGINFO;
rc = sigaction(SIGUSR1, &act, NULL);
if (rc == -1) {
perror("Error in sigaction");
return 1;
}
memset(&urb, 0, sizeof(urb));
urb.type = USBDEVFS_URB_TYPE_CONTROL;
urb.endpoint = USB_DIR_IN | 0;
urb.buffer = buf;
urb.buffer_length = sizeof(buf);
urb.signr = SIGUSR1;
req = (struct usb_ctrlrequest *) buf;
req->bRequestType = USB_DIR_IN | USB_TYPE_STANDARD | USB_RECIP_DEVICE;
req->bRequest = USB_REQ_GET_DESCRIPTOR;
req->wValue = USB_DT_DEVICE << 8;
req->wIndex = 0;
req->wLength = sizeof(buf) - sizeof(*req);
rc = ioctl(fd, USBDEVFS_SUBMITURB, &urb);
if (rc == -1) {
perror("Error in SUBMITURB ioctl");
return 1;
}
rc = ioctl(fd, USBDEVFS_REAPURB, &ptr);
if (rc == -1) {
perror("Error in REAPURB ioctl");
return 1;
}
close(fd);
return 0;
}
