[PATCH V1 0/3] rpmsg char fixes for race conditions in device reboot

[PATCH V1 0/3] rpmsg char fixes for race conditions in device reboot

[Deepak Kumar Singh]

Fix for different race conditions observed in rpmsg_char driver
for reboot test of device.

Deepak Kumar Singh (3):
  rpmsg: glink: Free device context only when cdev not in use
  rpmsg: glink: Add lock to avoid race when rpmsg device is released
  rpmsg: glink: Add lock for ctrl device

 drivers/rpmsg/rpmsg_char.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH V1 1/3] rpmsg: glink: Free device context only when cdev not in use

[Deepak Kumar Singh]

Struct device holding cdev should not be freed unless cdev
is not in use. It is possible that user space has opened
char device while kernel has freed the associated struct
device context.

Mark dev kobj as parent of cdev, so that chardev_add gets
an extra reference to dev. This ensures device context is not
freed until cdev is is not in uses.
---
 drivers/rpmsg/rpmsg_char.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index c03a118..72ee101 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -417,6 +417,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
 	dev->id = ret;
 	dev_set_name(dev, "rpmsg%d", ret);
 
+	cdev_set_parent(&eptdev->cdev, &dev->kobj);
 	ret = cdev_add(&eptdev->cdev, dev->devt, 1);
 	if (ret)
 		goto free_ept_ida;
@@ -533,6 +534,7 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
 	dev->id = ret;
 	dev_set_name(&ctrldev->dev, "rpmsg_ctrl%d", ret);
 
+	cdev_set_parent(&ctrldev->cdev, &dev->kobj);
 	ret = cdev_add(&ctrldev->cdev, dev->devt, 1);
 	if (ret)
 		goto free_ctrl_ida;
-- 
2.7.4

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

[Deepak Kumar Singh]

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.

Use per ept lock to avoid race between rpmsg_destroy_ept and
rpmsg_destory_ept.
---
 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

[PATCH V1 3/3] rpmsg: glink: Add lock for ctrl device

[Deepak Kumar Singh]

Race between rpmsg_eptdev_create and rpmsg_chrdev_remove
can sometime casue crash while accessing rpdev while new
endpoint is being created. Using lock ensure no new eptdev
is created after rpmsg_chrdev_remove has been completed.
---
 drivers/rpmsg/rpmsg_char.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index 2108ef8..3e5b85d 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -27,6 +27,7 @@
 
 static dev_t rpmsg_major;
 static struct class *rpmsg_class;
+struct mutex ctrl_lock;
 
 static DEFINE_IDA(rpmsg_ctrl_ida);
 static DEFINE_IDA(rpmsg_ept_ida);
@@ -396,9 +397,12 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
 	struct device *dev;
 	int ret;
 
+	mutex_lock(&ctrl_lock);
 	eptdev = kzalloc(sizeof(*eptdev), GFP_KERNEL);
-	if (!eptdev)
+	if (!eptdev) {
+		mutex_unlock(&ctrl_lock);
 		return -ENOMEM;
+	}
 
 	dev = &eptdev->dev;
 	eptdev->rpdev = rpdev;
@@ -443,6 +447,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
 		put_device(dev);
 	}
 
+	mutex_unlock(&ctrl_lock);
 	return ret;
 
 free_ept_ida:
@@ -453,6 +458,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
 	put_device(dev);
 	kfree(eptdev);
 
+	mutex_unlock(&ctrl_lock);
 	return ret;
 }
 
@@ -525,6 +531,7 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
 	if (!ctrldev)
 		return -ENOMEM;
 
+	mutex_init(&ctrl_lock);
 	ctrldev->rpdev = rpdev;
 
 	dev = &ctrldev->dev;
@@ -581,12 +588,14 @@ static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev)
 	int ret;
 
 	/* Destroy all endpoints */
+	mutex_lock(&ctrl_lock);
 	ret = device_for_each_child(&ctrldev->dev, NULL, rpmsg_eptdev_destroy);
 	if (ret)
 		dev_warn(&rpdev->dev, "failed to nuke endpoints: %d\n", ret);
 
 	device_del(&ctrldev->dev);
 	put_device(&ctrldev->dev);
