Skip to content

Commit 16e5184

Browse files
TaeheeYoointel-lab-lkp
authored andcommitted
netdevsim: fix race conditions in netdevsim operations
This patch fixes a several locking problem. 1. netdevsim basic operations(new_device, del_device, new_port, and del_port) are called with sysfs. These operations use the same resource so they should acquire a lock for the whole resource not only for a list. 2. devices are managed by nsim_bus_dev_list. and all devices are deleted in the __exit() routine. After delete routine, new_device() and del_device() should be disallowed. So, the global flag variable 'enable' is added. 3. new_port() and del_port() would be called before resources are allocated or initialized. If so, panic will occur. In order to avoid this scenario, variable 'nsim_bus_dev->init' is added. Test commands: #SHELL1 modprobe netdevsim while : do echo "1 1" > /sys/bus/netdevsim/new_device echo "1 1" > /sys/bus/netdevsim/del_device done #SHELL2 while : do echo 1 > /sys/devices/netdevsim1/new_port echo 1 > /sys/devices/netdevsim1/del_port done Splat looks like: [ 66.648015][ T1095] kasan: CONFIG_KASAN_INLINE enabled [ 66.660685][ T1095] kasan: GPF could be caused by NULL-ptr deref or user memory access [ 66.662106][ T1095] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI [ 66.663151][ T1095] CPU: 0 PID: 1095 Comm: bash Not tainted 5.5.0-rc6+ torvalds#318 [ 66.664046][ T1095] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 66.665308][ T1095] RIP: 0010:__mutex_lock+0x10a/0x14b0 [ 66.666056][ T1095] Code: 08 84 d2 0f 85 7f 12 00 00 44 8b 0d 70 c4 66 02 45 85 c9 75 29 49 8d 7f 68 48 b8 00 f [ 66.670158][ T1095] RSP: 0018:ffff8880d36efbb0 EFLAGS: 00010206 [ 66.672254][ T1095] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 [ 66.673392][ T1095] RDX: 0000000000000021 RSI: ffffffffbb922ac0 RDI: 0000000000000108 [ 66.674563][ T1095] RBP: ffff8880d36efd30 R08: ffffffffc033ead0 R09: 0000000000000000 [ 66.675731][ T1095] R10: ffff8880d36efd50 R11: ffff8880ca1f8040 R12: 0000000000000000 [ 66.676897][ T1095] R13: dffffc0000000000 R14: ffffffffbd17a7c0 R15: 00000000000000a0 [ 66.678005][ T1095] FS: 00007fe4a170f740(0000) GS:ffff8880d9c00000(0000) knlGS:0000000000000000 [ 66.679101][ T1095] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 66.679906][ T1095] CR2: 000055fa392f7ca0 CR3: 00000000b136a003 CR4: 00000000000606f0 [ 66.681467][ T1095] Call Trace: [ 66.681899][ T1095] ? nsim_dev_port_add+0x50/0x150 [netdevsim] [ 66.682681][ T1095] ? mutex_lock_io_nested+0x1380/0x1380 [ 66.683371][ T1095] ? _kstrtoull+0x76/0x160 [ 66.683819][ T1095] ? _parse_integer+0xf0/0xf0 [ 66.684324][ T1095] ? kernfs_fop_write+0x1cf/0x410 [ 66.684861][ T1095] ? sysfs_file_ops+0x160/0x160 [ 66.687441][ T1095] ? kstrtouint+0x86/0x110 [ 66.687961][ T1095] ? nsim_dev_port_add+0x50/0x150 [netdevsim] [ 66.688646][ T1095] nsim_dev_port_add+0x50/0x150 [netdevsim] [ 66.689269][ T1095] ? sysfs_file_ops+0x160/0x160 [ 66.690547][ T1095] new_port_store+0x99/0xb0 [netdevsim] [ 66.691114][ T1095] ? del_port_store+0xb0/0xb0 [netdevsim] [ 66.691699][ T1095] ? sysfs_file_ops+0x112/0x160 [ 66.692193][ T1095] ? sysfs_kf_write+0x3b/0x180 [ 66.692677][ T1095] kernfs_fop_write+0x276/0x410 [ 66.693176][ T1095] ? __sb_start_write+0x215/0x2e0 [ 66.693695][ T1095] vfs_write+0x197/0x4a0 [ 66.694136][ T1095] ksys_write+0x141/0x1d0 [ ... ] Fixes: f9d9db4 ("netdevsim: add bus attributes to add new and delete devices") Fixes: 794b2c0 ("netdevsim: extend device attrs to support port addition and deletion") Signed-off-by: Taehee Yoo <[email protected]>
1 parent fa865ba commit 16e5184

File tree

2 files changed

+28
-8
lines changed

2 files changed

+28
-8
lines changed

