| Message ID | 1643223886-28170-4-git-send-email-quic_deesin@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | rpmsg char fixes for race conditions in device reboot | expand |
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 >
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 = {