Skip to content

Commit 8526ad9

Browse files
TaeheeYookuba-moo
authored andcommitted
netdevsim: fix panic in nsim_dev_take_snapshot_write()
nsim_dev_take_snapshot_write() uses nsim_dev and nsim_dev->dummy_region. So, during this function, these data shouldn't be removed. But there is no protecting stuff in this function. There are two similar cases. 1. reload case reload could be called during nsim_dev_take_snapshot_write(). When reload is being executed, nsim_dev_reload_down() is called and it calls nsim_dev_reload_destroy(). nsim_dev_reload_destroy() calls devlink_region_destroy() to destroy nsim_dev->dummy_region. So, during nsim_dev_take_snapshot_write(), nsim_dev->dummy_region() would be removed. At this point, snapshot_write() would access freed pointer. In order to fix this case, take_snapshot file will be removed before devlink_region_destroy(). The take_snapshot file will be re-created by ->reload_up(). 2. del_device_store case del_device_store() also could call nsim_dev_reload_destroy() during nsim_dev_take_snapshot_write(). If so, panic would occur. This problem is actually the same problem with the first case. So, this problem will be fixed by the first case's solution. Test commands: modprobe netdevsim while : do echo 1 > /sys/bus/netdevsim/new_device & echo 1 > /sys/bus/netdevsim/del_device & devlink dev reload netdevsim/netdevsim1 & echo 1 > /sys/kernel/debug/netdevsim/netdevsim1/take_snapshot & done Splat looks like: [ 45.564513][ T975] general protection fault, probably for non-canonical address 0xdffffc000000003a: 0000 [#1] SMP DEI [ 45.566131][ T975] KASAN: null-ptr-deref in range [0x00000000000001d0-0x00000000000001d7] [ 45.566135][ T975] CPU: 1 PID: 975 Comm: bash Not tainted 5.5.0+ #322 [ 45.569020][ T975] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 45.569026][ T975] RIP: 0010:__mutex_lock+0x10a/0x14b0 [ 45.570518][ T975] Code: 08 84 d2 0f 85 7f 12 00 00 44 8b 0d 10 23 65 02 45 85 c9 75 29 49 8d 7f 68 48 b8 00 00 00 0f [ 45.570522][ T975] RSP: 0018:ffff888046ccfbf0 EFLAGS: 00010206 [ 45.572305][ T975] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 [ 45.572308][ T975] RDX: 000000000000003a RSI: ffffffffac926440 RDI: 00000000000001d0 [ 45.576843][ T975] RBP: ffff888046ccfd70 R08: ffffffffab610645 R09: 0000000000000000 [ 45.576847][ T975] R10: ffff888046ccfd90 R11: ffffed100d6360ad R12: 0000000000000000 [ 45.578471][ T975] R13: dffffc0000000000 R14: ffffffffae1976c0 R15: 0000000000000168 [ 45.578475][ T975] FS: 00007f614d6e7740(0000) GS:ffff88806c400000(0000) knlGS:0000000000000000 [ 45.581492][ T975] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 45.582942][ T975] CR2: 00005618677d1cf0 CR3: 000000005fb9c002 CR4: 00000000000606e0 [ 45.584543][ T975] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 45.586633][ T975] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 45.589889][ T975] Call Trace: [ 45.591445][ T975] ? devlink_region_snapshot_create+0x55/0x4a0 [ 45.601250][ T975] ? mutex_lock_io_nested+0x1380/0x1380 [ 45.602817][ T975] ? mutex_lock_io_nested+0x1380/0x1380 [ 45.603875][ T975] ? mark_held_locks+0xa5/0xe0 [ 45.604769][ T975] ? _raw_spin_unlock_irqrestore+0x2d/0x50 [ 45.606147][ T975] ? __mutex_unlock_slowpath+0xd0/0x670 [ 45.607723][ T975] ? crng_backtrack_protect+0x80/0x80 [ 45.613530][ T975] ? wait_for_completion+0x390/0x390 [ 45.615152][ T975] ? devlink_region_snapshot_create+0x55/0x4a0 [ 45.616834][ T975] devlink_region_snapshot_create+0x55/0x4a0 [ ... ] Fixes: 4418f86 ("netdevsim: implement support for devlink region and snapshots") Signed-off-by: Taehee Yoo <[email protected]> Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 6ab6336 commit 8526ad9

File tree

2 files changed

+12
-2
lines changed

2 files changed

+12
-2
lines changed

drivers/net/netdevsim/dev.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,11 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
8888
&nsim_dev->max_macs);
8989
debugfs_create_bool("test1", 0600, nsim_dev->ddir,
9090
&nsim_dev->test1);
91-
debugfs_create_file("take_snapshot", 0200, nsim_dev->ddir, nsim_dev,
92-
&nsim_dev_take_snapshot_fops);
91+
nsim_dev->take_snapshot = debugfs_create_file("take_snapshot",
92+
0200,
93+
nsim_dev->ddir,
94+
nsim_dev,
95+
&nsim_dev_take_snapshot_fops);
9396
debugfs_create_bool("dont_allow_reload", 0600, nsim_dev->ddir,
9497
&nsim_dev->dont_allow_reload);
9598
debugfs_create_bool("fail_reload", 0600, nsim_dev->ddir,
@@ -740,6 +743,11 @@ static int nsim_dev_reload_create(struct nsim_dev *nsim_dev,
740743
if (err)
741744
goto err_health_exit;
742745

746+
nsim_dev->take_snapshot = debugfs_create_file("take_snapshot",
747+
0200,
748+
nsim_dev->ddir,
749+
nsim_dev,
750+
&nsim_dev_take_snapshot_fops);
743751
return 0;
744752

745753
err_health_exit:
@@ -853,6 +861,7 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
853861

854862
if (devlink_is_reload_failed(devlink))
855863
return;
864+
debugfs_remove(nsim_dev->take_snapshot);
856865
nsim_dev_port_del_all(nsim_dev);
857866
nsim_dev_health_exit(nsim_dev);
858867
nsim_dev_traps_exit(devlink);

drivers/net/netdevsim/netdevsim.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ struct nsim_dev {
160160
struct nsim_trap_data *trap_data;
161161
struct dentry *ddir;
162162
struct dentry *ports_ddir;
163+
struct dentry *take_snapshot;
163164
struct bpf_offload_dev *bpf_dev;
164165
bool bpf_bind_accept;
165166
u32 bpf_bind_verifier_delay;

0 commit comments

Comments
 (0)