* [PATCH v4] usb: gadget: configfs: Fix KASAN use-after-free [not found] <1484647168-30135-1-git-send-email-jilin@nvidia.com> @ 2021-03-11 6:42 ` Macpaul Lin 2021-03-11 6:53 ` Macpaul Lin 0 siblings, 1 reply; 3+ messages in thread From: Macpaul Lin @ 2021-03-11 6:42 UTC (permalink / raw) To: Jim Lin, Thadeu Lima de Souza Cascardo, Felipe Balbi, Greg Kroah-Hartman, Matthias Brugger, linux-usb, linux-kernel, linux-arm-kernel, linux-mediatek Cc: Ainge Hsu, Eddie Hung, Kuohong Wang, Mediatek WSD Upstream, Macpaul Lin, Macpaul Lin, stable From: Jim Lin <jilin@nvidia.com> When gadget is disconnected, running sequence is like this. . composite_disconnect . Call trace: usb_string_copy+0xd0/0x128 gadget_config_name_configuration_store+0x4 gadget_config_name_attr_store+0x40/0x50 configfs_write_file+0x198/0x1f4 vfs_write+0x100/0x220 SyS_write+0x58/0xa8 . configfs_composite_unbind . configfs_composite_bind In configfs_composite_bind, it has "cn->strings.s = cn->configuration;" When usb_string_copy is invoked. it would allocate memory, copy input string, release previous pointed memory space, and use new allocated memory. When gadget is connected, host sends down request to get information. Call trace: usb_gadget_get_string+0xec/0x168 lookup_string+0x64/0x98 composite_setup+0xa34/0x1ee8 If gadget is disconnected and connected quickly, in the failed case, cn->configuration memory has been released by usb_string_copy kfree but configfs_composite_bind hasn't been run in time to assign new allocated "cn->configuration" pointer to "cn->strings.s". When "strlen(s->s) of usb_gadget_get_string is being executed, the dangling memory is accessed, "BUG: KASAN: use-after-free" error occurs. Signed-off-by: Jim Lin <jilin@nvidia.com> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> Cc: stable@vger.kernel.org --- Changes in v2: Changes in v3: - Change commit description Changes in v4: - Fix build error and adapt patch to kernel-5.12-rc1. Replace definition "MAX_USB_STRING_WITH_NULL_LEN" with "USB_MAX_STRING_WITH_NULL_LEN". - Note: The patch v2 and v3 has been verified by Thadeu Lima de Souza Cascardo <cascardo@canonical.com> http://spinics.net/lists/kernel/msg3840792.html and Macpaul Lin <macpaul.lin@mediatek.com> on Android kernels. http://lkml.org/lkml/2020/6/11/8 - The patch is suggested to be applied to LTS versions. drivers/usb/gadget/configfs.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 0d56f33..15a607c 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -97,6 +97,8 @@ struct gadget_config_name { struct list_head list; }; +#define USB_MAX_STRING_WITH_NULL_LEN (USB_MAX_STRING_LEN+1) + static int usb_string_copy(const char *s, char **s_copy) { int ret; @@ -106,12 +108,16 @@ static int usb_string_copy(const char *s, char **s_copy) if (ret > USB_MAX_STRING_LEN) return -EOVERFLOW; - str = kstrdup(s, GFP_KERNEL); - if (!str) - return -ENOMEM; + if (copy) { + str = copy; + } else { + str = kmalloc(USB_MAX_STRING_WITH_NULL_LEN, GFP_KERNEL); + if (!str) + return -ENOMEM; + } + strcpy(str, s); if (str[ret - 1] == '\n') str[ret - 1] = '\0'; - kfree(copy); *s_copy = str; return 0; } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v4] usb: gadget: configfs: Fix KASAN use-after-free 2021-03-11 6:42 ` [PATCH v4] usb: gadget: configfs: Fix KASAN use-after-free Macpaul Lin @ 2021-03-11 6:53 ` Macpaul Lin 2021-03-11 10:52 ` Thadeu Lima de Souza Cascardo 0 siblings, 1 reply; 3+ messages in thread From: Macpaul Lin @ 2021-03-11 6:53 UTC (permalink / raw) To: Jim Lin, Thadeu Lima de Souza Cascardo Cc: Thadeu Lima de Souza Cascardo, Felipe Balbi, Greg Kroah-Hartman, Matthias Brugger, linux-usb, linux-kernel, linux-arm-kernel, linux-mediatek, Ainge Hsu, Eddie Hung, Kuohong Wang, Mediatek WSD Upstream, Macpaul Lin, stable On Thu, 2021-03-11 at 14:42 +0800, Macpaul Lin wrote: > From: Jim Lin <jilin@nvidia.com> > > When gadget is disconnected, running sequence is like this. > . composite_disconnect > . Call trace: > usb_string_copy+0xd0/0x128 > gadget_config_name_configuration_store+0x4 > gadget_config_name_attr_store+0x40/0x50 > configfs_write_file+0x198/0x1f4 > vfs_write+0x100/0x220 > SyS_write+0x58/0xa8 > . configfs_composite_unbind > . configfs_composite_bind > > In configfs_composite_bind, it has > "cn->strings.s = cn->configuration;" > > When usb_string_copy is invoked. it would > allocate memory, copy input string, release previous pointed memory space, > and use new allocated memory. > > When gadget is connected, host sends down request to get information. > Call trace: > usb_gadget_get_string+0xec/0x168 > lookup_string+0x64/0x98 > composite_setup+0xa34/0x1ee8 > > If gadget is disconnected and connected quickly, in the failed case, > cn->configuration memory has been released by usb_string_copy kfree but > configfs_composite_bind hasn't been run in time to assign new allocated > "cn->configuration" pointer to "cn->strings.s". > > When "strlen(s->s) of usb_gadget_get_string is being executed, the dangling > memory is accessed, "BUG: KASAN: use-after-free" error occurs. > > Signed-off-by: Jim Lin <jilin@nvidia.com> > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > Cc: stable@vger.kernel.org > --- > Changes in v2: > Changes in v3: > - Change commit description > Changes in v4: > - Fix build error and adapt patch to kernel-5.12-rc1. > Replace definition "MAX_USB_STRING_WITH_NULL_LEN" with > "USB_MAX_STRING_WITH_NULL_LEN". > - Note: The patch v2 and v3 has been verified by > Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > http://spinics.net/lists/kernel/msg3840792.html Dear Cascardo, Would you please help to confirm if you've tested it on Linux PC, Chrome OS, or an Android OS? Thanks! Macpaul Lin > and > Macpaul Lin <macpaul.lin@mediatek.com> on Android kernels. > http://lkml.org/lkml/2020/6/11/8 > - The patch is suggested to be applied to LTS versions. > > drivers/usb/gadget/configfs.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c > index 0d56f33..15a607c 100644 > --- a/drivers/usb/gadget/configfs.c > +++ b/drivers/usb/gadget/configfs.c > @@ -97,6 +97,8 @@ struct gadget_config_name { > struct list_head list; > }; > > +#define USB_MAX_STRING_WITH_NULL_LEN (USB_MAX_STRING_LEN+1) > + > static int usb_string_copy(const char *s, char **s_copy) > { > int ret; > @@ -106,12 +108,16 @@ static int usb_string_copy(const char *s, char **s_copy) > if (ret > USB_MAX_STRING_LEN) > return -EOVERFLOW; > > - str = kstrdup(s, GFP_KERNEL); > - if (!str) > - return -ENOMEM; > + if (copy) { > + str = copy; > + } else { > + str = kmalloc(USB_MAX_STRING_WITH_NULL_LEN, GFP_KERNEL); > + if (!str) > + return -ENOMEM; > + } > + strcpy(str, s); > if (str[ret - 1] == '\n') > str[ret - 1] = '\0'; > - kfree(copy); > *s_copy = str; > return 0; > } ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] usb: gadget: configfs: Fix KASAN use-after-free 2021-03-11 6:53 ` Macpaul Lin @ 2021-03-11 10:52 ` Thadeu Lima de Souza Cascardo 0 siblings, 0 replies; 3+ messages in thread From: Thadeu Lima de Souza Cascardo @ 2021-03-11 10:52 UTC (permalink / raw) To: Macpaul Lin Cc: Jim Lin, Felipe Balbi, Greg Kroah-Hartman, Matthias Brugger, linux-usb, linux-kernel, linux-arm-kernel, linux-mediatek, Ainge Hsu, Eddie Hung, Kuohong Wang, Mediatek WSD Upstream, Macpaul Lin, stable On Thu, Mar 11, 2021 at 02:53:52PM +0800, Macpaul Lin wrote: > On Thu, 2021-03-11 at 14:42 +0800, Macpaul Lin wrote: > > From: Jim Lin <jilin@nvidia.com> > > > > When gadget is disconnected, running sequence is like this. > > . composite_disconnect > > . Call trace: > > usb_string_copy+0xd0/0x128 > > gadget_config_name_configuration_store+0x4 > > gadget_config_name_attr_store+0x40/0x50 > > configfs_write_file+0x198/0x1f4 > > vfs_write+0x100/0x220 > > SyS_write+0x58/0xa8 > > . configfs_composite_unbind > > . configfs_composite_bind > > > > In configfs_composite_bind, it has > > "cn->strings.s = cn->configuration;" > > > > When usb_string_copy is invoked. it would > > allocate memory, copy input string, release previous pointed memory space, > > and use new allocated memory. > > > > When gadget is connected, host sends down request to get information. > > Call trace: > > usb_gadget_get_string+0xec/0x168 > > lookup_string+0x64/0x98 > > composite_setup+0xa34/0x1ee8 > > > > If gadget is disconnected and connected quickly, in the failed case, > > cn->configuration memory has been released by usb_string_copy kfree but > > configfs_composite_bind hasn't been run in time to assign new allocated > > "cn->configuration" pointer to "cn->strings.s". > > > > When "strlen(s->s) of usb_gadget_get_string is being executed, the dangling > > memory is accessed, "BUG: KASAN: use-after-free" error occurs. > > > > Signed-off-by: Jim Lin <jilin@nvidia.com> > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > > Cc: stable@vger.kernel.org > > --- > > Changes in v2: > > Changes in v3: > > - Change commit description > > Changes in v4: > > - Fix build error and adapt patch to kernel-5.12-rc1. > > Replace definition "MAX_USB_STRING_WITH_NULL_LEN" with > > "USB_MAX_STRING_WITH_NULL_LEN". > > - Note: The patch v2 and v3 has been verified by > > Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > > http://spinics.net/lists/kernel/msg3840792.html > > Dear Cascardo, > > Would you please help to confirm if you've tested it on Linux PC, > Chrome OS, or an Android OS? I tested v3 on Ubuntu GNU/Linux. I will test v4. Cascardo. > > Thanks! > Macpaul Lin > > > and > > Macpaul Lin <macpaul.lin@mediatek.com> on Android kernels. > > http://lkml.org/lkml/2020/6/11/8 > > - The patch is suggested to be applied to LTS versions. > > > > drivers/usb/gadget/configfs.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c > > index 0d56f33..15a607c 100644 > > --- a/drivers/usb/gadget/configfs.c > > +++ b/drivers/usb/gadget/configfs.c > > @@ -97,6 +97,8 @@ struct gadget_config_name { > > struct list_head list; > > }; > > > > +#define USB_MAX_STRING_WITH_NULL_LEN (USB_MAX_STRING_LEN+1) > > + > > static int usb_string_copy(const char *s, char **s_copy) > > { > > int ret; > > @@ -106,12 +108,16 @@ static int usb_string_copy(const char *s, char **s_copy) > > if (ret > USB_MAX_STRING_LEN) > > return -EOVERFLOW; > > > > - str = kstrdup(s, GFP_KERNEL); > > - if (!str) > > - return -ENOMEM; > > + if (copy) { > > + str = copy; > > + } else { > > + str = kmalloc(USB_MAX_STRING_WITH_NULL_LEN, GFP_KERNEL); > > + if (!str) > > + return -ENOMEM; > > + } > > + strcpy(str, s); > > if (str[ret - 1] == '\n') > > str[ret - 1] = '\0'; > > - kfree(copy); > > *s_copy = str; > > return 0; > > } > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-11 10:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1484647168-30135-1-git-send-email-jilin@nvidia.com>
2021-03-11 6:42 ` [PATCH v4] usb: gadget: configfs: Fix KASAN use-after-free Macpaul Lin
2021-03-11 6:53 ` Macpaul Lin
2021-03-11 10:52 ` Thadeu Lima de Souza Cascardo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox