d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

usb(4): Separate the fast path and the slow path to avoid races and use-after-free for the USB FS interface.

Browse Source 
Bad behaving user-space USB applicatoins may crash the kernel by issuing
USB FS related ioctl(2)'s out of their expected order. By default
the USB FS ioctl(2) interface is only available to the
administrator, root, and driver applications like webcamd(8) needs
to be hijacked in order for this to happen.

The issue is the fast-path code does not always see updates made
by the slow-path code, and may then work on freed memory.

This is easily fixed by using an EPOCH(9) type of synchronization
mechanism. A SX(9) lock will be used as a substitute for EPOCH(9),
due to the need for sleepability. In addition most calls going into
the fast-path originate from a single user-space process and the
need for multi-thread performance is not present.

Differential Revision:	https://reviews.freebsd.org/D39373
Reviewed by:	markj@
Reported by:	C Turt <ecturt@gmail.com>
admbugs:	994
MFC after:	1 week
Sponsored by:	NVIDIA Networking
This commit is contained in:
Hans Petter Selasky 2023-03-31 19:14:18 +02:00
parent 03a2e432d5
commit 9b077d72bc
4 changed files with 82 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
sys/dev/usb/usb_dev.c
View File

@ -2,7 +2,7 @@
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2006-2008 Hans Petter Selasky. All rights reserved.
* Copyright (c) 2006-2023 Hans Petter Selasky
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@ -386,6 +386,7 @@ usb_fifo_alloc(struct mtx *mtx)
f = malloc(sizeof(*f), M_USBDEV, M_WAITOK | M_ZERO);
cv_init(&f->cv_io, "FIFO-IO");
cv_init(&f->cv_drain, "FIFO-DRAIN");
sx_init(&f->fs_fastpath_lock, "FIFO-FP");
f->priv_mtx = mtx;
f->refcount = 1;
knlist_init_mtx(&f->selinfo.si_note, mtx);
@ -626,6 +627,7 @@ usb_fifo_free(struct usb_fifo *f)
cv_destroy(&f->cv_io);
cv_destroy(&f->cv_drain);
sx_destroy(&f->fs_fastpath_lock);
knlist_clear(&f->selinfo.si_note, 0);
seldrain(&f->selinfo);

6
sys/dev/usb/usb_dev.h
View File

@ -2,7 +2,7 @@
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2008 Hans Petter Selasky. All rights reserved.
* Copyright (c) 2008-2023 Hans Petter Selasky
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@ -110,13 +110,15 @@ struct usb_fifo {
struct selinfo selinfo;
struct cv cv_io;
struct cv cv_drain;
struct sx fs_fastpath_lock;
struct usb_fifo_methods *methods;
struct usb_symlink *symlink[2];/* our symlinks */
struct proc *async_p; /* process that wants SIGIO */
struct usb_fs_endpoint *fs_ep_ptr;
struct usb_device *udev;
#define USB_FS_XFER_MAX 126
struct usb_xfer *xfer[2];
struct usb_xfer **fs_xfer;
struct usb_xfer *fs_xfer[USB_FS_XFER_MAX];
struct mtx *priv_mtx; /* client data */
/* set if FIFO is opened by a FILE: */
struct usb_cdev_privdata *curr_cpd;

6
sys/dev/usb/usb_device.c
View File

@ -2,7 +2,7 @@
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2008-2020 Hans Petter Selasky. All rights reserved.
* Copyright (c) 2008-2023 Hans Petter Selasky
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@ -2829,7 +2829,7 @@ usb_fifo_free_wrap(struct usb_device *udev,
continue;
}
if ((f->dev_ep_index == 0) &&
(f->fs_xfer == NULL)) {
(f->fs_ep_max == 0)) {
/* no need to free this FIFO */
continue;
}
@ -2837,7 +2837,7 @@ usb_fifo_free_wrap(struct usb_device *udev,
if ((f->methods == &usb_ugen_methods) &&
(f->dev_ep_index == 0) &&
(!(flag & USB_UNCFG_FLAG_FREE_EP0)) &&
(f->fs_xfer == NULL)) {
(f->fs_ep_max == 0)) {
/* no need to free this FIFO */
continue;
}

109
sys/dev/usb/usb_generic.c
View File

@ -2,7 +2,7 @@
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2008-2022 Hans Petter Selasky
* Copyright (c) 2008-2023 Hans Petter Selasky
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@ -968,10 +968,10 @@ ugen_fs_init(struct usb_fifo *f,
int error;
/* verify input parameters */
if (fs_ep_ptr == NULL || ep_index_max > 127)
if (fs_ep_ptr == NULL || ep_index_max > USB_FS_XFER_MAX)
return (EINVAL);
if (f->fs_xfer != NULL)
if (f->fs_ep_max != 0)
return (EBUSY);
if (f->dev_ep_index != 0 || ep_index_max == 0)
@ -982,11 +982,11 @@ ugen_fs_init(struct usb_fifo *f,
error = usb_fifo_alloc_buffer(f, 1, ep_index_max);
if (error == 0) {
f->fs_xfer = malloc(sizeof(f->fs_xfer[0]) *
ep_index_max, M_USB, M_WAITOK | M_ZERO);
mtx_lock(f->priv_mtx);
f->fs_ep_max = ep_index_max;
f->fs_ep_ptr = fs_ep_ptr;
f->fs_ep_sz = fs_ep_sz;
mtx_unlock(f->priv_mtx);
}
return (error);
}
@ -994,15 +994,26 @@ ugen_fs_init(struct usb_fifo *f,
int
ugen_fs_uninit(struct usb_fifo *f)
{
if (f->fs_xfer == NULL) {
if (f->fs_ep_max == 0)
return (EINVAL);
}
usbd_transfer_unsetup(f->fs_xfer, f->fs_ep_max);
free(f->fs_xfer, M_USB);
f->fs_xfer = NULL;
/*
* Prevent calls into the fast-path code, by setting fs_ep_max
* to zero:
*/
sx_xlock(&f->fs_fastpath_lock);
mtx_lock(f->priv_mtx);
f->fs_ep_max = 0;
mtx_unlock(f->priv_mtx);
sx_xunlock(&f->fs_fastpath_lock);
usbd_transfer_unsetup(f->fs_xfer, USB_FS_XFER_MAX);
mtx_lock(f->priv_mtx);
f->fs_ep_ptr = NULL;
f->flag_iscomplete = 0;
mtx_unlock(f->priv_mtx);
usb_fifo_free_buffer(f);
return (0);
}
@ -1109,10 +1120,26 @@ usb_fs_open(struct usb_fifo *f, struct usb_fs_open *popen,
static int
usb_fs_close(struct usb_fifo *f, struct usb_fs_close *pclose)
{
struct usb_xfer *xfer;
if (pclose->ep_index >= f->fs_ep_max)
return (EINVAL);
usbd_transfer_unsetup(f->fs_xfer + pclose->ep_index, 1);
/*
* Prevent calls into the fast-path code, by setting the
* fs_xfer[] in question to NULL:
*/
sx_xlock(&f->fs_fastpath_lock);
mtx_lock(f->priv_mtx);
xfer = f->fs_xfer[pclose->ep_index];
f->fs_xfer[pclose->ep_index] = NULL;
mtx_unlock(f->priv_mtx);
sx_xunlock(&f->fs_fastpath_lock);
if (xfer == NULL)
return (EINVAL);
usbd_transfer_unsetup(&xfer, 1);
return (0);
}
@ -1249,14 +1276,16 @@ ugen_fs_copy_in(struct usb_fifo *f, uint8_t ep_index)
int error;
uint8_t isread;
mtx_lock(f->priv_mtx);
if (ep_index >= f->fs_ep_max) {
mtx_unlock(f->priv_mtx);
return (EINVAL);
}
xfer = f->fs_xfer[ep_index];
if (xfer == NULL) {
mtx_unlock(f->priv_mtx);
return (EINVAL);
}
mtx_lock(f->priv_mtx);
if (usbd_transfer_pending(xfer)) {
mtx_unlock(f->priv_mtx);
return (EBUSY); /* should not happen */
@ -1529,14 +1558,16 @@ ugen_fs_copy_out(struct usb_fifo *f, uint8_t ep_index)
int error;
uint8_t isread;
if (ep_index >= f->fs_ep_max)
return (EINVAL);
xfer = f->fs_xfer[ep_index];
if (xfer == NULL)
return (EINVAL);
mtx_lock(f->priv_mtx);
if (ep_index >= f->fs_ep_max) {
mtx_unlock(f->priv_mtx);
return (EINVAL);
}
xfer = f->fs_xfer[ep_index];
if (xfer == NULL) {
mtx_unlock(f->priv_mtx);
return (EINVAL);
}
if (!xfer->flags_int.transferring &&
!xfer->flags_int.started) {
mtx_unlock(f->priv_mtx);
@ -1675,10 +1706,6 @@ ugen_ioctl(struct usb_fifo *f, u_long cmd, void *addr, int fflags)
struct usb_fs_complete *pcomp;
struct usb_fs_start *pstart;
struct usb_fs_stop *pstop;
struct usb_fs_open *popen;
struct usb_fs_open_stream *popen_stream;
struct usb_fs_close *pclose;
struct usb_fs_clear_stall_sync *pstall;
void *addr;
} u;
struct usb_xfer *xfer;
@ -1691,6 +1718,7 @@ ugen_ioctl(struct usb_fifo *f, u_long cmd, void *addr, int fflags)
switch (cmd) {
case USB_FS_COMPLETE:
sx_slock(&f->fs_fastpath_lock);
mtx_lock(f->priv_mtx);
error = ugen_fs_get_complete(f, &ep_index);
mtx_unlock(f->priv_mtx);
@ -1701,9 +1729,11 @@ ugen_ioctl(struct usb_fifo *f, u_long cmd, void *addr, int fflags)
u.pcomp->ep_index = ep_index;
error = ugen_fs_copy_out(f, u.pcomp->ep_index);
}
sx_sunlock(&f->fs_fastpath_lock);
break;
case USB_FS_START:
sx_slock(&f->fs_fastpath_lock);
error = ugen_fs_copy_in(f, u.pstart->ep_index);
if (error == 0) {
mtx_lock(f->priv_mtx);
@ -1711,6 +1741,7 @@ ugen_ioctl(struct usb_fifo *f, u_long cmd, void *addr, int fflags)
usbd_transfer_start(xfer);
mtx_unlock(f->priv_mtx);
}
sx_sunlock(&f->fs_fastpath_lock);
break;
case USB_FS_STOP:
@ -1739,20 +1770,6 @@ ugen_ioctl(struct usb_fifo *f, u_long cmd, void *addr, int fflags)
mtx_unlock(f->priv_mtx);
break;
case USB_FS_OPEN:
case USB_FS_OPEN_STREAM:
error = usb_fs_open(f, u.popen, fflags,
(cmd == USB_FS_OPEN_STREAM) ? u.popen_stream->stream_id : 0);
break;
case USB_FS_CLOSE:
error = usb_fs_close(f, u.pclose);
break;
case USB_FS_CLEAR_STALL_SYNC:
error = usb_fs_clear_stall_sync(f, u.pstall);
break;
default:
error = ENOIOCTL;
break;
@ -2212,6 +2229,10 @@ ugen_ioctl_post(struct usb_fifo *f, u_long cmd, void *addr, int fflags)
struct usb_fs_init32 *pinit32;
#endif
struct usb_fs_uninit *puninit;
struct usb_fs_open *popen;
struct usb_fs_open_stream *popen_stream;
struct usb_fs_close *pclose;
struct usb_fs_clear_stall_sync *pstall;
struct usb_device_port_path *dpp;
uint32_t *ptime;
void *addr;
@ -2440,6 +2461,20 @@ ugen_ioctl_post(struct usb_fifo *f, u_long cmd, void *addr, int fflags)
error = ugen_fs_uninit(f);
break;
case USB_FS_OPEN:
case USB_FS_OPEN_STREAM:
error = usb_fs_open(f, u.popen, fflags,
(cmd == USB_FS_OPEN_STREAM) ? u.popen_stream->stream_id : 0);
break;
case USB_FS_CLOSE:
error = usb_fs_close(f, u.pclose);
break;
case USB_FS_CLEAR_STALL_SYNC:
error = usb_fs_clear_stall_sync(f, u.pstall);
break;
default:
mtx_lock(f->priv_mtx);
error = ugen_iface_ioctl(f, cmd, addr, fflags);