Eric Lee / smarc-fsl-linux-kernel

Download as
Browse Code ยป

Commit 69503e585192fdd84b240f18a0873d20e18a2e0a

Authored by Vladis Dronov
Committed by Wim Van Sebroeck
1 parent b1301b9022

watchdog: fix UAF in reboot notifier handling in watchdog core code 

After the commit 44ea39420fc9 ("drivers/watchdog: make use of
devm_register_reboot_notifier()") the struct notifier_block reboot_nb in
the struct watchdog_device is removed from the reboot notifiers chain at
the time watchdog's chardev is closed. But at least in i6300esb.c case
reboot_nb is embedded in the struct esb_dev which can be freed on its
device removal and before the chardev is closed, thus UAF at reboot:

[    7.728581] esb_probe: esb_dev.watchdog_device ffff91316f91ab28
ts# uname -r                            note the address ^^^
5.5.0-rc5-ae6088-wdog
ts# ./openwdog0 &
[1] 696
ts# opened /dev/watchdog0, sleeping 10s...
ts# echo 1 > /sys/devices/pci0000\:00/0000\:00\:09.0/remove
[  178.086079] devres:rel_nodes: dev ffff91317668a0b0 data ffff91316f91ab28
           esb_dev.watchdog_device.reboot_nb memory is freed here ^^^
ts# ...woken up
[  181.459010] devres:rel_nodes: dev ffff913171781000 data ffff913174a1dae8
[  181.460195] devm_unreg_reboot_notifier: res ffff913174a1dae8 nb ffff91316f91ab78
                                     attempt to use memory already freed ^^^
[  181.461063] devm_unreg_reboot_notifier: nb->call 6b6b6b6b6b6b6b6b
[  181.461243] devm_unreg_reboot_notifier: nb->next 6b6b6b6b6b6b6b6b
                freed memory is filled with a slub poison ^^^
[1]+  Done                    ./openwdog0
ts# reboot
[  229.921862] systemd-shutdown[1]: Rebooting.
[  229.939265] notifier_call_chain: nb ffffffff9c6c2f20 nb->next ffffffff9c6d50c0
[  229.943080] notifier_call_chain: nb ffffffff9c6d50c0 nb->next 6b6b6b6b6b6b6b6b
[  229.946054] notifier_call_chain: nb 6b6b6b6b6b6b6b6b INVAL
[  229.957584] general protection fault: 0000 [#1] SMP
[  229.958770] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 5.5.0-rc5-ae6088-wdog
[  229.960224] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), ...
[  229.963288] RIP: 0010:notifier_call_chain+0x66/0xd0
[  229.969082] RSP: 0018:ffffb20dc0013d88 EFLAGS: 00010246
[  229.970812] RAX: 000000000000002e RBX: 6b6b6b6b6b6b6b6b RCX: 00000000000008b3
[  229.972929] RDX: 0000000000000000 RSI: 0000000000000096 RDI: ffffffff9ccc46ac
[  229.975028] RBP: 0000000000000001 R08: 0000000000000000 R09: 00000000000008b3
[  229.977039] R10: 0000000000000001 R11: ffffffff9c26c740 R12: 0000000000000000
[  229.979155] R13: 6b6b6b6b6b6b6b6b R14: 0000000000000000 R15: 00000000fffffffa
...   slub_debug=FZP poison ^^^
[  229.989089] Call Trace:
[  229.990157]  blocking_notifier_call_chain+0x43/0x59
[  229.991401]  kernel_restart_prepare+0x14/0x30
[  229.992607]  kernel_restart+0x9/0x30
[  229.993800]  __do_sys_reboot+0x1d2/0x210
[  230.000149]  do_syscall_64+0x3d/0x130
[  230.001277]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  230.002639] RIP: 0033:0x7f5461bdd177
[  230.016402] Modules linked in: i6300esb
[  230.050261] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Fix the crash by reverting 44ea39420fc9 so unregister_reboot_notifier()
is called when watchdog device is removed. This also makes handling of
the reboot notifier unified with the handling of the restart handler,
which is freed with unregister_restart_handler() in the same place.

Fixes: 44ea39420fc9 ("drivers/watchdog: make use of devm_register_reboot_notifier()")
Cc: stable@vger.kernel.org # v4.15+
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20200108125347.6067-1-vdronov@redhat.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Wim Van Sebroeck <wim@linux-watchdog.org>

Showing 2 changed files with 36 additions and 35 deletions Side-by-side Diff

