Messages in this thread | | | From | "Gross, Mark" <> | Subject | RE: [PATCH] drivers: char: tlclk.c: Avoid data race between init and interrupt handler | Date | Fri, 17 Apr 2020 15:51:08 +0000 |
| |
Looks reasonable to me. +1 Ack --mark
> -----Original Message----- > From: madhuparnabhowmik10@gmail.com <madhuparnabhowmik10@gmail.com> > Sent: Friday, April 17, 2020 8:35 AM > To: Gross, Mark <mark.gross@intel.com>; arnd@arndb.de; > gregkh@linuxfoundation.org > Cc: linux-kernel@vger.kernel.org; andrianov@ispras.ru; ldv- > project@linuxtesting.org; Madhuparna Bhowmik > <madhuparnabhowmik10@gmail.com> > Subject: [PATCH] drivers: char: tlclk.c: Avoid data race between init and interrupt > handler > > From: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com> > > After registering character device the file operation callbacks can be called. The > open callback registers interrupt handler. > Therefore interrupt handler can execute in parallel with rest of the init function. To > avoid such data race initialize telclk_interrupt variable and struct alarm_events > before registering character device. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com> > --- > drivers/char/tlclk.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/char/tlclk.c b/drivers/char/tlclk.c index > 6d81bb3bb503..896a3550fba9 100644 > --- a/drivers/char/tlclk.c > +++ b/drivers/char/tlclk.c > @@ -777,17 +777,21 @@ static int __init tlclk_init(void) { > int ret; > > + telclk_interrupt = (inb(TLCLK_REG7) & 0x0f); > + > + alarm_events = kzalloc( sizeof(struct tlclk_alarms), GFP_KERNEL); > + if (!alarm_events) { > + ret = -ENOMEM; > + goto out1; > + } > + > ret = register_chrdev(tlclk_major, "telco_clock", &tlclk_fops); > if (ret < 0) { > printk(KERN_ERR "tlclk: can't get major %d.\n", tlclk_major); > + kfree(alarm_events); > return ret; > } > tlclk_major = ret; > - alarm_events = kzalloc( sizeof(struct tlclk_alarms), GFP_KERNEL); > - if (!alarm_events) { > - ret = -ENOMEM; > - goto out1; > - } > > /* Read telecom clock IRQ number (Set by BIOS) */ > if (!request_region(TLCLK_BASE, 8, "telco_clock")) { @@ -796,7 +800,6 > @@ static int __init tlclk_init(void) > ret = -EBUSY; > goto out2; > } > - telclk_interrupt = (inb(TLCLK_REG7) & 0x0f); > > if (0x0F == telclk_interrupt ) { /* not MCPBL0010 ? */ > printk(KERN_ERR "telclk_interrupt = 0x%x non-mcpbl0010 hw.\n", > @@ -837,8 +840,8 @@ static int __init tlclk_init(void) > release_region(TLCLK_BASE, 8); > out2: > kfree(alarm_events); > -out1: > unregister_chrdev(tlclk_major, "telco_clock"); > +out1: > return ret; > } > > -- > 2.17.1
| |