There is a use-after-free issue, if access udc_name in function gadget_dev_desc_UDC_store after another context free udc_name in function unregister_gadget.
Contex 1: gadget_dev_desc_UDC_store()->unregister_gadget()-> free udc_name->set udc_name to NULL
Contex 2: gadget_dev_desc_UDC_show()-> access udc_name
Call trace: dump_backtrace+0x0/0x340 show_stack+0x14/0x1c dump_stack+0xe4/0x134 print_address_description+0x78/0x478 __kasan_report+0x270/0x2ec kasan_report+0x10/0x18 __asan_report_load1_noabort+0x18/0x20 string+0xf4/0x138 vsnprintf+0x428/0x14d0 sprintf+0xe4/0x12c gadget_dev_desc_UDC_show+0x54/0x64 configfs_read_file+0x210/0x3a0 __vfs_read+0xf0/0x49c vfs_read+0x130/0x2b4 SyS_read+0x114/0x208 el0_svc_naked+0x34/0x38
Add mutex_lock to protect this kind of scenario.
Signed-off-by: Eddie Hung eddie.hung@mediatek.com Signed-off-by: Macpaul Lin macpaul.lin@mediatek.com Cc: stable@vger.kernel.org --- drivers/usb/gadget/configfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 9dc06a4e1b30..21110b2865b9 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -221,9 +221,16 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct config_item *item,
static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char *page) { - char *udc_name = to_gadget_info(item)->composite.gadget_driver.udc_name; + struct gadget_info *gi = to_gadget_info(item); + char *udc_name; + int ret; + + mutex_lock(&gi->lock); + udc_name = gi->composite.gadget_driver.udc_name; + ret = sprintf(page, "%s\n", udc_name ?: ""); + mutex_unlock(&gi->lock);
- return sprintf(page, "%s\n", udc_name ?: ""); + return ret; }
static int unregister_gadget(struct gadget_info *gi)
On 20-07-16 14:41:06, Macpaul Lin wrote:
There is a use-after-free issue, if access udc_name in function gadget_dev_desc_UDC_store after another context free udc_name in function unregister_gadget.
Contex 1:
%s/contex/context
gadget_dev_desc_UDC_store()->unregister_gadget()-> free udc_name->set udc_name to NULL
Contex 2:
The same, otherwise:
Reviewed-by: Peter Chen peter.chen@nxp.com
Peter
gadget_dev_desc_UDC_show()-> access udc_name
Call trace: dump_backtrace+0x0/0x340 show_stack+0x14/0x1c dump_stack+0xe4/0x134 print_address_description+0x78/0x478 __kasan_report+0x270/0x2ec kasan_report+0x10/0x18 __asan_report_load1_noabort+0x18/0x20 string+0xf4/0x138 vsnprintf+0x428/0x14d0 sprintf+0xe4/0x12c gadget_dev_desc_UDC_show+0x54/0x64 configfs_read_file+0x210/0x3a0 __vfs_read+0xf0/0x49c vfs_read+0x130/0x2b4 SyS_read+0x114/0x208 el0_svc_naked+0x34/0x38
Add mutex_lock to protect this kind of scenario.
Signed-off-by: Eddie Hung eddie.hung@mediatek.com Signed-off-by: Macpaul Lin macpaul.lin@mediatek.com Cc: stable@vger.kernel.org
drivers/usb/gadget/configfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 9dc06a4e1b30..21110b2865b9 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -221,9 +221,16 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct config_item *item, static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char *page) {
- char *udc_name = to_gadget_info(item)->composite.gadget_driver.udc_name;
- struct gadget_info *gi = to_gadget_info(item);
- char *udc_name;
- int ret;
- mutex_lock(&gi->lock);
- udc_name = gi->composite.gadget_driver.udc_name;
- ret = sprintf(page, "%s\n", udc_name ?: "");
- mutex_unlock(&gi->lock);
- return sprintf(page, "%s\n", udc_name ?: "");
- return ret;
} static int unregister_gadget(struct gadget_info *gi) -- 2.18.0
From: Eddie Hung eddie.hung@mediatek.com
There is a use-after-free issue, if access udc_name in function gadget_dev_desc_UDC_store after another context free udc_name in function unregister_gadget.
Context 1: gadget_dev_desc_UDC_store()->unregister_gadget()-> free udc_name->set udc_name to NULL
Context 2: gadget_dev_desc_UDC_show()-> access udc_name
Call trace: dump_backtrace+0x0/0x340 show_stack+0x14/0x1c dump_stack+0xe4/0x134 print_address_description+0x78/0x478 __kasan_report+0x270/0x2ec kasan_report+0x10/0x18 __asan_report_load1_noabort+0x18/0x20 string+0xf4/0x138 vsnprintf+0x428/0x14d0 sprintf+0xe4/0x12c gadget_dev_desc_UDC_show+0x54/0x64 configfs_read_file+0x210/0x3a0 __vfs_read+0xf0/0x49c vfs_read+0x130/0x2b4 SyS_read+0x114/0x208 el0_svc_naked+0x34/0x38
Add mutex_lock to protect this kind of scenario.
Signed-off-by: Eddie Hung eddie.hung@mediatek.com Signed-off-by: Macpaul Lin macpaul.lin@mediatek.com Reviewed-by: Peter Chen peter.chen@nxp.com Cc: stable@vger.kernel.org --- Changes for v2: - Fix typo %s/contex/context, Thanks Peter.
drivers/usb/gadget/configfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 9dc06a4e1b30..21110b2865b9 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -221,9 +221,16 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct config_item *item,
static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char *page) { - char *udc_name = to_gadget_info(item)->composite.gadget_driver.udc_name; + struct gadget_info *gi = to_gadget_info(item); + char *udc_name; + int ret; + + mutex_lock(&gi->lock); + udc_name = gi->composite.gadget_driver.udc_name; + ret = sprintf(page, "%s\n", udc_name ?: ""); + mutex_unlock(&gi->lock);
- return sprintf(page, "%s\n", udc_name ?: ""); + return ret; }
static int unregister_gadget(struct gadget_info *gi)
On Sat, 2020-07-18 at 10:45 +0800, Macpaul Lin wrote:
From: Eddie Hung eddie.hung@mediatek.com
Well, it's strange, I simply replaced the uploader's name to my colleague, git send-email pop up this line automatically.
Shouldn't I do that kind of change. It did not happened before. Do I need to change it back and update patch v3?
There is a use-after-free issue, if access udc_name in function gadget_dev_desc_UDC_store after another context free udc_name in function unregister_gadget.
Context 1: gadget_dev_desc_UDC_store()->unregister_gadget()-> free udc_name->set udc_name to NULL
Context 2: gadget_dev_desc_UDC_show()-> access udc_name
Call trace: dump_backtrace+0x0/0x340 show_stack+0x14/0x1c dump_stack+0xe4/0x134 print_address_description+0x78/0x478 __kasan_report+0x270/0x2ec kasan_report+0x10/0x18 __asan_report_load1_noabort+0x18/0x20 string+0xf4/0x138 vsnprintf+0x428/0x14d0 sprintf+0xe4/0x12c gadget_dev_desc_UDC_show+0x54/0x64 configfs_read_file+0x210/0x3a0 __vfs_read+0xf0/0x49c vfs_read+0x130/0x2b4 SyS_read+0x114/0x208 el0_svc_naked+0x34/0x38
Add mutex_lock to protect this kind of scenario.
Signed-off-by: Eddie Hung eddie.hung@mediatek.com Signed-off-by: Macpaul Lin macpaul.lin@mediatek.com Reviewed-by: Peter Chen peter.chen@nxp.com Cc: stable@vger.kernel.org
Changes for v2:
- Fix typo %s/contex/context, Thanks Peter.
drivers/usb/gadget/configfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
Thanks. Macpaul Lin
On Sat, Jul 18, 2020 at 10:58:53AM +0800, Macpaul Lin wrote:
On Sat, 2020-07-18 at 10:45 +0800, Macpaul Lin wrote:
From: Eddie Hung eddie.hung@mediatek.com
Well, it's strange, I simply replaced the uploader's name to my colleague, git send-email pop up this line automatically.
Shouldn't I do that kind of change. It did not happened before. Do I need to change it back and update patch v3?
Who is the real author of this, Eddie or you? If Eddie, this is correct, if you, it is not.
thanks,
greg k-h
On Tue, 2020-07-21 at 13:33 +0200, Greg Kroah-Hartman wrote:
On Sat, Jul 18, 2020 at 10:58:53AM +0800, Macpaul Lin wrote:
On Sat, 2020-07-18 at 10:45 +0800, Macpaul Lin wrote:
From: Eddie Hung eddie.hung@mediatek.com
Well, it's strange, I simply replaced the uploader's name to my colleague, git send-email pop up this line automatically.
Shouldn't I do that kind of change. It did not happened before. Do I need to change it back and update patch v3?
Who is the real author of this, Eddie or you? If Eddie, this is correct, if you, it is not.
thanks,
greg k-h
It is Eddie! I just changed the uploader to the correct author from my working tree! Thanks!
Regards, Macpaul Lin
On Sat, 2020-07-18 at 10:45 +0800, Macpaul Lin wrote:
From: Eddie Hung eddie.hung@mediatek.com There is a use-after-free issue, if access udc_name in function gadget_dev_desc_UDC_store after another context free udc_name in function unregister_gadget.
Context 1: gadget_dev_desc_UDC_store()->unregister_gadget()-> free udc_name->set udc_name to NULL
Context 2: gadget_dev_desc_UDC_show()-> access udc_name
Call trace: dump_backtrace+0x0/0x340 show_stack+0x14/0x1c dump_stack+0xe4/0x134 print_address_description+0x78/0x478 __kasan_report+0x270/0x2ec kasan_report+0x10/0x18 __asan_report_load1_noabort+0x18/0x20 string+0xf4/0x138 vsnprintf+0x428/0x14d0 sprintf+0xe4/0x12c gadget_dev_desc_UDC_show+0x54/0x64 configfs_read_file+0x210/0x3a0 __vfs_read+0xf0/0x49c vfs_read+0x130/0x2b4 SyS_read+0x114/0x208 el0_svc_naked+0x34/0x38
Add mutex_lock to protect this kind of scenario.
Signed-off-by: Eddie Hung eddie.hung@mediatek.com Signed-off-by: Macpaul Lin macpaul.lin@mediatek.com Reviewed-by: Peter Chen peter.chen@nxp.com Cc: stable@vger.kernel.org
Changes for v2:
- Fix typo %s/contex/context, Thanks Peter.
drivers/usb/gadget/configfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 9dc06a4e1b30..21110b2865b9 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -221,9 +221,16 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct config_item *item, static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char *page) {
- char *udc_name = to_gadget_info(item)->composite.gadget_driver.udc_name;
- struct gadget_info *gi = to_gadget_info(item);
- char *udc_name;
- int ret;
- mutex_lock(&gi->lock);
- udc_name = gi->composite.gadget_driver.udc_name;
- ret = sprintf(page, "%s\n", udc_name ?: "");
- mutex_unlock(&gi->lock);
- return sprintf(page, "%s\n", udc_name ?: "");
- return ret;
} static int unregister_gadget(struct gadget_info *gi)
Just want to remind we have a fix here for usb/gadget/configfs.c. If the patch need to be further revised, please let us know.
Thanks! Macpaul Lin
Hi,
Macpaul Lin macpaul.lin@mediatek.com writes:
From: Eddie Hung eddie.hung@mediatek.com
There is a use-after-free issue, if access udc_name in function gadget_dev_desc_UDC_store after another context free udc_name in function unregister_gadget.
Context 1: gadget_dev_desc_UDC_store()->unregister_gadget()-> free udc_name->set udc_name to NULL
Context 2: gadget_dev_desc_UDC_show()-> access udc_name
Call trace: dump_backtrace+0x0/0x340 show_stack+0x14/0x1c dump_stack+0xe4/0x134 print_address_description+0x78/0x478 __kasan_report+0x270/0x2ec kasan_report+0x10/0x18 __asan_report_load1_noabort+0x18/0x20 string+0xf4/0x138 vsnprintf+0x428/0x14d0 sprintf+0xe4/0x12c gadget_dev_desc_UDC_show+0x54/0x64 configfs_read_file+0x210/0x3a0 __vfs_read+0xf0/0x49c vfs_read+0x130/0x2b4 SyS_read+0x114/0x208 el0_svc_naked+0x34/0x38
Add mutex_lock to protect this kind of scenario.
Signed-off-by: Eddie Hung eddie.hung@mediatek.com Signed-off-by: Macpaul Lin macpaul.lin@mediatek.com Reviewed-by: Peter Chen peter.chen@nxp.com Cc: stable@vger.kernel.org
patch doesn't apply:
$ patch -p1 --dry-run /usr/bin/patch: **** Only garbage was found in the patch input.
Please resend using git send-email and make sure your smtp server sends it as plain text, not base64.
From: Eddie Hung eddie.hung@mediatek.com
There is a use-after-free issue, if access udc_name in function gadget_dev_desc_UDC_store after another context free udc_name in function unregister_gadget.
Context 1: gadget_dev_desc_UDC_store()->unregister_gadget()-> free udc_name->set udc_name to NULL
Context 2: gadget_dev_desc_UDC_show()-> access udc_name
Call trace: dump_backtrace+0x0/0x340 show_stack+0x14/0x1c dump_stack+0xe4/0x134 print_address_description+0x78/0x478 __kasan_report+0x270/0x2ec kasan_report+0x10/0x18 __asan_report_load1_noabort+0x18/0x20 string+0xf4/0x138 vsnprintf+0x428/0x14d0 sprintf+0xe4/0x12c gadget_dev_desc_UDC_show+0x54/0x64 configfs_read_file+0x210/0x3a0 __vfs_read+0xf0/0x49c vfs_read+0x130/0x2b4 SyS_read+0x114/0x208 el0_svc_naked+0x34/0x38
Add mutex_lock to protect this kind of scenario.
Signed-off-by: Eddie Hung eddie.hung@mediatek.com Signed-off-by: Macpaul Lin macpaul.lin@mediatek.com Reviewed-by: Peter Chen peter.chen@nxp.com Cc: stable@vger.kernel.org --- Changes for v2: - Fix typo %s/contex/context, Thanks Peter.
drivers/usb/gadget/configfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index cbff3b02840d..8501b27f3c95 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -230,9 +230,16 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct config_item *item,
static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char *page) { - char *udc_name = to_gadget_info(item)->composite.gadget_driver.udc_name; + struct gadget_info *gi = to_gadget_info(item); + char *udc_name; + int ret; + + mutex_lock(&gi->lock); + udc_name = gi->composite.gadget_driver.udc_name; + ret = sprintf(page, "%s\n", udc_name ?: ""); + mutex_unlock(&gi->lock);
- return sprintf(page, "%s\n", udc_name ?: ""); + return ret; }
static int unregister_gadget(struct gadget_info *gi)
From: Eddie Hung eddie.hung@mediatek.com
There is a use-after-free issue, if access udc_name in function gadget_dev_desc_UDC_store after another context free udc_name in function unregister_gadget.
Context 1: gadget_dev_desc_UDC_store()->unregister_gadget()-> free udc_name->set udc_name to NULL
Context 2: gadget_dev_desc_UDC_show()-> access udc_name
Call trace: dump_backtrace+0x0/0x340 show_stack+0x14/0x1c dump_stack+0xe4/0x134 print_address_description+0x78/0x478 __kasan_report+0x270/0x2ec kasan_report+0x10/0x18 __asan_report_load1_noabort+0x18/0x20 string+0xf4/0x138 vsnprintf+0x428/0x14d0 sprintf+0xe4/0x12c gadget_dev_desc_UDC_show+0x54/0x64 configfs_read_file+0x210/0x3a0 __vfs_read+0xf0/0x49c vfs_read+0x130/0x2b4 SyS_read+0x114/0x208 el0_svc_naked+0x34/0x38
Add mutex_lock to protect this kind of scenario.
Signed-off-by: Eddie Hung eddie.hung@mediatek.com Signed-off-by: Macpaul Lin macpaul.lin@mediatek.com Reviewed-by: Peter Chen peter.chen@nxp.com Cc: stable@vger.kernel.org --- Changes for v2: - Fix typo %s/contex/context, Thanks Peter.
drivers/usb/gadget/configfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 56051bb..d9743f4 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -221,9 +221,16 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct config_item *item,
static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char *page) { - char *udc_name = to_gadget_info(item)->composite.gadget_driver.udc_name; + struct gadget_info *gi = to_gadget_info(item); + char *udc_name; + int ret; + + mutex_lock(&gi->lock); + udc_name = gi->composite.gadget_driver.udc_name; + ret = sprintf(page, "%s\n", udc_name ?: ""); + mutex_unlock(&gi->lock);
- return sprintf(page, "%s\n", udc_name ?: ""); + return ret; }
static int unregister_gadget(struct gadget_info *gi)
linux-stable-mirror@lists.linaro.org
-
Felipe Balbi -
Greg Kroah-Hartman -
Macpaul Lin -
Macpaul Lin -
Peter Chen