drivers/watchdog/watchdog_core.c
Diff comments   View file @ 69503e5
... ... @@ -147,6 +147,25 @@
147 147 }
148 148 EXPORT_SYMBOL_GPL(watchdog_init_timeout);
149 149  
  150 +static int watchdog_reboot_notifier(struct notifier_block *nb,
  151 + unsigned long code, void *data)
  152 +{
  153 + struct watchdog_device *wdd;
  154 +
  155 + wdd = container_of(nb, struct watchdog_device, reboot_nb);
  156 + if (code == SYS_DOWN || code == SYS_HALT) {
  157 + if (watchdog_active(wdd)) {
  158 + int ret;
  159 +
  160 + ret = wdd->ops->stop(wdd);
  161 + if (ret)
  162 + return NOTIFY_BAD;
  163 + }
  164 + }
  165 +
  166 + return NOTIFY_DONE;
  167 +}
  168 +
150 169 static int watchdog_restart_notifier(struct notifier_block *nb,
151 170 unsigned long action, void *data)
152 171 {
... ... @@ -235,6 +254,19 @@
235 254 }
236 255 }
237 256  
  257 + if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {
  258 + wdd->reboot_nb.notifier_call = watchdog_reboot_notifier;
  259 +
  260 + ret = register_reboot_notifier(&wdd->reboot_nb);
  261 + if (ret) {
  262 + pr_err("watchdog%d: Cannot register reboot notifier (%d)\n",
  263 + wdd->id, ret);
  264 + watchdog_dev_unregister(wdd);
  265 + ida_simple_remove(&watchdog_ida, id);
  266 + return ret;
  267 + }
  268 + }
  269 +
238 270 if (wdd->ops->restart) {
239 271 wdd->restart_nb.notifier_call = watchdog_restart_notifier;
240 272  
... ... @@ -288,6 +320,9 @@
288 320  
289 321 if (wdd->ops->restart)
290 322 unregister_restart_handler(&wdd->restart_nb);
  323 +
  324 + if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status))
  325 + unregister_reboot_notifier(&wdd->reboot_nb);
291 326  
292 327 watchdog_dev_unregister(wdd);
293 328 ida_simple_remove(&watchdog_ida, wdd->id);
drivers/watchdog/watchdog_dev.c
Diff comments   View file @ 69503e5
... ... @@ -38,7 +38,6 @@
38 38 #include <linux/miscdevice.h> /* For handling misc devices */
39 39 #include <linux/module.h> /* For module stuff/... */
40 40 #include <linux/mutex.h> /* For mutexes */
41   -#include <linux/reboot.h> /* For reboot notifier */
42 41 #include <linux/slab.h> /* For memory functions */
43 42 #include <linux/types.h> /* For standard types (like size_t) */
44 43 #include <linux/watchdog.h> /* For watchdog specific items */
... ... @@ -1097,25 +1096,6 @@
1097 1096 put_device(&wd_data->dev);
1098 1097 }
1099 1098  
1100   -static int watchdog_reboot_notifier(struct notifier_block *nb,
1101   - unsigned long code, void *data)
1102   -{
1103   - struct watchdog_device *wdd;
1104   -
1105   - wdd = container_of(nb, struct watchdog_device, reboot_nb);
1106   - if (code == SYS_DOWN || code == SYS_HALT) {
1107   - if (watchdog_active(wdd)) {
1108   - int ret;
1109   -
1110   - ret = wdd->ops->stop(wdd);
1111   - if (ret)
1112   - return NOTIFY_BAD;
1113   - }
1114   - }
1115   -
1116   - return NOTIFY_DONE;
1117   -}
1118   -
1119 1099 /*
1120 1100 * watchdog_dev_register: register a watchdog device
1121 1101 * @wdd: watchdog device
1122 1102  
... ... @@ -1134,22 +1114,8 @@
1134 1114 return ret;
1135 1115  
1136 1116 ret = watchdog_register_pretimeout(wdd);
1137   - if (ret) {
  1117 + if (ret)
1138 1118 watchdog_cdev_unregister(wdd);
1139   - return ret;
1140   - }
1141   -
1142   - if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {
1143   - wdd->reboot_nb.notifier_call = watchdog_reboot_notifier;
1144   -
1145   - ret = devm_register_reboot_notifier(&wdd->wd_data->dev,
1146   - &wdd->reboot_nb);
1147   - if (ret) {
1148   - pr_err("watchdog%d: Cannot register reboot notifier (%d)\n",
1149   - wdd->id, ret);
1150   - watchdog_dev_unregister(wdd);
1151   - }
1152   - }
1153 1119  
1154 1120 return ret;
1155 1121 }