Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

watchdog: Add Locking support

This patch fixes some potential multithreading issues, despite only
allowing one process to open the /dev/watchdog device, we can still get
called multiple times at the same time, since a program could be using thread,
or could share the fd after a fork.

This causes 2 potential problems:
1) watchdog_start / open do an unlocked test_n_set / test_n_clear,
if these 2 race, the watchdog could be stopped while the active
bit indicates it is running or visa versa.

2) Most watchdog_dev drivers probably assume that only one
watchdog-op will get called at a time, this is not necessary
true atm.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

authored by

Hans de Goede and committed by
Wim Van Sebroeck
f4e9c82f 7a879824

+29
+2
Documentation/watchdog/watchdog-kernel-api.txt
··· 50 50 unsigned int min_timeout; 51 51 unsigned int max_timeout; 52 52 void *driver_data; 53 + struct mutex lock; 53 54 unsigned long status; 54 55 }; 55 56 ··· 75 74 * driver_data: a pointer to the drivers private data of a watchdog device. 76 75 This data should only be accessed via the watchdog_set_drvdata and 77 76 watchdog_get_drvdata routines. 77 + * lock: Mutex for WatchDog Timer Driver Core internal use only. 78 78 * status: this field contains a number of status bits that give extra 79 79 information about the status of the device (Like: is the watchdog timer 80 80 running/active, is the nowayout bit set, is the device opened via
+1
drivers/watchdog/watchdog_core.c
··· 79 79 * corrupted in a later stage then we expect a kernel panic! 80 80 */ 81 81 82 + mutex_init(&wdd->lock); 82 83 id = ida_simple_get(&watchdog_ida, 0, MAX_DOGS, GFP_KERNEL); 83 84 if (id < 0) 84 85 return id;
+21
drivers/watchdog/watchdog_dev.c
··· 63 63 { 64 64 int err = 0; 65 65 66 + mutex_lock(&wddev->lock); 67 + 66 68 if (!watchdog_active(wddev)) 67 69 goto out_ping; 68 70 ··· 74 72 err = wddev->ops->start(wddev); /* restart watchdog */ 75 73 76 74 out_ping: 75 + mutex_unlock(&wddev->lock); 77 76 return err; 78 77 } 79 78 ··· 91 88 { 92 89 int err = 0; 93 90 91 + mutex_lock(&wddev->lock); 92 + 94 93 if (watchdog_active(wddev)) 95 94 goto out_start; 96 95 ··· 101 96 set_bit(WDOG_ACTIVE, &wddev->status); 102 97 103 98 out_start: 99 + mutex_unlock(&wddev->lock); 104 100 return err; 105 101 } 106 102 ··· 119 113 { 120 114 int err = 0; 121 115 116 + mutex_lock(&wddev->lock); 117 + 122 118 if (!watchdog_active(wddev)) 123 119 goto out_stop; 124 120 ··· 135 127 clear_bit(WDOG_ACTIVE, &wddev->status); 136 128 137 129 out_stop: 130 + mutex_unlock(&wddev->lock); 138 131 return err; 139 132 } 140 133 ··· 156 147 if (!wddev->ops->status) 157 148 return -EOPNOTSUPP; 158 149 150 + mutex_lock(&wddev->lock); 151 + 159 152 *status = wddev->ops->status(wddev); 160 153 154 + mutex_unlock(&wddev->lock); 161 155 return err; 162 156 } 163 157 ··· 183 171 (timeout < wddev->min_timeout || timeout > wddev->max_timeout)) 184 172 return -EINVAL; 185 173 174 + mutex_lock(&wddev->lock); 175 + 186 176 err = wddev->ops->set_timeout(wddev, timeout); 187 177 178 + mutex_unlock(&wddev->lock); 188 179 return err; 189 180 } 190 181 ··· 208 193 if (!wddev->ops->get_timeleft) 209 194 return -EOPNOTSUPP; 210 195 196 + mutex_lock(&wddev->lock); 197 + 211 198 *timeleft = wddev->ops->get_timeleft(wddev); 212 199 200 + mutex_unlock(&wddev->lock); 213 201 return err; 214 202 } 215 203 ··· 231 213 if (!wddev->ops->ioctl) 232 214 return -ENOIOCTLCMD; 233 215 216 + mutex_lock(&wddev->lock); 217 + 234 218 err = wddev->ops->ioctl(wddev, cmd, arg); 235 219 220 + mutex_unlock(&wddev->lock); 236 221 return err; 237 222 } 238 223
+5
include/linux/watchdog.h
··· 104 104 * @min_timeout:The watchdog devices minimum timeout value. 105 105 * @max_timeout:The watchdog devices maximum timeout value. 106 106 * @driver-data:Pointer to the drivers private data. 107 + * @lock: Lock for watchdog core internal use only. 107 108 * @status: Field that contains the devices internal status bits. 108 109 * 109 110 * The watchdog_device structure contains all information about a ··· 112 111 * 113 112 * The driver-data field may not be accessed directly. It must be accessed 114 113 * via the watchdog_set_drvdata and watchdog_get_drvdata helpers. 114 + * 115 + * The lock field is for watchdog core internal use only and should not be 116 + * touched. 115 117 */ 116 118 struct watchdog_device { 117 119 int id; ··· 128 124 unsigned int min_timeout; 129 125 unsigned int max_timeout; 130 126 void *driver_data; 127 + struct mutex lock; 131 128 unsigned long status; 132 129 /* Bit numbers for status flags */ 133 130 #define WDOG_ACTIVE 0 /* Is the watchdog running/active */