Messages in this thread | | | Date | Tue, 9 Jun 2020 07:18:16 -0500 | From | Corey Minyard <> | Subject | Re: [PATCH] ipmi: code cleanup and prevent potential issue. |
| |
On Tue, Jun 09, 2020 at 01:04:10AM -0500, wu000273@umn.edu wrote: > From: Qiushi Wu <wu000273@umn.edu> > > All the previous get/put operations against intf->refcount are > inside the mutex. Thus, put the last kref_put() also inside mutex > to make sure get/put functions execute in order and prevent the > potential race condition.
No, this can result in a crash. intf and intf->bmc_reg_mutex will be freed by intf_free. In fact, every call to kref_put() on intf better be outside any mutex/lock in intf. If you saw any, that is a bug, please report that. kref_get() is fine inside the mutex.
Plus, this is not a race condition. get/put is atomic.
-corey
> > Signed-off-by: Qiushi Wu <wu000273@umn.edu> > --- > drivers/char/ipmi/ipmi_msghandler.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c > index e1b22fe0916c..d34343e34272 100644 > --- a/drivers/char/ipmi/ipmi_msghandler.c > +++ b/drivers/char/ipmi/ipmi_msghandler.c > @@ -2583,10 +2583,11 @@ static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc, > *guid = bmc->guid; > } > > + kref_put(&intf->refcount, intf_free); > + > mutex_unlock(&bmc->dyn_mutex); > mutex_unlock(&intf->bmc_reg_mutex); > > - kref_put(&intf->refcount, intf_free); > return rv; > } > > -- > 2.17.1 >
| |