Messages in this thread Patch in this message | | | Date | Thu, 7 Apr 2022 22:28:27 +0530 | Subject | Re: [PATCH V1 2/3] rpmsg: glink: Add lock to avoid race when rpmsg device is released | From | Deepak Kumar Singh <> |
| |
On 3/12/2022 2:22 AM, Bjorn Andersson wrote: > On Wed 26 Jan 13:04 CST 2022, Deepak Kumar Singh wrote: > >> When remote host goes down glink char device channel is freed, >> At the same time user space apps can still try to open rpmsg_char >> device which will result in calling rpmsg_create_ept. This may cause >> reference to already freed context of glink chardev channel. >> > Hi Deepak, > > Could you please be a little bit more specific on the details of where > you're seeing this race? Perhaps I'm just missing something obvious?
Crash is observed in reboot test case.
Log prints suggested that ept was destroyed just before crash in rpmsg_eptdev_create().
Below code was executed before crash -
static int rpmsg_eptdev_destroy(struct device *dev, void *data) { struct rpmsg_eptdev *eptdev = dev_to_eptdev(dev);
mutex_lock(&eptdev->ept_lock); if (eptdev->ept) { rpmsg_destroy_ept(eptdev->ept); eptdev->ept = NULL; } mutex_unlock(&eptdev->ept_lock);
/* wake up any blocked readers */ wake_up_interruptible(&eptdev->readq);
device_del(&eptdev->dev); put_device(&eptdev->dev);
return 0; }
one crash was observed in rpmsg_eptdev_create() and other in rpmsg_eptdev_poll() -
1)
rpmsg_create_ept+0x40/0xa0 rpmsg_eptdev_open+0x88/0x138 chrdev_open+0xc4/0x1c8 do_dentry_open+0x230/0x378 vfs_open+0x3c/0x48 path_openat+0x93c/0xa78 do_filp_open+0x98/0x118 do_sys_openat2+0x90/0x220 do_sys_open+0x64/0x8c
2)
rpmsg_poll+0x5c/0x80 rpmsg_eptdev_poll+0x84/0xa4 do_sys_poll+0x22c/0x5c8
>> Use per ept lock to avoid race between rpmsg_destroy_ept and >> rpmsg_destory_ept. > I presume one of these should say rpmsg_eptdev_open(). yes, i will correct this in next patch. > Regards, > Bjorn > >> --- >> drivers/rpmsg/rpmsg_char.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c >> index 72ee101..2108ef8 100644 >> --- a/drivers/rpmsg/rpmsg_char.c >> +++ b/drivers/rpmsg/rpmsg_char.c >> @@ -85,6 +85,7 @@ static int rpmsg_eptdev_destroy(struct device *dev, void *data) >> struct rpmsg_eptdev *eptdev = dev_to_eptdev(dev); >> >> mutex_lock(&eptdev->ept_lock); >> + eptdev->rpdev = NULL; >> if (eptdev->ept) { >> rpmsg_destroy_ept(eptdev->ept); >> eptdev->ept = NULL; >> @@ -145,15 +146,24 @@ static int rpmsg_eptdev_open(struct inode *inode, struct file *filp) >> >> get_device(dev); >> >> + mutex_lock(&eptdev->ept_lock); >> + if (!eptdev->rpdev) { >> + put_device(dev); >> + mutex_unlock(&eptdev->ept_lock); >> + return -ENETRESET; >> + } >> + >> ept = rpmsg_create_ept(rpdev, rpmsg_ept_cb, eptdev, eptdev->chinfo); >> if (!ept) { >> dev_err(dev, "failed to open %s\n", eptdev->chinfo.name); >> + mutex_unlock(&eptdev->ept_lock); >> put_device(dev); >> return -EINVAL; >> } >> >> ept->sig_cb = rpmsg_sigs_cb; >> eptdev->ept = ept; >> + mutex_unlock(&eptdev->ept_lock); >> filp->private_data = eptdev; >> >> return 0; >> @@ -285,7 +295,9 @@ static __poll_t rpmsg_eptdev_poll(struct file *filp, poll_table *wait) >> if (eptdev->sig_pending) >> mask |= EPOLLPRI; >> >> + mutex_lock(&eptdev->ept_lock); >> mask |= rpmsg_poll(eptdev->ept, filp, wait); >> + mutex_unlock(&eptdev->ept_lock); >> >> return mask; >> } >> -- >> 2.7.4 >>
| |