Skip to content

Commit ebe2b1a

Browse files
Udipto Goswamigregkh
authored andcommitted
usb: f_fs: Fix use-after-free for epfile
Consider a case where ffs_func_eps_disable is called from ffs_func_disable as part of composition switch and at the same time ffs_epfile_release get called from userspace. ffs_epfile_release will free up the read buffer and call ffs_data_closed which in turn destroys ffs->epfiles and mark it as NULL. While this was happening the driver has already initialized the local epfile in ffs_func_eps_disable which is now freed and waiting to acquire the spinlock. Once spinlock is acquired the driver proceeds with the stale value of epfile and tries to free the already freed read buffer causing use-after-free. Following is the illustration of the race: CPU1 CPU2 ffs_func_eps_disable epfiles (local copy) ffs_epfile_release ffs_data_closed if (last file closed) ffs_data_reset ffs_data_clear ffs_epfiles_destroy spin_lock dereference epfiles Fix this races by taking epfiles local copy & assigning it under spinlock and if epfiles(local) is null then update it in ffs->epfiles then finally destroy it. Extending the scope further from the race, protecting the ep related structures, and concurrent accesses. Fixes: a9e6f83 ("usb: gadget: f_fs: stop sleeping in ffs_func_eps_disable") Co-developed-by: Udipto Goswami <[email protected]> Reviewed-by: John Keeping <[email protected]> Signed-off-by: Pratham Pratap <[email protected]> Signed-off-by: Udipto Goswami <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent b470947 commit ebe2b1a

File tree

1 file changed

+42
-14
lines changed
  • drivers/usb/gadget/function

1 file changed

+42
-14
lines changed

drivers/usb/gadget/function/f_fs.c

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1711,16 +1711,24 @@ static void ffs_data_put(struct ffs_data *ffs)
17111711

17121712
static void ffs_data_closed(struct ffs_data *ffs)
17131713
{
1714+
struct ffs_epfile *epfiles;
1715+
unsigned long flags;
1716+
17141717
ENTER();
17151718

17161719
if (atomic_dec_and_test(&ffs->opened)) {
17171720
if (ffs->no_disconnect) {
17181721
ffs->state = FFS_DEACTIVATED;
1719-
if (ffs->epfiles) {
1720-
ffs_epfiles_destroy(ffs->epfiles,
1721-
ffs->eps_count);
1722-
ffs->epfiles = NULL;
1723-
}
1722+
spin_lock_irqsave(&ffs->eps_lock, flags);
1723+
epfiles = ffs->epfiles;
1724+
ffs->epfiles = NULL;
1725+
spin_unlock_irqrestore(&ffs->eps_lock,
1726+
flags);
1727+
1728+
if (epfiles)
1729+
ffs_epfiles_destroy(epfiles,
1730+
ffs->eps_count);
1731+
17241732
if (ffs->setup_state == FFS_SETUP_PENDING)
17251733
__ffs_ep0_stall(ffs);
17261734
} else {
@@ -1767,14 +1775,27 @@ static struct ffs_data *ffs_data_new(const char *dev_name)
17671775

17681776
static void ffs_data_clear(struct ffs_data *ffs)
17691777
{
1778+
struct ffs_epfile *epfiles;
1779+
unsigned long flags;
1780+
17701781
ENTER();
17711782

17721783
ffs_closed(ffs);
17731784

17741785
BUG_ON(ffs->gadget);
17751786

1776-
if (ffs->epfiles) {
1777-
ffs_epfiles_destroy(ffs->epfiles, ffs->eps_count);
1787+
spin_lock_irqsave(&ffs->eps_lock, flags);
1788+
epfiles = ffs->epfiles;
1789+
ffs->epfiles = NULL;
1790+
spin_unlock_irqrestore(&ffs->eps_lock, flags);
1791+
1792+
/*
1793+
* potential race possible between ffs_func_eps_disable
1794+
* & ffs_epfile_release therefore maintaining a local
1795+
* copy of epfile will save us from use-after-free.
1796+
*/
1797+
if (epfiles) {
1798+
ffs_epfiles_destroy(epfiles, ffs->eps_count);
17781799
ffs->epfiles = NULL;
17791800
}
17801801

@@ -1922,12 +1943,15 @@ static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count)
19221943

19231944
static void ffs_func_eps_disable(struct ffs_function *func)
19241945
{
1925-
struct ffs_ep *ep = func->eps;
1926-
struct ffs_epfile *epfile = func->ffs->epfiles;
1927-
unsigned count = func->ffs->eps_count;
1946+
struct ffs_ep *ep;
1947+
struct ffs_epfile *epfile;
1948+
unsigned short count;
19281949
unsigned long flags;
19291950

19301951
spin_lock_irqsave(&func->ffs->eps_lock, flags);
1952+
count = func->ffs->eps_count;
1953+
epfile = func->ffs->epfiles;
1954+
ep = func->eps;
19311955
while (count--) {
19321956
/* pending requests get nuked */
19331957
if (ep->ep)
@@ -1945,14 +1969,18 @@ static void ffs_func_eps_disable(struct ffs_function *func)
19451969

19461970
static int ffs_func_eps_enable(struct ffs_function *func)
19471971
{
1948-
struct ffs_data *ffs = func->ffs;
1949-
struct ffs_ep *ep = func->eps;
1950-
struct ffs_epfile *epfile = ffs->epfiles;
1951-
unsigned count = ffs->eps_count;
1972+
struct ffs_data *ffs;
1973+
struct ffs_ep *ep;
1974+
struct ffs_epfile *epfile;
1975+
unsigned short count;
19521976
unsigned long flags;
19531977
int ret = 0;
19541978

19551979
spin_lock_irqsave(&func->ffs->eps_lock, flags);
1980+
ffs = func->ffs;
1981+
ep = func->eps;
1982+
epfile = ffs->epfiles;
1983+
count = ffs->eps_count;
19561984
while(count--) {
19571985
ep->ep->driver_data = ep;
19581986

0 commit comments

Comments
 (0)