| Message ID | 20230215-wip-mcp2221-v2-1-109f71fd036e@redhat.com |
|---|---|
| State | Accepted |
| Commit | 47e91fdfa511139f2549687edb0d8649b123227b |
| Headers | show |
| Series | [v2] HID: mcp-2221: prevent UAF in delayed work | expand |
On Thu, 16 Feb 2023, Benjamin Tissoires wrote: > If the device is plugged/unplugged without giving time for mcp_init_work() > to complete, we might kick in the devm free code path and thus have > unavailable struct mcp_2221 while in delayed work. > > Canceling the delayed_work item is enough to solve the issue, because > cancel_delayed_work_sync will prevent the work item to requeue itself. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Acked-by: Jiri Kosina <jkosina@suse.cz> Thanks Benjamin.
On Mon, Feb 20, 2023 at 10:10 AM Jiri Kosina <jikos@kernel.org> wrote: > > On Thu, 16 Feb 2023, Benjamin Tissoires wrote: > > > If the device is plugged/unplugged without giving time for mcp_init_work() > > to complete, we might kick in the devm free code path and thus have > > unavailable struct mcp_2221 while in delayed work. > > > > Canceling the delayed_work item is enough to solve the issue, because > > cancel_delayed_work_sync will prevent the work item to requeue itself. > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Acked-by: Jiri Kosina <jkosina@suse.cz> Thanks a lot. I realized I was missing the Fixes 960f9df7c620 and Cc: stable tags. I am adding those right now and will push it as soon as the minimum CI reports back that it's OK. Cheers, Benjamin > > Thanks Benjamin. > > -- > Jiri Kosina > SUSE Labs >
On Thu, 16 Feb 2023 11:22:58 +0100, Benjamin Tissoires wrote: > If the device is plugged/unplugged without giving time for mcp_init_work() > to complete, we might kick in the devm free code path and thus have > unavailable struct mcp_2221 while in delayed work. > > Canceling the delayed_work item is enough to solve the issue, because > cancel_delayed_work_sync will prevent the work item to requeue itself. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.3/mcp2221), thanks! [1/1] HID: mcp-2221: prevent UAF in delayed work https://git.kernel.org/hid/hid/c/47e91fdfa511 Cheers,
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index e61dd039354b..f74a977cf8f8 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -922,6 +922,9 @@ static void mcp2221_hid_unregister(void *ptr) /* This is needed to be sure hid_hw_stop() isn't called twice by the subsystem */ static void mcp2221_remove(struct hid_device *hdev) { + struct mcp2221 *mcp = hid_get_drvdata(hdev); + + cancel_delayed_work_sync(&mcp->init_work); } #if IS_REACHABLE(CONFIG_IIO)
If the device is plugged/unplugged without giving time for mcp_init_work() to complete, we might kick in the devm free code path and thus have unavailable struct mcp_2221 while in delayed work. Canceling the delayed_work item is enough to solve the issue, because cancel_delayed_work_sync will prevent the work item to requeue itself. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- Similar to Pietro's series, we can see the pattern in hid-mcp2221, except that this time the ledclass is not involved. Link: https://lore.kernel.org/linux-input/20230125-hid-unregister-leds-v4-5-7860c5763c38@diag.uniroma1.it/ --- Changes in v2: - drop the spinlock/boolean - Link to v1: https://lore.kernel.org/r/20230215-wip-mcp2221-v1-1-d7d1da261a5c@redhat.com --- drivers/hid/hid-mcp2221.c | 3 +++ 1 file changed, 3 insertions(+) --- base-commit: d883fd110dc17308a1506c5bf17e00ce9fe7b2a2 change-id: 20230215-wip-mcp2221-979d4115efb5 Best regards,