+	mutex_unlock(&ctrl_lock);
 }
 
 static struct rpmsg_driver rpmsg_chrdev_driver = {
-- 
2.7.4

Re: [PATCH V1 1/3] rpmsg: glink: Free device context only when cdev not in use

[Mathieu Poirier]

Hi Deepak,

On Thu, Jan 27, 2022 at 12:34:44AM +0530, Deepak Kumar Singh wrote:
> Struct device holding cdev should not be freed unless cdev
> is not in use. It is possible that user space has opened
> char device while kernel has freed the associated struct
> device context.
> 
> Mark dev kobj as parent of cdev, so that chardev_add gets
> an extra reference to dev. This ensures device context is not
> freed until cdev is is not in uses.
> ---
>  drivers/rpmsg/rpmsg_char.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> index c03a118..72ee101 100644
> --- a/drivers/rpmsg/rpmsg_char.c
> +++ b/drivers/rpmsg/rpmsg_char.c
> @@ -417,6 +417,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
>  	dev->id = ret;
>  	dev_set_name(dev, "rpmsg%d", ret);
>  
> +	cdev_set_parent(&eptdev->cdev, &dev->kobj);
>  	ret = cdev_add(&eptdev->cdev, dev->devt, 1);

This issue should have been fixed when cdev_add() was replaced by
cdev_device_add(), something you will find on v5.17-rc2.

Also, this set is generating checkpatch warnings and as such I will not review
the other patches in it. 

Thanks,
Mathieu

>  	if (ret)
>  		goto free_ept_ida;
> @@ -533,6 +534,7 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
>  	dev->id = ret;
>  	dev_set_name(&ctrldev->dev, "rpmsg_ctrl%d", ret);
>  
> +	cdev_set_parent(&ctrldev->cdev, &dev->kobj);
>  	ret = cdev_add(&ctrldev->cdev, dev->devt, 1);
>  	if (ret)
>  		goto free_ctrl_ida;
> -- 
> 2.7.4
> 

Re: [PATCH V1 1/3] rpmsg: glink: Free device context only when cdev not in use

[Deepak Kumar Singh]

On 2/3/2022 11:05 PM, Mathieu Poirier wrote:
> Hi Deepak,
>
> On Thu, Jan 27, 2022 at 12:34:44AM +0530, Deepak Kumar Singh wrote:
>> Struct device holding cdev should not be freed unless cdev
>> is not in use. It is possible that user space has opened
>> char device while kernel has freed the associated struct
>> device context.
>>
>> Mark dev kobj as parent of cdev, so that chardev_add gets
>> an extra reference to dev. This ensures device context is not
>> freed until cdev is is not in uses.
>> ---
>>   drivers/rpmsg/rpmsg_char.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
>> index c03a118..72ee101 100644
>> --- a/drivers/rpmsg/rpmsg_char.c
>> +++ b/drivers/rpmsg/rpmsg_char.c
>> @@ -417,6 +417,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
>>   	dev->id = ret;
>>   	dev_set_name(dev, "rpmsg%d", ret);
>>   
>> +	cdev_set_parent(&eptdev->cdev, &dev->kobj);
>>   	ret = cdev_add(&eptdev->cdev, dev->devt, 1);
> This issue should have been fixed when cdev_add() was replaced by
> cdev_device_add(), something you will find on v5.17-rc2.
>
> Also, this set is generating checkpatch warnings and as such I will not review
> the other patches in it.
>
> Thanks,
> Mathieu

Thank you Mathieu for info!! i will recheck other 2 patches for 
checkpatch warnings.

Thanks,

Deepak

>>   	if (ret)
>>   		goto free_ept_ida;
>> @@ -533,6 +534,7 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
>>   	dev->id = ret;
>>   	dev_set_name(&ctrldev->dev, "rpmsg_ctrl%d", ret);
>>   
>> +	cdev_set_parent(&ctrldev->cdev, &dev->kobj);
>>   	ret = cdev_add(&ctrldev->cdev, dev->devt, 1);
>>   	if (ret)
>>   		goto free_ctrl_ida;
>> -- 
>> 2.7.4
>>

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

[Bjorn Andersson]

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
> 

Re: [PATCH V1 3/3] rpmsg: glink: Add lock for ctrl device

[Bjorn Andersson]

On Wed 26 Jan 13:04 CST 2022, Deepak Kumar Singh wrote:

> Race between rpmsg_eptdev_create and rpmsg_chrdev_remove
> can sometime casue crash while accessing rpdev while new
> endpoint is being created. Using lock ensure no new eptdev
> is created after rpmsg_chrdev_remove has been completed.

This patch lacks a Signed-off-by.

Isn't this solving the same problem as the previous patch? Would be nice
with some more specifics on the race that you're seeing.

Thanks,
Bjorn

> ---
>  drivers/rpmsg/rpmsg_char.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> index 2108ef8..3e5b85d 100644
> --- a/drivers/rpmsg/rpmsg_char.c
> +++ b/drivers/rpmsg/rpmsg_char.c
> @@ -27,6 +27,7 @@
>  
>  static dev_t rpmsg_major;
>  static struct class *rpmsg_class;
> +struct mutex ctrl_lock;
>  
>  static DEFINE_IDA(rpmsg_ctrl_ida);
>  static DEFINE_IDA(rpmsg_ept_ida);
> @@ -396,9 +397,12 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
>  	struct device *dev;
>  	int ret;
>  
> +	mutex_lock(&ctrl_lock);
>  	eptdev = kzalloc(sizeof(*eptdev), GFP_KERNEL);
> -	if (!eptdev)
> +	if (!eptdev) {
> +		mutex_unlock(&ctrl_lock);
>  		return -ENOMEM;
> +	}
>  
>  	dev = &eptdev->dev;
>  	eptdev->rpdev = rpdev;
> @@ -443,6 +447,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
>  		put_device(dev);
>  	}
>  
> +	mutex_unlock(&ctrl_lock);
>  	return ret;
>  
>  free_ept_ida:
> @@ -453,6 +458,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
>  	put_device(dev);
>  	kfree(eptdev);
>  
> +	mutex_unlock(&ctrl_lock);
>  	return ret;
>  }
>  
> @@ -525,6 +531,7 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
>  	if (!ctrldev)
>  		return -ENOMEM;
>  
> +	mutex_init(&ctrl_lock);
>  	ctrldev->rpdev = rpdev;
>  
>  	dev = &ctrldev->dev;
> @@ -581,12 +588,14 @@ static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev)
>  	int ret;
>  
>  	/* Destroy all endpoints */
> +	mutex_lock(&ctrl_lock);
>  	ret = device_for_each_child(&ctrldev->dev, NULL, rpmsg_eptdev_destroy);
>  	if (ret)
>  		dev_warn(&rpdev->dev, "failed to nuke endpoints: %d\n", ret);
>  
>  	device_del(&ctrldev->dev);
>  	put_device(&ctrldev->dev);
> +	mutex_unlock(&ctrl_lock);
>  }
>  
>  static struct rpmsg_driver rpmsg_chrdev_driver = {
> -- 
> 2.7.4
> 

Re: [PATCH V1 3/3] rpmsg: glink: Add lock for ctrl device

[Deepak Kumar Singh]

On 3/12/2022 2:24 AM, Bjorn Andersson wrote:
> On Wed 26 Jan 13:04 CST 2022, Deepak Kumar Singh wrote:
>
>> Race between rpmsg_eptdev_create and rpmsg_chrdev_remove
>> can sometime casue crash while accessing rpdev while new
>> endpoint is being created. Using lock ensure no new eptdev
>> is created after rpmsg_chrdev_remove has been completed.
> This patch lacks a Signed-off-by.
I will correct that in next patch.
> Isn't this solving the same problem as the previous patch? Would be nice
> with some more specifics on the race that you're seeing.
>
> Thanks,
> Bjorn

Issue was observed after having patch 2, in reboot test case.

Here observation was, user space daemon was able to create rpmsg0 device 
through

ctrl device and it was in process of rpmsg_eptdev_create() but as such 
ept creation was not yet done.

At the same time rpmsg_chrdev_remove() call happened which caused ctrl 
device to be freed.

backtrace of crash -

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

>> ---
>>   drivers/rpmsg/rpmsg_char.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
>> index 2108ef8..3e5b85d 100644
>> --- a/drivers/rpmsg/rpmsg_char.c
>> +++ b/drivers/rpmsg/rpmsg_char.c
>> @@ -27,6 +27,7 @@
>>   
>>   static dev_t rpmsg_major;
>>   static struct class *rpmsg_class;
>> +struct mutex ctrl_lock;
>>   
>>   static DEFINE_IDA(rpmsg_ctrl_ida);
>>   static DEFINE_IDA(rpmsg_ept_ida);
>> @@ -396,9 +397,12 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
>>   	struct device *dev;
>>   	int ret;
>>   
>> +	mutex_lock(&ctrl_lock);
>>   	eptdev = kzalloc(sizeof(*eptdev), GFP_KERNEL);
>> -	if (!eptdev)
>> +	if (!eptdev) {
>> +		mutex_unlock(&ctrl_lock);
>>   		return -ENOMEM;
>> +	}
>>   
>>   	dev = &eptdev->dev;
>>   	eptdev->rpdev = rpdev;
>> @@ -443,6 +447,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
>>   		put_device(dev);
>>   	}
>>   
>> +	mutex_unlock(&ctrl_lock);
>>   	return ret;
>>   
>>   free_ept_ida:
>> @@ -453,6 +458,7 @@ static int rpmsg_eptdev_create(struct rpmsg_ctrldev *ctrldev,
>>   	put_device(dev);
>>   	kfree(eptdev);
>>   
>> +	mutex_unlock(&ctrl_lock);
>>   	return ret;
>>   }
>>   
>> @@ -525,6 +531,7 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
>>   	if (!ctrldev)
>>   		return -ENOMEM;
>>   
>> +	mutex_init(&ctrl_lock);
>>   	ctrldev->rpdev = rpdev;
>>   
>>   	dev = &ctrldev->dev;
>> @@ -581,12 +588,14 @@ static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev)
>>   	int ret;
>>   
>>   	/* Destroy all endpoints */
>> +	mutex_lock(&ctrl_lock);
>>   	ret = device_for_each_child(&ctrldev->dev, NULL, rpmsg_eptdev_destroy);
>>   	if (ret)
>>   		dev_warn(&rpdev->dev, "failed to nuke endpoints: %d\n", ret);
>>   
>>   	device_del(&ctrldev->dev);
>>   	put_device(&ctrldev->dev);
>> +	mutex_unlock(&ctrl_lock);
>>   }
>>   
>>   static struct rpmsg_driver rpmsg_chrdev_driver = {
>> -- 
>> 2.7.4
>>

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

[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
>>