linux-kernel - Re: [PATCH] watchdog: Fix race condition in registration code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <201304061019.34473.a.miskiewicz@gmail.com>
Date:	Sat, 6 Apr 2013 10:19:34 +0200
From:	Arkadiusz Miskiewicz <a.miskiewicz@...il.com>
To:	Guenter Roeck <linux@...ck-us.net>
Cc:	linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org,
	Wim Van Sebroeck <wim@...ana.be>
Subject: Re: [PATCH] watchdog: Fix race condition in registration code

On Saturday 06 of April 2013, Guenter Roeck wrote:
> A race condition exists when registering the first watchdog device.
> Sequence of events:
> 
> - watchdog_register_device calls watchdog_dev_register
> - watchdog_dev_register creates the watchdog misc device by calling
>   misc_register.
>   At that time, the matching character device (/dev/watchdog0) does not yet
>   exist, and old_wdd is not set either.
> - Userspace gets an event and opens /dev/watchdog
> - watchdog_open is called and sets sets wdd = old_wdd, which is still NULL,
>   and tries to dereference it. This causes the kernel to panic.
> 
> Seen with systemd trying to open /dev/watchdog immediately after
> it was created.
> 
> Reported-by: Arkadiusz Miskiewicz <a.miskiewicz@...il.com>

Please use
Reported-by: Arkadiusz Miśkiewicz <arekm@...en.pl>

I have to use gmail address because maven.pl domain is blocked due to some 
unknown, secret reason and vger.kernel.org postmasters (Dave M etc) are less 
than helpful:

"We are under no obligation to explain why you were banned nor to remove
the ban.

If you don't like this, you can run your own list server and on it determine
your own set of policies."


> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
> Arkadiusz,
> 
> would be great if you can test this in your system.

Did few reboots without oops but this test isn't reliable. Previously I wasn't 
able to reproduce this on demand. It just happens sometime. If any problem 
popup I'll let you know.

So for now
Tested-by: Arkadiusz Miśkiewicz <arekm@...en.pl>

> 
>  drivers/watchdog/watchdog_dev.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/watchdog_dev.c
> b/drivers/watchdog/watchdog_dev.c index 08b48bb..faf4e18 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -523,6 +523,7 @@ int watchdog_dev_register(struct watchdog_device
> *watchdog) int err, devno;
> 
>  	if (watchdog->id == 0) {
> +		old_wdd = watchdog;
>  		watchdog_miscdev.parent = watchdog->parent;
>  		err = misc_register(&watchdog_miscdev);
>  		if (err != 0) {
> @@ -531,9 +532,9 @@ int watchdog_dev_register(struct watchdog_device
> *watchdog) if (err == -EBUSY)
>  				pr_err("%s: a legacy watchdog module is probably present.\n",
>  					watchdog->info->identity);
> +			old_wdd = NULL;
>  			return err;
>  		}
> -		old_wdd = watchdog;
>  	}
> 
>  	/* Fill in the data structures */


-- 
Arkadiusz Miśkiewicz, arekm / maven.pl
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/