drivers/net/netdevsim/bus.c

+27-8
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616

1717
static DEFINE_IDA(nsim_bus_dev_ids);
1818
static LIST_HEAD(nsim_bus_dev_list);
19-
static DEFINE_MUTEX(nsim_bus_dev_list_lock);
19+
static DEFINE_MUTEX(nsim_bus_dev_ops_lock);
20+
static bool enable;
2021

2122
static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
2223
{
@@ -99,6 +100,8 @@ new_port_store(struct device *dev, struct device_attribute *attr,
99100
unsigned int port_index;
100101
int ret;
101102

103+
if (!nsim_bus_dev->init)
104+
return -EBUSY;
102105
ret = kstrtouint(buf, 0, &port_index);
103106
if (ret)
104107
return ret;
@@ -116,6 +119,8 @@ del_port_store(struct device *dev, struct device_attribute *attr,
116119
unsigned int port_index;
117120
int ret;
118121

122+
if (!nsim_bus_dev->init)
123+
return -EBUSY;
119124
ret = kstrtouint(buf, 0, &port_index);
120125
if (ret)
121126
return ret;
@@ -179,13 +184,21 @@ new_device_store(struct bus_type *bus, const char *buf, size_t count)
179184
pr_err("Format for adding new device is \"id port_count\" (uint uint).\n");
180185
return -EINVAL;
181186
}
187+
mutex_lock(&nsim_bus_dev_ops_lock);
188+
if (!enable) {
189+
mutex_unlock(&nsim_bus_dev_ops_lock);
190+
return -EBUSY;
191+
}
182192
nsim_bus_dev = nsim_bus_dev_new(id, port_count);
183-
if (IS_ERR(nsim_bus_dev))
193+
if (IS_ERR(nsim_bus_dev)) {
194+
mutex_unlock(&nsim_bus_dev_ops_lock);
184195
return PTR_ERR(nsim_bus_dev);
196+
}
197+
198+
nsim_bus_dev->init = true;
185199

186-
mutex_lock(&nsim_bus_dev_list_lock);
187200
list_add_tail(&nsim_bus_dev->list, &nsim_bus_dev_list);
188-
mutex_unlock(&nsim_bus_dev_list_lock);
201+
mutex_unlock(&nsim_bus_dev_ops_lock);
189202

190203
return count;
191204
}
@@ -214,7 +227,11 @@ del_device_store(struct bus_type *bus, const char *buf, size_t count)
214227
}
215228

216229
err = -ENOENT;
217-
mutex_lock(&nsim_bus_dev_list_lock);
230+
mutex_lock(&nsim_bus_dev_ops_lock);
231+
if (!enable) {
232+
mutex_unlock(&nsim_bus_dev_ops_lock);
233+
return -EBUSY;
234+
}
218235
list_for_each_entry_safe(nsim_bus_dev, tmp, &nsim_bus_dev_list, list) {
219236
if (nsim_bus_dev->dev.id != id)
220237
continue;
@@ -223,7 +240,7 @@ del_device_store(struct bus_type *bus, const char *buf, size_t count)
223240
err = 0;
224241
break;
225242
}
226-
mutex_unlock(&nsim_bus_dev_list_lock);
243+
mutex_unlock(&nsim_bus_dev_ops_lock);
227244
return !err ? count : err;
228245
}
229246
static BUS_ATTR_WO(del_device);
@@ -320,6 +337,7 @@ int nsim_bus_init(void)
320337
err = driver_register(&nsim_driver);
321338
if (err)
322339
goto err_bus_unregister;
340+
enable = true;
323341
return 0;
324342

325343
err_bus_unregister:
@@ -331,12 +349,13 @@ void nsim_bus_exit(void)
331349
{
332350
struct nsim_bus_dev *nsim_bus_dev, *tmp;
333351

334-
mutex_lock(&nsim_bus_dev_list_lock);
352+
mutex_lock(&nsim_bus_dev_ops_lock);
353+
enable = false;
335354
list_for_each_entry_safe(nsim_bus_dev, tmp, &nsim_bus_dev_list, list) {
336355
list_del(&nsim_bus_dev->list);
337356
nsim_bus_dev_del(nsim_bus_dev);
338357
}
339-
mutex_unlock(&nsim_bus_dev_list_lock);
358+
mutex_unlock(&nsim_bus_dev_ops_lock);
340359
driver_unregister(&nsim_driver);
341360
bus_unregister(&nsim_bus);
342361
}

drivers/net/netdevsim/netdevsim.h

+1
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ struct nsim_bus_dev {
240240
*/
241241
unsigned int num_vfs;
242242
struct nsim_vf_config *vfconfigs;
243+
bool init;
243244
};
244245

245246
int nsim_bus_init(void);

0 commit comments

Comments
 (0)