[BUG] usb: gadget: configfs: use-after-free of gadget strings during bind and under a bound gadget

[Yu Zhang]

Hi,

The gadget-side string configfs objects (the strings/<langid> directories
and the strings beneath them) are created, modified and destroyed with no
serialization against, and no lifetime coupling to, a bound gadget. This
gives two use-after-frees, both reproduced on an acm gadget over
dummy_hcd with KASAN. Both are root/configfs triggered.

== Background ==

Writing the UDC attribute (gadget_dev_desc_UDC_store()) runs
usb_gadget_register_driver() while holding gi->lock, and
configfs_composite_bind() -> configfs_attach_gadget_strings() then walks
gi->string_list and, per language, language->gadget_strings to build the
usb_string tables (the "the gi->lock is hold by the caller" path).

The configfs callbacks that build and tear those lists down do NOT take
gi->lock:
  - gadget_language_make() / gadget_language_attr_release() add/remove
    languages on gi->string_list;
  - gadget_language_string_make() / gadget_language_string_drop() add/
    remove strings on language->gadget_strings and adjust nstrings.
(Contrast function_make()/function_drop(), which take gi->lock around
gi->available_func.)

A store to the gadget's "UDC" attribute and a mkdir/rmdir of a child
strings/<langid> directory are not serialized by the configfs core: they
hold disjoint locks (the store holds the gadget-root fragment rwsem plus
gi->lock; the rmdir holds su_mutex plus the langid's own fragment rwsem
plus the strings/ i_rwsem), so they run concurrently on different CPUs.

== UAF #1: gi->string_list walked during bind vs. concurrent rmdir ==

configfs_attach_gadget_strings() dereferences a struct gadget_language
that a concurrent "rmdir strings/<langid>" frees:

  BUG: KASAN: slab-use-after-free in configfs_composite_bind+0x2ba/0xe70
  Read of size 4 at addr ffff8880085db900 by task ...   (language->nstrings)
   configfs_composite_bind+0x2ba/0xe70
   gadget_bind_driver+0x170/0x5f0
   really_probe+...
   gadget_dev_desc_UDC_store+0x179/0x220
   configfs_write_iter+0x1a7/0x250
   vfs_write+0x5df/0x940
  Allocated by task ...:
   gadget_language_make+0x93/0x340
   configfs_mkdir+...
   vfs_mkdir+0x1ed/0x4d0
   __x64_sys_mkdir+0x47/0x60
  Freed by task ...:
   kfree+...
   config_item_put+0x89/0xa0
   configfs_rmdir+0x42f/0x5b0
   __x64_sys_rmdir+0x34/0x50

== UAF #2: a bound gadget references strings that rmdir frees ==

configfs_attach_gadget_strings() stores raw pointers to each
language->manufacturer/product/serialnumber (and each string->string)
into the cdev's string tables; usb_gstrings_attach() does not copy the
string contents. Removing a language (or a string) with rmdir while the
gadget is bound frees those buffers in gadget_language_attr_release() /
gadget_string_release(), and the next host string request dereferences
them:

  BUG: KASAN: slab-use-after-free in strlen+0xe/0x40
  Read of size 1 at addr ffff8880065f2000 by task swapper/2/0  (GET_DESCRIPTOR)
   strlen+0xe/0x40
   usb_gadget_get_string+0xbb/0x1b0
   composite_setup+0x33fa/0x4690
   configfs_composite_setup+0xbb/0xd0
  Allocated by task ...:
   usb_string_copy+0xed/0x110
   gadget_language_product_store+0x22/0x60
  Freed by task ...:
   gadget_language_attr_release+0x36/0x60
   config_item_put+...
   configfs_rmdir+0x42f/0x5b0

== Reproduction ==

acm gadget on dummy_hcd (CONFIG_USB_DUMMY_HCD, CONFIG_USB_CONFIGFS,
CONFIG_USB_CONFIGFS_ACM), KASAN:

  mkdir g1; echo ... > g1/idVendor; echo ... > g1/idProduct
  mkdir g1/strings/0x409
  echo ... > g1/strings/0x409/{manufacturer,product,serialnumber}
  mkdir g1/functions/acm.0
  mkdir g1/configs/c.1
  ln -s g1/functions/acm.0 g1/configs/c.1/
  # race:
  echo <udc> > g1/UDC        &      # bind: walks the string lists
  rmdir g1/strings/0x409             # frees the language

UAF #1 fires during the bind walk; UAF #2 fires on the next host string
descriptor request after the language is freed under the bound gadget.
(I widened the two windows with a debug mdelay to make them
deterministic; without it they are narrow races.)

UAF #1 is a narrow race (bind vs. rmdir); UAF #2 does not even need a
race: removing a language under a bound gadget is enough, the next host
string request then reads freed memory.