* [PATCH v4 0/2] usb: gadget: use after free in dev_config @ 2021-12-31 17:21 Hangyu Hua 2021-12-31 17:21 ` [PATCH v4 1/2] usb: gadget: don't release an existing dev->buf Hangyu Hua 2021-12-31 17:21 ` [PATCH v4 2/2] usb: gadget: clear related members when goto fail Hangyu Hua 0 siblings, 2 replies; 5+ messages in thread From: Hangyu Hua @ 2021-12-31 17:21 UTC (permalink / raw) To: balbi, gregkh, axboe, dan.carpenter, jj251510319013, stern Cc: linux-usb, linux-kernel, Hangyu Hua There are two bugs: dev->buf does not need to be released if it already exists before executing dev_config. dev->config and dev->hs_config and dev->dev need to be cleaned if dev_config fails to avoid UAF. v2: 1. break one patch up into two separate patches. 2. use "fail:" to clear all members. v3: fix a mistake in [PATCH v3 2/2] v4: avoid multiple unnecessary statement labels in [PATCH v4 2/2] Hangyu Hua (2): usb: gadget: don't release an existing dev->buf usb: gadget: clear related members when goto fail drivers/usb/gadget/legacy/inode.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 1/2] usb: gadget: don't release an existing dev->buf 2021-12-31 17:21 [PATCH v4 0/2] usb: gadget: use after free in dev_config Hangyu Hua @ 2021-12-31 17:21 ` Hangyu Hua 2021-12-31 17:34 ` Alan Stern 2021-12-31 17:21 ` [PATCH v4 2/2] usb: gadget: clear related members when goto fail Hangyu Hua 1 sibling, 1 reply; 5+ messages in thread From: Hangyu Hua @ 2021-12-31 17:21 UTC (permalink / raw) To: balbi, gregkh, axboe, dan.carpenter, jj251510319013, stern Cc: linux-usb, linux-kernel, Hangyu Hua dev->buf does not need to be released if it already exists before executing dev_config. Signed-off-by: Hangyu Hua <hbh25y@gmail.com> --- drivers/usb/gadget/legacy/inode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c index 3b58f4fc0a80..eaad03c0252f 100644 --- a/drivers/usb/gadget/legacy/inode.c +++ b/drivers/usb/gadget/legacy/inode.c @@ -1826,8 +1826,9 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) spin_lock_irq (&dev->lock); value = -EINVAL; if (dev->buf) { + spin_unlock_irq(&dev->lock); kfree(kbuf); - goto fail; + return value; } dev->buf = kbuf; -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] usb: gadget: don't release an existing dev->buf 2021-12-31 17:21 ` [PATCH v4 1/2] usb: gadget: don't release an existing dev->buf Hangyu Hua @ 2021-12-31 17:34 ` Alan Stern 0 siblings, 0 replies; 5+ messages in thread From: Alan Stern @ 2021-12-31 17:34 UTC (permalink / raw) To: Hangyu Hua Cc: balbi, gregkh, axboe, dan.carpenter, jj251510319013, linux-usb, linux-kernel On Sat, Jan 01, 2022 at 01:21:37AM +0800, Hangyu Hua wrote: > dev->buf does not need to be released if it already exists before > executing dev_config. > > Signed-off-by: Hangyu Hua <hbh25y@gmail.com> > --- Acked-by: Alan Stern <stern@rowland.harvard.edu> > drivers/usb/gadget/legacy/inode.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c > index 3b58f4fc0a80..eaad03c0252f 100644 > --- a/drivers/usb/gadget/legacy/inode.c > +++ b/drivers/usb/gadget/legacy/inode.c > @@ -1826,8 +1826,9 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) > spin_lock_irq (&dev->lock); > value = -EINVAL; > if (dev->buf) { > + spin_unlock_irq(&dev->lock); > kfree(kbuf); > - goto fail; > + return value; > } > dev->buf = kbuf; > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 2/2] usb: gadget: clear related members when goto fail 2021-12-31 17:21 [PATCH v4 0/2] usb: gadget: use after free in dev_config Hangyu Hua 2021-12-31 17:21 ` [PATCH v4 1/2] usb: gadget: don't release an existing dev->buf Hangyu Hua @ 2021-12-31 17:21 ` Hangyu Hua 2021-12-31 17:35 ` Alan Stern 1 sibling, 1 reply; 5+ messages in thread From: Hangyu Hua @ 2021-12-31 17:21 UTC (permalink / raw) To: balbi, gregkh, axboe, dan.carpenter, jj251510319013, stern Cc: linux-usb, linux-kernel, Hangyu Hua dev->config and dev->hs_config and dev->dev need to be cleaned if dev_config fails to avoid UAF. Signed-off-by: Hangyu Hua <hbh25y@gmail.com> --- drivers/usb/gadget/legacy/inode.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c index eaad03c0252f..25c8809e0a38 100644 --- a/drivers/usb/gadget/legacy/inode.c +++ b/drivers/usb/gadget/legacy/inode.c @@ -1875,8 +1875,8 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) value = usb_gadget_probe_driver(&gadgetfs_driver); if (value != 0) { - kfree (dev->buf); - dev->buf = NULL; + spin_lock_irq(&dev->lock); + goto fail; } else { /* at this point "good" hardware has for the first time * let the USB the host see us. alternatively, if users @@ -1893,6 +1893,9 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) return value; fail: + dev->config = NULL; + dev->hs_config = NULL; + dev->dev = NULL; spin_unlock_irq (&dev->lock); pr_debug ("%s: %s fail %zd, %p\n", shortname, __func__, value, dev); kfree (dev->buf); -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 2/2] usb: gadget: clear related members when goto fail 2021-12-31 17:21 ` [PATCH v4 2/2] usb: gadget: clear related members when goto fail Hangyu Hua @ 2021-12-31 17:35 ` Alan Stern 0 siblings, 0 replies; 5+ messages in thread From: Alan Stern @ 2021-12-31 17:35 UTC (permalink / raw) To: Hangyu Hua Cc: balbi, gregkh, axboe, dan.carpenter, jj251510319013, linux-usb, linux-kernel On Sat, Jan 01, 2022 at 01:21:38AM +0800, Hangyu Hua wrote: > dev->config and dev->hs_config and dev->dev need to be cleaned if > dev_config fails to avoid UAF. > > Signed-off-by: Hangyu Hua <hbh25y@gmail.com> > --- Acked-by: Alan Stern <stern@rowland.harvard.edu> > drivers/usb/gadget/legacy/inode.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c > index eaad03c0252f..25c8809e0a38 100644 > --- a/drivers/usb/gadget/legacy/inode.c > +++ b/drivers/usb/gadget/legacy/inode.c > @@ -1875,8 +1875,8 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) > > value = usb_gadget_probe_driver(&gadgetfs_driver); > if (value != 0) { > - kfree (dev->buf); > - dev->buf = NULL; > + spin_lock_irq(&dev->lock); > + goto fail; > } else { > /* at this point "good" hardware has for the first time > * let the USB the host see us. alternatively, if users > @@ -1893,6 +1893,9 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) > return value; > > fail: > + dev->config = NULL; > + dev->hs_config = NULL; > + dev->dev = NULL; > spin_unlock_irq (&dev->lock); > pr_debug ("%s: %s fail %zd, %p\n", shortname, __func__, value, dev); > kfree (dev->buf); > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-31 17:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-12-31 17:21 [PATCH v4 0/2] usb: gadget: use after free in dev_config Hangyu Hua 2021-12-31 17:21 ` [PATCH v4 1/2] usb: gadget: don't release an existing dev->buf Hangyu Hua 2021-12-31 17:34 ` Alan Stern 2021-12-31 17:21 ` [PATCH v4 2/2] usb: gadget: clear related members when goto fail Hangyu Hua 2021-12-31 17:35 ` Alan Stern
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox