All of lore.kernel.org
 help / color / mirror / Atom feed
From: "krzysztof.sobota@nokia.com" <krzysztof.sobota@nokia.com>
To: Guenter Roeck <linux@roeck-us.net>,
	wim@linux-watchdog.org, linux-watchdog@vger.kernel.org,
	linux-kernel@vger.kernel.org
Cc: alexander.sverdlin@nokia.com
Subject: [PATCH v2] watchdog: initialize device before misc_register
Date: Thu, 16 Jul 2020 10:31:07 +0200	[thread overview]
Message-ID: <55fa2e05-9a99-b205-2dad-b797786af22a@nokia.com> (raw)
In-Reply-To: <a9338b3b-92d8-08b9-2f4e-59abceb1b307@roeck-us.net>

watchdog: initialize device before misc_register

When watchdog device is being registered, it calls misc_register that
makes watchdog available for systemd to open. This is a data race
scenario, because when device is open it may still have device struct
not initialized - this in turn causes a crash. This patch moves
device initialization before misc_register call and it solves the
problem printed below.

------------[ cut here ]------------
WARNING: CPU: 3 PID: 1 at lib/kobject.c:612 kobject_get+0x50/0x54
kobject: '(null)' ((ptrval)): is not initialized, yet kobject_get() is 
being called.
Modules linked in: k2_reset_status(O) davinci_wdt(+) 
sfn_platform_hwbcn(O) fsmddg_sfn(O) clk_misc_mmap(O) clk_sw_bcn(O) 
fsp_reset(O) cma_mod(O) slave_sup_notif(O) fpga_master(O) latency(O+) 
evnotify(O) enable_arm_pmu(O) xge(O) rio_mport_cdev br_netfilter bridge 
stp llc nvrd_checksum(O) ipv6
CPU: 3 PID: 1 Comm: systemd Tainted: G           O 
4.19.113-g2579778-fsm4_k2 #1
Hardware name: Keystone
[<c02126c4>] (unwind_backtrace) from [<c020da94>] (show_stack+0x18/0x1c)
[<c020da94>] (show_stack) from [<c07f87d8>] (dump_stack+0xb4/0xe8)
[<c07f87d8>] (dump_stack) from [<c0221f70>] (__warn+0xfc/0x114)
[<c0221f70>] (__warn) from [<c0221fd8>] (warn_slowpath_fmt+0x50/0x74)
[<c0221fd8>] (warn_slowpath_fmt) from [<c07fd394>] (kobject_get+0x50/0x54)
[<c07fd394>] (kobject_get) from [<c0602ce8>] (get_device+0x1c/0x24)
[<c0602ce8>] (get_device) from [<c06961e0>] (watchdog_open+0x90/0xf0)
[<c06961e0>] (watchdog_open) from [<c06001dc>] (misc_open+0x130/0x17c)
[<c06001dc>] (misc_open) from [<c0388228>] (chrdev_open+0xec/0x1a8)
[<c0388228>] (chrdev_open) from [<c037fa98>] (do_dentry_open+0x204/0x3cc)
[<c037fa98>] (do_dentry_open) from [<c0391e2c>] (path_openat+0x330/0x1148)
[<c0391e2c>] (path_openat) from [<c0394518>] (do_filp_open+0x78/0xec)
[<c0394518>] (do_filp_open) from [<c0381100>] (do_sys_open+0x130/0x1f4)
[<c0381100>] (do_sys_open) from [<c0201000>] (ret_fast_syscall+0x0/0x28)
Exception stack(0xd2ceffa8 to 0xd2cefff0)
ffa0:                   b6f69968 00000000 ffffff9c b6ebd210 000a0001 
00000000
ffc0: b6f69968 00000000 00000000 00000142 fffffffd ffffffff 00b65530 
bed7bb78
ffe0: 00000142 bed7ba70 b6cc2503 b6cc41d6
---[ end trace 7b16eb105513974f ]---

