Re: [PATCH V1 2/3] rpmsg: glink: Add lock to avoid race when rpmsg device is released

Bjorn Andersson
2022-03-11T20:53:01Z

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?

> 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().

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
>