* [PATCH v7] usb: f_fs: Fix use-after-free for epfile
@ 2022-01-05 6:31 Udipto Goswami
2022-01-05 13:15 ` kernel test robot
0 siblings, 1 reply; 2+ messages in thread
From: Udipto Goswami @ 2022-01-05 6:31 UTC (permalink / raw)
To: Felipe Balbi, Greg Kroah-Hartman, John Keeping
Cc: Udipto Goswami, linux-usb, Pratham Pratap, Pavankumar Kondeti,
Jack Pham
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: a9e6f83c2df (usb: gadget: f_fs: stop sleeping in
ffs_func_eps_disable)
Reviewed-by: John Keeping <john@metanate.com>
Signed-off-by: Pratham Pratap <quic_ppratap@quicinc.com>
Co-developed-by: Udipto Goswami <quic_ugoswami@quicinc.com>
Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com>
---
v7: Extending the scope to protect epfiles.
drivers/usb/gadget/function/f_fs.c | 59 ++++++++++++++++++++++++++++----------
1 file changed, 44 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 3c584da..83f4eba 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1711,16 +1711,24 @@ static void ffs_data_put(struct ffs_data *ffs)
static void ffs_data_closed(struct ffs_data *ffs)
{
+ struct ffs_epfile *epfiles;
+ unsigned long flags;
+
ENTER();
if (atomic_dec_and_test(&ffs->opened)) {
if (ffs->no_disconnect) {
ffs->state = FFS_DEACTIVATED;
- if (ffs->epfiles) {
- ffs_epfiles_destroy(ffs->epfiles,
- ffs->eps_count);
- ffs->epfiles = NULL;
- }
+ spin_lock_irqsave(&ffs->eps_lock, flags);
+ epfiles = ffs->epfiles;
+ ffs->epfiles = NULL;
+ spin_unlock_irqrestore(&ffs->eps_lock,
+ flags);
+
+ if (epfiles)
+ ffs_epfiles_destroy(epfiles,
+ ffs->eps_count);
+
if (ffs->setup_state == FFS_SETUP_PENDING)
__ffs_ep0_stall(ffs);
} else {
@@ -1767,14 +1775,27 @@ static struct ffs_data *ffs_data_new(const char *dev_name)
static void ffs_data_clear(struct ffs_data *ffs)
{
+ struct ffs_epfile *epfiles;
+ unsigned long flags;
+
ENTER();
ffs_closed(ffs);
BUG_ON(ffs->gadget);
- if (ffs->epfiles)
- ffs_epfiles_destroy(ffs->epfiles, ffs->eps_count);
+ spin_lock_irqsave(&ffs->eps_lock, flags);
+ epfiles = ffs->epfiles;
+ ffs->epfiles = NULL;
+ spin_unlock_irqrestore(&ffs->eps_lock, flags);
+
+ /*
+ * potential race possible between ffs_func_eps_disable
+ * & ffs_epfile_release therefore maintaining a local
+ * copy of epfile will save us from use-after-free.
+ */
+ if (epfiles)
+ ffs_epfiles_destroy(epfiles, ffs->eps_count);
if (ffs->ffs_eventfd)
eventfd_ctx_put(ffs->ffs_eventfd);
@@ -1790,7 +1811,6 @@ static void ffs_data_reset(struct ffs_data *ffs)
ffs_data_clear(ffs);
- ffs->epfiles = NULL;
ffs->raw_descs_data = NULL;
ffs->raw_descs = NULL;
ffs->raw_strings = NULL;
@@ -1895,7 +1915,9 @@ static int ffs_epfiles_create(struct ffs_data *ffs)
}
}
+ spin_lock_irqsave(&ffs->eps_lock, flags);
ffs->epfiles = epfiles;
+ spin_unlock_irqrestore(&ffs->eps_lock, flags);
return 0;
}
@@ -1919,12 +1941,15 @@ static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count)
static void ffs_func_eps_disable(struct ffs_function *func)
{
- struct ffs_ep *ep = func->eps;
- struct ffs_epfile *epfile = func->ffs->epfiles;
- unsigned count = func->ffs->eps_count;
+ struct ffs_ep *ep;
+ struct ffs_epfile *epfile;
+ unsigned short count;
unsigned long flags;
spin_lock_irqsave(&func->ffs->eps_lock, flags);
+ count = func->ffs->eps_count;
+ epfile = func->ffs->epfiles;
+ ep = func->eps;
while (count--) {
/* pending requests get nuked */
if (ep->ep)
@@ -1942,14 +1967,18 @@ static void ffs_func_eps_disable(struct ffs_function *func)
static int ffs_func_eps_enable(struct ffs_function *func)
{
- struct ffs_data *ffs = func->ffs;
- struct ffs_ep *ep = func->eps;
- struct ffs_epfile *epfile = ffs->epfiles;
- unsigned count = ffs->eps_count;
+ struct ffs_data *ffs;
+ struct ffs_ep *ep;
+ struct ffs_epfile *epfile;
+ unsigned count;
unsigned long flags;
int ret = 0;
spin_lock_irqsave(&func->ffs->eps_lock, flags);
+ ffs = func->ffs;
+ ep = func->eps;
+ epfiles = ffs->epfiles;
+ count = ffs->eps_count;
while(count--) {
ep->ep->driver_data = ep;
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v7] usb: f_fs: Fix use-after-free for epfile
2022-01-05 6:31 [PATCH v7] usb: f_fs: Fix use-after-free for epfile Udipto Goswami
@ 2022-01-05 13:15 ` kernel test robot
0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2022-01-05 13:15 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 8498 bytes --]
Hi Udipto,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on peter-chen-usb/for-usb-next]
[cannot apply to usb/usb-testing balbi-usb/testing/next v5.16-rc8 next-20220105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Udipto-Goswami/usb-f_fs-Fix-use-after-free-for-epfile/20220105-143439
base: https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git for-usb-next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220105/202201052146.RZUTvDGn-lkp(a)intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0a319144fb2e68829c0d23f5b5505a19a207c906
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Udipto-Goswami/usb-f_fs-Fix-use-after-free-for-epfile/20220105-143439
git checkout 0a319144fb2e68829c0d23f5b5505a19a207c906
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:17,
from include/linux/list.h:9,
from include/linux/rculist.h:10,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/blkdev.h:5,
from drivers/usb/gadget/function/f_fs.c:17:
drivers/usb/gadget/function/f_fs.c: In function 'ffs_epfiles_create':
>> drivers/usb/gadget/function/f_fs.c:1918:43: error: 'flags' undeclared (first use in this function)
1918 | spin_lock_irqsave(&ffs->eps_lock, flags);
| ^~~~~
include/linux/typecheck.h:11:16: note: in definition of macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/spinlock.h:384:9: note: in expansion of macro 'raw_spin_lock_irqsave'
384 | raw_spin_lock_irqsave(spinlock_check(lock), flags); \
| ^~~~~~~~~~~~~~~~~~~~~
drivers/usb/gadget/function/f_fs.c:1918:9: note: in expansion of macro 'spin_lock_irqsave'
1918 | spin_lock_irqsave(&ffs->eps_lock, flags);
| ^~~~~~~~~~~~~~~~~
drivers/usb/gadget/function/f_fs.c:1918:43: note: each undeclared identifier is reported only once for each function it appears in
1918 | spin_lock_irqsave(&ffs->eps_lock, flags);
| ^~~~~
include/linux/typecheck.h:11:16: note: in definition of macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/spinlock.h:384:9: note: in expansion of macro 'raw_spin_lock_irqsave'
384 | raw_spin_lock_irqsave(spinlock_check(lock), flags); \
| ^~~~~~~~~~~~~~~~~~~~~
drivers/usb/gadget/function/f_fs.c:1918:9: note: in expansion of macro 'spin_lock_irqsave'
1918 | spin_lock_irqsave(&ffs->eps_lock, flags);
| ^~~~~~~~~~~~~~~~~
include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
12 | (void)(&__dummy == &__dummy2); \
| ^~
include/linux/spinlock.h:251:17: note: in expansion of macro 'typecheck'
251 | typecheck(unsigned long, flags); \
| ^~~~~~~~~
include/linux/spinlock.h:384:9: note: in expansion of macro 'raw_spin_lock_irqsave'
384 | raw_spin_lock_irqsave(spinlock_check(lock), flags); \
| ^~~~~~~~~~~~~~~~~~~~~
drivers/usb/gadget/function/f_fs.c:1918:9: note: in expansion of macro 'spin_lock_irqsave'
1918 | spin_lock_irqsave(&ffs->eps_lock, flags);
| ^~~~~~~~~~~~~~~~~
drivers/usb/gadget/function/f_fs.c: In function 'ffs_func_eps_enable':
>> drivers/usb/gadget/function/f_fs.c:1980:9: error: 'epfiles' undeclared (first use in this function); did you mean 'epfile'?
1980 | epfiles = ffs->epfiles;
| ^~~~~~~
| epfile
vim +/flags +1918 drivers/usb/gadget/function/f_fs.c
1888
1889 static int ffs_epfiles_create(struct ffs_data *ffs)
1890 {
1891 struct ffs_epfile *epfile, *epfiles;
1892 unsigned i, count;
1893
1894 ENTER();
1895
1896 count = ffs->eps_count;
1897 epfiles = kcalloc(count, sizeof(*epfiles), GFP_KERNEL);
1898 if (!epfiles)
1899 return -ENOMEM;
1900
1901 epfile = epfiles;
1902 for (i = 1; i <= count; ++i, ++epfile) {
1903 epfile->ffs = ffs;
1904 mutex_init(&epfile->mutex);
1905 if (ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR)
1906 sprintf(epfile->name, "ep%02x", ffs->eps_addrmap[i]);
1907 else
1908 sprintf(epfile->name, "ep%u", i);
1909 epfile->dentry = ffs_sb_create_file(ffs->sb, epfile->name,
1910 epfile,
1911 &ffs_epfile_operations);
1912 if (!epfile->dentry) {
1913 ffs_epfiles_destroy(epfiles, i - 1);
1914 return -ENOMEM;
1915 }
1916 }
1917
> 1918 spin_lock_irqsave(&ffs->eps_lock, flags);
1919 ffs->epfiles = epfiles;
1920 spin_unlock_irqrestore(&ffs->eps_lock, flags);
1921 return 0;
1922 }
1923
1924 static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count)
1925 {
1926 struct ffs_epfile *epfile = epfiles;
1927
1928 ENTER();
1929
1930 for (; count; --count, ++epfile) {
1931 BUG_ON(mutex_is_locked(&epfile->mutex));
1932 if (epfile->dentry) {
1933 d_delete(epfile->dentry);
1934 dput(epfile->dentry);
1935 epfile->dentry = NULL;
1936 }
1937 }
1938
1939 kfree(epfiles);
1940 }
1941
1942 static void ffs_func_eps_disable(struct ffs_function *func)
1943 {
1944 struct ffs_ep *ep;
1945 struct ffs_epfile *epfile;
1946 unsigned short count;
1947 unsigned long flags;
1948
1949 spin_lock_irqsave(&func->ffs->eps_lock, flags);
1950 count = func->ffs->eps_count;
1951 epfile = func->ffs->epfiles;
1952 ep = func->eps;
1953 while (count--) {
1954 /* pending requests get nuked */
1955 if (ep->ep)
1956 usb_ep_disable(ep->ep);
1957 ++ep;
1958
1959 if (epfile) {
1960 epfile->ep = NULL;
1961 __ffs_epfile_read_buffer_free(epfile);
1962 ++epfile;
1963 }
1964 }
1965 spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
1966 }
1967
1968 static int ffs_func_eps_enable(struct ffs_function *func)
1969 {
1970 struct ffs_data *ffs;
1971 struct ffs_ep *ep;
1972 struct ffs_epfile *epfile;
1973 unsigned count;
1974 unsigned long flags;
1975 int ret = 0;
1976
1977 spin_lock_irqsave(&func->ffs->eps_lock, flags);
1978 ffs = func->ffs;
1979 ep = func->eps;
> 1980 epfiles = ffs->epfiles;
1981 count = ffs->eps_count;
1982 while(count--) {
1983 ep->ep->driver_data = ep;
1984
1985 ret = config_ep_by_speed(func->gadget, &func->function, ep->ep);
1986 if (ret) {
1987 pr_err("%s: config_ep_by_speed(%s) returned %d\n",
1988 __func__, ep->ep->name, ret);
1989 break;
1990 }
1991
1992 ret = usb_ep_enable(ep->ep);
1993 if (!ret) {
1994 epfile->ep = ep;
1995 epfile->in = usb_endpoint_dir_in(ep->ep->desc);
1996 epfile->isoc = usb_endpoint_xfer_isoc(ep->ep->desc);
1997 } else {
1998 break;
1999 }
2000
2001 ++ep;
2002 ++epfile;
2003 }
2004
2005 wake_up_interruptible(&ffs->wait);
2006 spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
2007
2008 return ret;
2009 }
2010
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-05 13:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-05 6:31 [PATCH v7] usb: f_fs: Fix use-after-free for epfile Udipto Goswami
2022-01-05 13:15 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.