------------[ cut here ]------------
WARNING: CPU: 3 PID: 1 at lib/refcount.c:153 kobject_get+0x24/0x54
refcount_t: increment on 0; use-after-free.
Modules linked in: k2_reset_status(O) davinci_wdt(+) 
sfn_platform_hwbcn(O) fsmddg_sfn(O) clk_misc_mmap(O) clk_sw_bcn(O) 
fsp_reset(O) cma_mod(O) slave_sup_notif(O) fpga_master(O) latency(O+) 
evnotify(O) enable_arm_pmu(O) xge(O) rio_mport_cdev br_netfilter bridge 
stp llc nvrd_checksum(O) ipv6
CPU: 3 PID: 1 Comm: systemd Tainted: G        W  O 
4.19.113-g2579778-fsm4_k2 #1
Hardware name: Keystone
[<c02126c4>] (unwind_backtrace) from [<c020da94>] (show_stack+0x18/0x1c)
[<c020da94>] (show_stack) from [<c07f87d8>] (dump_stack+0xb4/0xe8)
[<c07f87d8>] (dump_stack) from [<c0221f70>] (__warn+0xfc/0x114)
[<c0221f70>] (__warn) from [<c0221fd8>] (warn_slowpath_fmt+0x50/0x74)
[<c0221fd8>] (warn_slowpath_fmt) from [<c07fd368>] (kobject_get+0x24/0x54)
[<c07fd368>] (kobject_get) from [<c0602ce8>] (get_device+0x1c/0x24)
[<c0602ce8>] (get_device) from [<c06961e0>] (watchdog_open+0x90/0xf0)
[<c06961e0>] (watchdog_open) from [<c06001dc>] (misc_open+0x130/0x17c)
[<c06001dc>] (misc_open) from [<c0388228>] (chrdev_open+0xec/0x1a8)
[<c0388228>] (chrdev_open) from [<c037fa98>] (do_dentry_open+0x204/0x3cc)
[<c037fa98>] (do_dentry_open) from [<c0391e2c>] (path_openat+0x330/0x1148)
[<c0391e2c>] (path_openat) from [<c0394518>] (do_filp_open+0x78/0xec)
[<c0394518>] (do_filp_open) from [<c0381100>] (do_sys_open+0x130/0x1f4)
[<c0381100>] (do_sys_open) from [<c0201000>] (ret_fast_syscall+0x0/0x28)
Exception stack(0xd2ceffa8 to 0xd2cefff0)
ffa0:                   b6f69968 00000000 ffffff9c b6ebd210 000a0001 
00000000
ffc0: b6f69968 00000000 00000000 00000142 fffffffd ffffffff 00b65530 
bed7bb78
ffe0: 00000142 bed7ba70 b6cc2503 b6cc41d6
---[ end trace 7b16eb1055139750 ]---

Fixes: 72139dfa2464 ("watchdog: Fix the race between the release of 
watchdog_core_data and cdev")
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Krzysztof Sobota <krzysztof.sobota@nokia.com>
---
v1 -> v2:
* removed Change-Id tag
* added Review-by tag
---
  drivers/watchdog/watchdog_dev.c | 18 +++++++++---------
  1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c 
b/drivers/watchdog/watchdog_dev.c
index 10b2090f3e5e..1c322caecf7f 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -947,6 +947,15 @@ static int watchdog_cdev_register(struct 
watchdog_device *wdd)
         if (IS_ERR_OR_NULL(watchdog_kworker))
                 return -ENODEV;

+       device_initialize(&wd_data->dev);
+       wd_data->dev.devt = MKDEV(MAJOR(watchdog_devt), wdd->id);
+       wd_data->dev.class = &watchdog_class;
+       wd_data->dev.parent = wdd->parent;
+       wd_data->dev.groups = wdd->groups;
+       wd_data->dev.release = watchdog_core_data_release;
+       dev_set_drvdata(&wd_data->dev, wdd);
+       dev_set_name(&wd_data->dev, "watchdog%d", wdd->id);
+
         kthread_init_work(&wd_data->work, watchdog_ping_work);
         hrtimer_init(&wd_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
         wd_data->timer.function = watchdog_timer_expired;
@@ -967,15 +976,6 @@ static int watchdog_cdev_register(struct 
watchdog_device *wdd)
                 }
         }

-       device_initialize(&wd_data->dev);
-       wd_data->dev.devt = MKDEV(MAJOR(watchdog_devt), wdd->id);
-       wd_data->dev.class = &watchdog_class;
-       wd_data->dev.parent = wdd->parent;
-       wd_data->dev.groups = wdd->groups;
-       wd_data->dev.release = watchdog_core_data_release;
-       dev_set_drvdata(&wd_data->dev, wdd);
-       dev_set_name(&wd_data->dev, "watchdog%d", wdd->id);
-
         /* Fill in the data structures */
         cdev_init(&wd_data->cdev, &watchdog_fops);

--
2.14.0


  reply	other threads:[~2020-07-16  8:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-15 13:57 [PATCH] watchdog: initialize device before misc_register krzysztof.sobota
2020-07-15 15:09 ` Guenter Roeck
2020-07-16  8:31   ` krzysztof.sobota [this message]
2020-07-16 11:32     ` [PATCH v3] " krzysztof.sobota
2020-07-16 11:48       ` Alexander Sverdlin
2020-07-16 14:00       ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55fa2e05-9a99-b205-2dad-b797786af22a@nokia.com \
    --to=krzysztof.sobota@nokia.com \
    --cc=alexander.sverdlin@nokia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.