* [PATCH v2 0/2] Fix two kernel warnings in glink driver
@ 2021-10-25 23:37 Sujit Kautkar
2021-10-25 23:37 ` [PATCH v2 1/2] rpmsg: glink: Fix use-after-free in qcom_glink_rpdev_release() Sujit Kautkar
0 siblings, 1 reply; 3+ messages in thread
From: Sujit Kautkar @ 2021-10-25 23:37 UTC (permalink / raw)
To: Andy Gross, Ohad Ben-Cohen
Cc: Stephen Boyd, Sibi Sankar, Bjorn Andersson, Sujit Kautkar,
linux-arm-msm, linux-kernel, linux-remoteproc
These changes addresses kernel warnings which shows up after enabling
debug kernel. First one fixes use-after-free warning and second fixes
warning by updating cdev APIs
Changes in v2:
- Fix typo in commit message
Sujit Kautkar (2):
rpmsg: glink: Fix use-after-free in qcom_glink_rpdev_release()
rpmsg: glink: Update cdev add/del API in
rpmsg_ctrldev_release_device()
drivers/rpmsg/qcom_glink_native.c | 5 ++++-
drivers/rpmsg/rpmsg_char.c | 5 ++---
2 files changed, 6 insertions(+), 4 deletions(-)
--
2.31.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 1/2] rpmsg: glink: Fix use-after-free in qcom_glink_rpdev_release()
2021-10-25 23:37 [PATCH v2 0/2] Fix two kernel warnings in glink driver Sujit Kautkar
@ 2021-10-25 23:37 ` Sujit Kautkar
2021-10-26 0:29 ` Matthias Kaehlcke
0 siblings, 1 reply; 3+ messages in thread
From: Sujit Kautkar @ 2021-10-25 23:37 UTC (permalink / raw)
To: Andy Gross, Ohad Ben-Cohen
Cc: Stephen Boyd, Sibi Sankar, Bjorn Andersson, Sujit Kautkar,
linux-arm-msm, linux-kernel, linux-remoteproc
qcom_glink_rpdev_release() sets channel->rpdev to NULL. However, with
debug enabled kernel, qcom_glink_rpdev_release() gets delayed due to
delayed kobject release and channel gets released by that time and
triggers below kernel warning. To avoid this use-after-free, add a
condition to checks if channel was already released.
| BUG: KASAN: use-after-free in qcom_glink_rpdev_release+0x54/0x70
| Write of size 8 at addr ffffffaba438e8d0 by task kworker/6:1/54
|
| CPU: 6 PID: 54 Comm: kworker/6:1 Not tainted 5.4.109-lockdep #16
| Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
| Workqueue: events kobject_delayed_cleanup
| Call trace:
| dump_backtrace+0x0/0x284
| show_stack+0x20/0x2c
| dump_stack+0xd4/0x170
| print_address_description+0x3c/0x4a8
| __kasan_report+0x144/0x168
| kasan_report+0x10/0x18
| __asan_report_store8_noabort+0x1c/0x24
| qcom_glink_rpdev_release+0x54/0x70
| device_release+0x68/0x14c
| kobject_delayed_cleanup+0x158/0x2cc
| process_one_work+0x7cc/0x10a4
| worker_thread+0x80c/0xcec
| kthread+0x2a8/0x314
| ret_from_fork+0x10/0x18
Signed-off-by: Sujit Kautkar <sujitka@chromium.org>
---
(no changes since v1)
drivers/rpmsg/qcom_glink_native.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index e1444fefdd1c0..cc3556a9385a9 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -270,6 +270,7 @@ static void qcom_glink_channel_release(struct kref *ref)
spin_unlock_irqrestore(&channel->intent_lock, flags);
kfree(channel->name);
+ channel = NULL;
kfree(channel);
}
@@ -1372,8 +1373,10 @@ static void qcom_glink_rpdev_release(struct device *dev)
{
struct rpmsg_device *rpdev = to_rpmsg_device(dev);
struct glink_channel *channel = to_glink_channel(rpdev->ept);
+ if (channel) {
+ channel->rpdev = NULL;
+ }
- channel->rpdev = NULL;
kfree(rpdev);
}
--
2.31.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/2] rpmsg: glink: Fix use-after-free in qcom_glink_rpdev_release()
2021-10-25 23:37 ` [PATCH v2 1/2] rpmsg: glink: Fix use-after-free in qcom_glink_rpdev_release() Sujit Kautkar
@ 2021-10-26 0:29 ` Matthias Kaehlcke
0 siblings, 0 replies; 3+ messages in thread
From: Matthias Kaehlcke @ 2021-10-26 0:29 UTC (permalink / raw)
To: Sujit Kautkar
Cc: Andy Gross, Ohad Ben-Cohen, Stephen Boyd, Sibi Sankar,
Bjorn Andersson, linux-arm-msm, linux-kernel, linux-remoteproc
On Mon, Oct 25, 2021 at 04:37:52PM -0700, Sujit Kautkar wrote:
> qcom_glink_rpdev_release() sets channel->rpdev to NULL. However, with
> debug enabled kernel, qcom_glink_rpdev_release() gets delayed due to
> delayed kobject release and channel gets released by that time and
> triggers below kernel warning. To avoid this use-after-free, add a
> condition to checks if channel was already released.
>
> | BUG: KASAN: use-after-free in qcom_glink_rpdev_release+0x54/0x70
> | Write of size 8 at addr ffffffaba438e8d0 by task kworker/6:1/54
> |
> | CPU: 6 PID: 54 Comm: kworker/6:1 Not tainted 5.4.109-lockdep #16
> | Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
> | Workqueue: events kobject_delayed_cleanup
> | Call trace:
> | dump_backtrace+0x0/0x284
> | show_stack+0x20/0x2c
> | dump_stack+0xd4/0x170
> | print_address_description+0x3c/0x4a8
> | __kasan_report+0x144/0x168
> | kasan_report+0x10/0x18
> | __asan_report_store8_noabort+0x1c/0x24
> | qcom_glink_rpdev_release+0x54/0x70
> | device_release+0x68/0x14c
> | kobject_delayed_cleanup+0x158/0x2cc
> | process_one_work+0x7cc/0x10a4
> | worker_thread+0x80c/0xcec
> | kthread+0x2a8/0x314
> | ret_from_fork+0x10/0x18
>
> Signed-off-by: Sujit Kautkar <sujitka@chromium.org>
> ---
>
> (no changes since v1)
>
> drivers/rpmsg/qcom_glink_native.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index e1444fefdd1c0..cc3556a9385a9 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -270,6 +270,7 @@ static void qcom_glink_channel_release(struct kref *ref)
> spin_unlock_irqrestore(&channel->intent_lock, flags);
>
> kfree(channel->name);
> + channel = NULL;
This doesn't make much sense, 'channel' is a local variable, the only effect
this has is that the memory of 'channel' isn't freed by the 'kfree' below.
Maybe this is debug code and wasn't intended to be part of this patch?
> kfree(channel);
> }
>
> @@ -1372,8 +1373,10 @@ static void qcom_glink_rpdev_release(struct device *dev)
> {
> struct rpmsg_device *rpdev = to_rpmsg_device(dev);
> struct glink_channel *channel = to_glink_channel(rpdev->ept);
> + if (channel) {
> + channel->rpdev = NULL;
> + }
Remove curly braces for single line branch.
>
> - channel->rpdev = NULL;
> kfree(rpdev);
> }
>
> --
> 2.31.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-26 0:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 23:37 [PATCH v2 0/2] Fix two kernel warnings in glink driver Sujit Kautkar
2021-10-25 23:37 ` [PATCH v2 1/2] rpmsg: glink: Fix use-after-free in qcom_glink_rpdev_release() Sujit Kautkar
2021-10-26 0:29 ` Matthias Kaehlcke
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).