drivers: char: tlclk.c: Avoid data race between init and interrupt handler
diff mbox series

Message ID 20200417153451.1551-1-madhuparnabhowmik10@gmail.com
State Accepted
Commit 44b8fb6eaa7c3fb770bf1e37619cdb3902cca1fc
Headers show
Series
  • drivers: char: tlclk.c: Avoid data race between init and interrupt handler
Related show

Commit Message

Madhuparna Bhowmik April 17, 2020, 3:34 p.m. UTC
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(-)

Comments

Gross, Mark April 17, 2020, 3:51 p.m. UTC | #1
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

Patch
diff mbox series

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;
 }