usb: gadget: inode: fix use-after-free of struct ep_data
PATCH v2 1/1 usb: gadget: inode: fix use-after-free of struct ep_data
gadgetfs_unbind() -> destroy_ep_files() frees each struct ep_data via put_ep(), but the inode still points at that ep_data through ->i_private with no reference of its own. A concurrent openat() on an endpoint file can therefore reach ep_open() and dereference the freed ep_data (the mutex fast path), even while the inode outlives it.
Give the inode its own reference: take get_ep() when the endpoint inode is created and drop it again from a new gadgetfs_evict_inode(), so an ep_data can never be freed while any inode still points at it. This is also safe against dcache pruning, since the inode reference keeps the count >= 1 until the endpoint is removed from dev->epfiles.
The inode now holds ep_data for its whole lifetime, so an open() racing with an unbind never dereferences freed ep_data.
---
Changes in v2:
- v1: https://lore.kernel.org/all/[email protected]/
- Added further explanation in commit message.
drivers/usb/gadget/legacy/inode.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
drivers/usb/gadget/legacy/inode.c +13 -0
@@ -1626,6 +1626,8 @@ static int activate_ep_files (struct dev_data *dev)if (!data->req) goto enomem1; +/* The inode keeps this ep_data alive via ->i_private. */ +get_ep (data); err =gadgetfs_create_file (dev->sb, data->name, data, &ep_io_operations);if (err)
@@ -2015,9 +2017,20 @@ static int gadgetfs_create_file (struct super_block *sb, char const *name,return 0 ; } +static void gadgetfs_evict_inode (struct inode *inode) +{ +/* EP inodes hold the reference on their ep_data via ->i_private. */ +if (inode->i_fop == &ep_io_operations) +put_ep (inode->i_private); + +truncate_inode_pages_final (&inode->i_data); +clear_inode (inode); +} +static const struct super_operations gadget_fs_operations = { .statfs = simple_statfs, .drop_inode = inode_just_drop, + .evict_inode = gadgetfs_evict_inode, }; static int
Signature · 1 line
2.51.0