* [PATCH] HID: fix a crash in hid_debug_events_release @ 2023-10-23 9:35 Charles Yi 2023-10-23 15:03 ` Rahul Rameshbabu 0 siblings, 1 reply; 4+ messages in thread From: Charles Yi @ 2023-10-23 9:35 UTC (permalink / raw) To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Charles Yi hid_debug_events_release() access released memory by hid_device_release(). This is fixed by the patch. Signed-off-by: Charles Yi <be286@163.com> --- drivers/hid/hid-core.c | 12 ++++++++++-- drivers/hid/hid-debug.c | 3 +++ include/linux/hid.h | 3 +++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 8992e3c1e769..e0181218ad85 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -702,15 +702,22 @@ static void hid_close_report(struct hid_device *device) * Free a device structure, all reports, and all fields. */ -static void hid_device_release(struct device *dev) +void hiddev_free(struct kref *ref) { - struct hid_device *hid = to_hid_device(dev); + struct hid_device *hid = container_of(ref, struct hid_device, ref); hid_close_report(hid); kfree(hid->dev_rdesc); kfree(hid); } +static void hid_device_release(struct device *dev) +{ + struct hid_device *hid = to_hid_device(dev); + + kref_put(&hid->ref, hiddev_free); +} + /* * Fetch a report description item from the data stream. We support long * items, though they are not used yet. @@ -2846,6 +2853,7 @@ struct hid_device *hid_allocate_device(void) spin_lock_init(&hdev->debug_list_lock); sema_init(&hdev->driver_input_lock, 1); mutex_init(&hdev->ll_open_lock); + kref_init(&hdev->ref); hid_bpf_device_init(hdev); diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index e7ef1ea107c9..7dd83ec74f8a 100644 --- a/drivers/hid/hid-debug.c +++ b/drivers/hid/hid-debug.c @@ -1135,6 +1135,7 @@ static int hid_debug_events_open(struct inode *inode, struct file *file) goto out; } list->hdev = (struct hid_device *) inode->i_private; + kref_get(&list->hdev->ref); file->private_data = list; mutex_init(&list->read_mutex); @@ -1227,6 +1228,8 @@ static int hid_debug_events_release(struct inode *inode, struct file *file) list_del(&list->node); spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags); kfifo_free(&list->hid_debug_fifo); + + kref_put(&list->hdev->ref, hiddev_free); kfree(list); return 0; diff --git a/include/linux/hid.h b/include/linux/hid.h index 964ca1f15e3f..3b08a2957229 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -679,6 +679,7 @@ struct hid_device { /* device report descriptor */ struct list_head debug_list; spinlock_t debug_list_lock; wait_queue_head_t debug_wait; + struct kref ref; unsigned int id; /* system unique id */ @@ -687,6 +688,8 @@ struct hid_device { /* device report descriptor */ #endif /* CONFIG_BPF */ }; +void hiddev_free(struct kref *ref); + #define to_hid_device(pdev) \ container_of(pdev, struct hid_device, dev) -- 2.34.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: fix a crash in hid_debug_events_release 2023-10-23 9:35 [PATCH] HID: fix a crash in hid_debug_events_release Charles Yi @ 2023-10-23 15:03 ` Rahul Rameshbabu 2023-10-24 9:49 ` be286 0 siblings, 1 reply; 4+ messages in thread From: Rahul Rameshbabu @ 2023-10-23 15:03 UTC (permalink / raw) To: Charles Yi; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel Hi Charles, On Mon, 23 Oct, 2023 17:35:00 +0800 "Charles Yi" <be286@163.com> wrote: > hid_debug_events_release() access released memory by > hid_device_release(). This is fixed by the patch. > A couple of things here. Can you add a Fixes: tag? https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes Before any v2 however, it would be nice to understand where this issue is coming from. I am wondering if it's really a core issue or rather an issue with a higher level device specific driver making use of the hid subsystem. I am having a hard time seeing how this issue occurs currently. A stack trace in a follow-up email to this one pertaining to the crash would be helpful. If you are resolving a syzbot report, a link to that report would suffice. This patch doesn't make a lot of sense to me as-is because hid_debug_events_release is about release resources related to hid debug events (at least from my current understanding). It should not be free-ing the underlying hid device/instance itself. > Signed-off-by: Charles Yi <be286@163.com> > --- > drivers/hid/hid-core.c | 12 ++++++++++-- > drivers/hid/hid-debug.c | 3 +++ > include/linux/hid.h | 3 +++ > 3 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 8992e3c1e769..e0181218ad85 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -702,15 +702,22 @@ static void hid_close_report(struct hid_device *device) > * Free a device structure, all reports, and all fields. > */ > > -static void hid_device_release(struct device *dev) > +void hiddev_free(struct kref *ref) > { > - struct hid_device *hid = to_hid_device(dev); > + struct hid_device *hid = container_of(ref, struct hid_device, ref); > > hid_close_report(hid); > kfree(hid->dev_rdesc); > kfree(hid); > } > > +static void hid_device_release(struct device *dev) > +{ > + struct hid_device *hid = to_hid_device(dev); > + > + kref_put(&hid->ref, hiddev_free); > +} > + > /* > * Fetch a report description item from the data stream. We support long > * items, though they are not used yet. > @@ -2846,6 +2853,7 @@ struct hid_device *hid_allocate_device(void) > spin_lock_init(&hdev->debug_list_lock); > sema_init(&hdev->driver_input_lock, 1); > mutex_init(&hdev->ll_open_lock); > + kref_init(&hdev->ref); > > hid_bpf_device_init(hdev); > > diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c > index e7ef1ea107c9..7dd83ec74f8a 100644 > --- a/drivers/hid/hid-debug.c > +++ b/drivers/hid/hid-debug.c > @@ -1135,6 +1135,7 @@ static int hid_debug_events_open(struct inode *inode, struct file *file) > goto out; > } > list->hdev = (struct hid_device *) inode->i_private; > + kref_get(&list->hdev->ref); > file->private_data = list; > mutex_init(&list->read_mutex); > > @@ -1227,6 +1228,8 @@ static int hid_debug_events_release(struct inode *inode, struct file *file) > list_del(&list->node); > spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags); > kfifo_free(&list->hid_debug_fifo); > + > + kref_put(&list->hdev->ref, hiddev_free); > kfree(list); > > return 0; > diff --git a/include/linux/hid.h b/include/linux/hid.h > index 964ca1f15e3f..3b08a2957229 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -679,6 +679,7 @@ struct hid_device { /* device report descriptor */ > struct list_head debug_list; > spinlock_t debug_list_lock; > wait_queue_head_t debug_wait; > + struct kref ref; > > unsigned int id; /* system unique id */ > > @@ -687,6 +688,8 @@ struct hid_device { /* device report descriptor */ > #endif /* CONFIG_BPF */ > }; > > +void hiddev_free(struct kref *ref); > + > #define to_hid_device(pdev) \ > container_of(pdev, struct hid_device, dev) -- Thank you for the patch, Rahul Rameshbabu ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re:Re: [PATCH] HID: fix a crash in hid_debug_events_release 2023-10-23 15:03 ` Rahul Rameshbabu @ 2023-10-24 9:49 ` be286 2023-10-29 16:44 ` Rahul Rameshbabu 0 siblings, 1 reply; 4+ messages in thread From: be286 @ 2023-10-24 9:49 UTC (permalink / raw) To: Rahul Rameshbabu; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel Hi Rahul, When hid_debug_events_release() was being called, in most case, hid_device_release() finish already, the memory of list->hdev freed by hid_device_release(), if list->hdev memory reallocate by others, and it's modified, zeroed, then list->hdev->debug_list_lock occasioned crash come out. The runing order: [ 258.201069] CPU: 0 PID: 203 Comm: kworker/0:2 Not tainted 5.10.110 #255 [ 258.201073] Hardware name: Rockchip RK3588 EVB4 LP4 V10 Board (DT) [ 258.201086] Workqueue: usb_hub_wq hub_event [ 258.201092] Call trace: [ 258.201100] dump_backtrace+0x0/0x1f8 [ 258.201105] show_stack+0x1c/0x2c [ 258.201112] dump_stack_lvl+0xd8/0x12c [ 258.201116] dump_stack+0x1c/0x60 [ 258.201122] hid_device_release+0x94/0xb4 [ 258.201127] device_release+0x38/0x94 [ 258.201133] kobject_cleanup+0xd0/0x174 [ 258.201137] kobject_put+0x68/0xa8 [ 258.201143] put_device+0x1c/0x2c [ 258.201146] hid_destroy_device+0x60/0x74 [ 258.201153] usbhid_disconnect+0x5c/0x90 [ 258.201157] usb_unbind_interface+0xc4/0x268 [ 258.201162] device_release_driver_internal+0x184/0x25c [ 258.201165] device_release_driver+0x1c/0x2c [ 258.201169] bus_remove_device+0xdc/0x138 [ 258.201173] device_del+0x1d0/0x3d8 [ 258.201177] usb_disable_device+0x108/0x228 [ 258.201181] usb_disconnect+0xf4/0x304 [ 258.201184] usb_disconnect+0xdc/0x304 [ 258.201188] hub_port_connect+0x88/0xa24 [ 258.201191] hub_port_connect_change+0x2d8/0x4cc [ 258.201195] port_event+0x550/0x614 [ 258.201198] hub_event+0x12c/0x494 [ 258.201203] process_one_work+0x1f4/0x490 [ 258.201207] worker_thread+0x278/0x4dc [ 258.201211] kthread+0x130/0x338 [ 258.201215] ret_from_fork+0x10/0x30 [ 259.925641] CPU: 1 PID: 2354 Comm: hidt_bridge Not tainted 5.10.110 #255 [ 259.925652] Hardware name: Rockchip RK3588 EVB4 LP4 V10 Board (DT) [ 259.925656] Call trace: [ 259.925671] dump_backtrace+0x0/0x1f8 [ 259.925676] show_stack+0x1c/0x2c [ 259.925685] dump_stack_lvl+0xd8/0x12c [ 259.925689] dump_stack+0x1c/0x60 [ 259.925697] hid_debug_events_release+0x24/0x134 [ 259.925704] full_proxy_release+0x50/0xbc [ 259.925709] __fput+0xdc/0x238 [ 259.925714] ____fput+0x14/0x24 [ 259.925720] task_work_run+0x90/0x148 [ 259.925725] do_exit+0x1bc/0x8a4 [ 259.925729] do_group_exit+0x8c/0xa4 [ 259.925734] get_signal+0x468/0x744 [ 259.925739] do_signal+0x84/0x280 [ 259.925743] do_notify_resume+0xd8/0x228 [ 259.925747] work_pending+0xc/0x3f0 The crash: [ 120.728477][ T4396] kernel BUG at lib/list_debug.c:53! [ 120.728505][ T4396] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP [ 120.739806][ T4396] Modules linked in: bcmdhd dhd_static_buf 8822cu pcie_mhi r8168 [ 120.747386][ T4396] CPU: 1 PID: 4396 Comm: hidt_bridge Not tainted 5.10.110 #257 [ 120.754771][ T4396] Hardware name: Rockchip RK3588 EVB4 LP4 V10 Board (DT) [ 120.761643][ T4396] pstate: 60400089 (nZCv daIf +PAN -UAO -TCO BTYPE=--) [ 120.768338][ T4396] pc : __list_del_entry_valid+0x98/0xac [ 120.773730][ T4396] lr : __list_del_entry_valid+0x98/0xac [ 120.779120][ T4396] sp : ffffffc01e62bb60 [ 120.783126][ T4396] x29: ffffffc01e62bb60 x28: ffffff818ce3a200 [ 120.789126][ T4396] x27: 0000000000000009 x26: 0000000000980000 [ 120.795126][ T4396] x25: ffffffc012431000 x24: ffffff802c6d4e00 [ 120.801125][ T4396] x23: ffffff8005c66f00 x22: ffffffc01183b5b8 [ 120.807125][ T4396] x21: ffffff819df2f100 x20: 0000000000000000 [ 120.813124][ T4396] x19: ffffff802c3f0700 x18: ffffffc01d2cd058 [ 120.819124][ T4396] x17: 0000000000000000 x16: 0000000000000000 [ 120.825124][ T4396] x15: 0000000000000004 x14: 0000000000003fff [ 120.831123][ T4396] x13: ffffffc012085588 x12: 0000000000000003 [ 120.837123][ T4396] x11: 00000000ffffbfff x10: 0000000000000003 [ 120.843123][ T4396] x9 : 455103d46b329300 x8 : 455103d46b329300 [ 120.849124][ T4396] x7 : 74707572726f6320 x6 : ffffffc0124b8cb5 [ 120.855124][ T4396] x5 : ffffffffffffffff x4 : 0000000000000000 [ 120.861123][ T4396] x3 : ffffffc011cf4f90 x2 : ffffff81fee7b948 [ 120.867122][ T4396] x1 : ffffffc011cf4f90 x0 : 0000000000000054 [ 120.873122][ T4396] Call trace: [ 120.876259][ T4396] __list_del_entry_valid+0x98/0xac [ 120.881304][ T4396] hid_debug_events_release+0x48/0x12c [ 120.886617][ T4396] full_proxy_release+0x50/0xbc [ 120.891323][ T4396] __fput+0xdc/0x238 [ 120.895075][ T4396] ____fput+0x14/0x24 [ 120.898911][ T4396] task_work_run+0x90/0x148 [ 120.903268][ T4396] do_exit+0x1bc/0x8a4 [ 120.907193][ T4396] do_group_exit+0x8c/0xa4 [ 120.911458][ T4396] get_signal+0x468/0x744 [ 120.915643][ T4396] do_signal+0x84/0x280 [ 120.919650][ T4396] do_notify_resume+0xd0/0x218 [ 120.924262][ T4396] work_pending+0xc/0x3f0 At 2023-10-23 23:03:35, "Rahul Rameshbabu" <sergeantsagara@protonmail.com> wrote: >Hi Charles, > >On Mon, 23 Oct, 2023 17:35:00 +0800 "Charles Yi" <be286@163.com> wrote: >> hid_debug_events_release() access released memory by >> hid_device_release(). This is fixed by the patch. >> > >A couple of things here. Can you add a Fixes: tag? > >https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes > >Before any v2 however, it would be nice to understand where this issue >is coming from. I am wondering if it's really a core issue or rather an >issue with a higher level device specific driver making use of the hid >subsystem. I am having a hard time seeing how this issue occurs >currently. A stack trace in a follow-up email to this one pertaining to >the crash would be helpful. If you are resolving a syzbot report, a link >to that report would suffice. > >This patch doesn't make a lot of sense to me as-is because >hid_debug_events_release is about release resources related to hid debug >events (at least from my current understanding). It should not be >free-ing the underlying hid device/instance itself. > >> Signed-off-by: Charles Yi <be286@163.com> >> --- >> drivers/hid/hid-core.c | 12 ++++++++++-- >> drivers/hid/hid-debug.c | 3 +++ >> include/linux/hid.h | 3 +++ >> 3 files changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c >> index 8992e3c1e769..e0181218ad85 100644 >> --- a/drivers/hid/hid-core.c >> +++ b/drivers/hid/hid-core.c >> @@ -702,15 +702,22 @@ static void hid_close_report(struct hid_device *device) >> * Free a device structure, all reports, and all fields. >> */ >> >> -static void hid_device_release(struct device *dev) >> +void hiddev_free(struct kref *ref) >> { >> - struct hid_device *hid = to_hid_device(dev); >> + struct hid_device *hid = container_of(ref, struct hid_device, ref); >> >> hid_close_report(hid); >> kfree(hid->dev_rdesc); >> kfree(hid); >> } >> >> +static void hid_device_release(struct device *dev) >> +{ >> + struct hid_device *hid = to_hid_device(dev); >> + >> + kref_put(&hid->ref, hiddev_free); >> +} >> + >> /* >> * Fetch a report description item from the data stream. We support long >> * items, though they are not used yet. >> @@ -2846,6 +2853,7 @@ struct hid_device *hid_allocate_device(void) >> spin_lock_init(&hdev->debug_list_lock); >> sema_init(&hdev->driver_input_lock, 1); >> mutex_init(&hdev->ll_open_lock); >> + kref_init(&hdev->ref); >> >> hid_bpf_device_init(hdev); >> >> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c >> index e7ef1ea107c9..7dd83ec74f8a 100644 >> --- a/drivers/hid/hid-debug.c >> +++ b/drivers/hid/hid-debug.c >> @@ -1135,6 +1135,7 @@ static int hid_debug_events_open(struct inode *inode, struct file *file) >> goto out; >> } >> list->hdev = (struct hid_device *) inode->i_private; >> + kref_get(&list->hdev->ref); >> file->private_data = list; >> mutex_init(&list->read_mutex); >> >> @@ -1227,6 +1228,8 @@ static int hid_debug_events_release(struct inode *inode, struct file *file) >> list_del(&list->node); >> spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags); >> kfifo_free(&list->hid_debug_fifo); >> + >> + kref_put(&list->hdev->ref, hiddev_free); >> kfree(list); >> >> return 0; >> diff --git a/include/linux/hid.h b/include/linux/hid.h >> index 964ca1f15e3f..3b08a2957229 100644 >> --- a/include/linux/hid.h >> +++ b/include/linux/hid.h >> @@ -679,6 +679,7 @@ struct hid_device { /* device report descriptor */ >> struct list_head debug_list; >> spinlock_t debug_list_lock; >> wait_queue_head_t debug_wait; >> + struct kref ref; >> >> unsigned int id; /* system unique id */ >> >> @@ -687,6 +688,8 @@ struct hid_device { /* device report descriptor */ >> #endif /* CONFIG_BPF */ >> }; >> >> +void hiddev_free(struct kref *ref); >> + >> #define to_hid_device(pdev) \ >> container_of(pdev, struct hid_device, dev) > >-- >Thank you for the patch, > >Rahul Rameshbabu ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: fix a crash in hid_debug_events_release 2023-10-24 9:49 ` be286 @ 2023-10-29 16:44 ` Rahul Rameshbabu 0 siblings, 0 replies; 4+ messages in thread From: Rahul Rameshbabu @ 2023-10-29 16:44 UTC (permalink / raw) To: be286; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel On Tue, 24 Oct, 2023 17:49:36 +0800 "be286" <be286@163.com> wrote: > Hi Rahul, > > > When hid_debug_events_release() was being called, in most case, > hid_device_release() finish already, the memory of list->hdev > freed by hid_device_release(), if list->hdev memory > reallocate by others, and it's modified, zeroed, then > list->hdev->debug_list_lock occasioned crash come out. > This makes sense to me. Thanks for the additional explanation. I would add this detail to the commit message body of your v2 to make the patch clearer. With this explanation, I think your patch makes a lot of sense to me and I understand better why you are seeing the trace you shared. > The runing order: > > [ 258.201069] CPU: 0 PID: 203 Comm: kworker/0:2 Not tainted 5.10.110 #255 > [ 258.201073] Hardware name: Rockchip RK3588 EVB4 LP4 V10 Board (DT) > [ 258.201086] Workqueue: usb_hub_wq hub_event > [ 258.201092] Call trace: > [ 258.201100] dump_backtrace+0x0/0x1f8 > [ 258.201105] show_stack+0x1c/0x2c > [ 258.201112] dump_stack_lvl+0xd8/0x12c > [ 258.201116] dump_stack+0x1c/0x60 > [ 258.201122] hid_device_release+0x94/0xb4 > [ 258.201127] device_release+0x38/0x94 > [ 258.201133] kobject_cleanup+0xd0/0x174 > [ 258.201137] kobject_put+0x68/0xa8 > [ 258.201143] put_device+0x1c/0x2c > [ 258.201146] hid_destroy_device+0x60/0x74 > [ 258.201153] usbhid_disconnect+0x5c/0x90 > [ 258.201157] usb_unbind_interface+0xc4/0x268 > [ 258.201162] device_release_driver_internal+0x184/0x25c > [ 258.201165] device_release_driver+0x1c/0x2c > [ 258.201169] bus_remove_device+0xdc/0x138 > [ 258.201173] device_del+0x1d0/0x3d8 > [ 258.201177] usb_disable_device+0x108/0x228 > [ 258.201181] usb_disconnect+0xf4/0x304 > [ 258.201184] usb_disconnect+0xdc/0x304 > [ 258.201188] hub_port_connect+0x88/0xa24 > [ 258.201191] hub_port_connect_change+0x2d8/0x4cc > [ 258.201195] port_event+0x550/0x614 > [ 258.201198] hub_event+0x12c/0x494 > [ 258.201203] process_one_work+0x1f4/0x490 > [ 258.201207] worker_thread+0x278/0x4dc > [ 258.201211] kthread+0x130/0x338 > [ 258.201215] ret_from_fork+0x10/0x30 > [ 259.925641] CPU: 1 PID: 2354 Comm: hidt_bridge Not tainted 5.10.110 #255 > [ 259.925652] Hardware name: Rockchip RK3588 EVB4 LP4 V10 Board (DT) > [ 259.925656] Call trace: > [ 259.925671] dump_backtrace+0x0/0x1f8 > [ 259.925676] show_stack+0x1c/0x2c > [ 259.925685] dump_stack_lvl+0xd8/0x12c > [ 259.925689] dump_stack+0x1c/0x60 > [ 259.925697] hid_debug_events_release+0x24/0x134 > [ 259.925704] full_proxy_release+0x50/0xbc > [ 259.925709] __fput+0xdc/0x238 > [ 259.925714] ____fput+0x14/0x24 > [ 259.925720] task_work_run+0x90/0x148 > [ 259.925725] do_exit+0x1bc/0x8a4 > [ 259.925729] do_group_exit+0x8c/0xa4 > [ 259.925734] get_signal+0x468/0x744 > [ 259.925739] do_signal+0x84/0x280 > [ 259.925743] do_notify_resume+0xd8/0x228 > [ 259.925747] work_pending+0xc/0x3f0 > > The crash: > > [ 120.728477][ T4396] kernel BUG at lib/list_debug.c:53! > [ 120.728505][ T4396] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP > [ 120.739806][ T4396] Modules linked in: bcmdhd dhd_static_buf 8822cu pcie_mhi r8168 > [ 120.747386][ T4396] CPU: 1 PID: 4396 Comm: hidt_bridge Not tainted 5.10.110 #257 > [ 120.754771][ T4396] Hardware name: Rockchip RK3588 EVB4 LP4 V10 Board (DT) > [ 120.761643][ T4396] pstate: 60400089 (nZCv daIf +PAN -UAO -TCO BTYPE=--) > [ 120.768338][ T4396] pc : __list_del_entry_valid+0x98/0xac > [ 120.773730][ T4396] lr : __list_del_entry_valid+0x98/0xac > [ 120.779120][ T4396] sp : ffffffc01e62bb60 > [ 120.783126][ T4396] x29: ffffffc01e62bb60 x28: ffffff818ce3a200 > [ 120.789126][ T4396] x27: 0000000000000009 x26: 0000000000980000 > [ 120.795126][ T4396] x25: ffffffc012431000 x24: ffffff802c6d4e00 > [ 120.801125][ T4396] x23: ffffff8005c66f00 x22: ffffffc01183b5b8 > [ 120.807125][ T4396] x21: ffffff819df2f100 x20: 0000000000000000 > [ 120.813124][ T4396] x19: ffffff802c3f0700 x18: ffffffc01d2cd058 > [ 120.819124][ T4396] x17: 0000000000000000 x16: 0000000000000000 > [ 120.825124][ T4396] x15: 0000000000000004 x14: 0000000000003fff > [ 120.831123][ T4396] x13: ffffffc012085588 x12: 0000000000000003 > [ 120.837123][ T4396] x11: 00000000ffffbfff x10: 0000000000000003 > [ 120.843123][ T4396] x9 : 455103d46b329300 x8 : 455103d46b329300 > [ 120.849124][ T4396] x7 : 74707572726f6320 x6 : ffffffc0124b8cb5 > [ 120.855124][ T4396] x5 : ffffffffffffffff x4 : 0000000000000000 > [ 120.861123][ T4396] x3 : ffffffc011cf4f90 x2 : ffffff81fee7b948 > [ 120.867122][ T4396] x1 : ffffffc011cf4f90 x0 : 0000000000000054 > [ 120.873122][ T4396] Call trace: > [ 120.876259][ T4396] __list_del_entry_valid+0x98/0xac > [ 120.881304][ T4396] hid_debug_events_release+0x48/0x12c > [ 120.886617][ T4396] full_proxy_release+0x50/0xbc > [ 120.891323][ T4396] __fput+0xdc/0x238 > [ 120.895075][ T4396] ____fput+0x14/0x24 > [ 120.898911][ T4396] task_work_run+0x90/0x148 > [ 120.903268][ T4396] do_exit+0x1bc/0x8a4 > [ 120.907193][ T4396] do_group_exit+0x8c/0xa4 > [ 120.911458][ T4396] get_signal+0x468/0x744 > [ 120.915643][ T4396] do_signal+0x84/0x280 > [ 120.919650][ T4396] do_notify_resume+0xd0/0x218 > [ 120.924262][ T4396] work_pending+0xc/0x3f0 > If you would like, you can use git notes to add the trace as well to the notes summary of the patches you send out. This isn't a must but in case you want to provide additional context in the future without having this be a part of the commit message body. https://git-scm.com/docs/git-format-patch#Documentation/git-format-patch.txt---notesltrefgt > > > > > > > > > > At 2023-10-23 23:03:35, "Rahul Rameshbabu" <sergeantsagara@protonmail.com> wrote: >>Hi Charles, >> >>On Mon, 23 Oct, 2023 17:35:00 +0800 "Charles Yi" <be286@163.com> wrote: >>> hid_debug_events_release() access released memory by >>> hid_device_release(). This is fixed by the patch. >>> >> >>A couple of things here. Can you add a Fixes: tag? >> >>https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes >> >>Before any v2 however, it would be nice to understand where this issue >>is coming from. I am wondering if it's really a core issue or rather an >>issue with a higher level device specific driver making use of the hid >>subsystem. I am having a hard time seeing how this issue occurs >>currently. A stack trace in a follow-up email to this one pertaining to >>the crash would be helpful. If you are resolving a syzbot report, a link >>to that report would suffice. >> >>This patch doesn't make a lot of sense to me as-is because >>hid_debug_events_release is about release resources related to hid debug >>events (at least from my current understanding). It should not be >>free-ing the underlying hid device/instance itself. >> >>> Signed-off-by: Charles Yi <be286@163.com> >>> --- [...] Sorry for the delay in being able to review this. Excited to see a v2 with a Fixes: tag. -- Thanks, Rahul Rameshbabu ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-29 16:44 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-23 9:35 [PATCH] HID: fix a crash in hid_debug_events_release Charles Yi 2023-10-23 15:03 ` Rahul Rameshbabu 2023-10-24 9:49 ` be286 2023-10-29 16:44 ` Rahul Rameshbabu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).