On 10/9/25 16:05, Vasileios Almpanis wrote:
The ploop_add_delta() function calls file_clone_open() to create multiple file handles, but only checks for NULL return values. However, file_clone_open() can return ERR_PTR() on failure, which gets stored as a corrupted file pointer in deltas[level].mtfile[i].Later, during cleanup in ploop_put_delta(), fput() is called on these corrupted pointers , causing the kernel to attempt accessing invalid memory addresses resulting in NULL pointer dereference crashes. BUG: kernel NULL pointer dereference, address: 0000000000000034 #PF: supervisor write access in kernel mode #PF: error_code(0x0002) - not-present page PGD 0 P4D 0 Oops: 0002 [#1] PREEMPT SMP PTI CPU: 6 PID: 274635 Comm: dmsetup ve: / Kdump: loaded Tainted: G X ------- --- 5.14.0-427.77.1.vz9.86.12 #1 86.12 Hardware name: Supermicro SYS-5039MS-H12NR/X11SSE-F, BIOS 2.3 12/12/2019 RIP: 0010:fput_many+0x7/0x90 RSP: 0018:ffffa55e45ab3bd0 EFLAGS: 00010286 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000001 RSI: 0000000000000001 RDI: fffffffffffffffc RBP: ffff8aaf304a2800 R08: ffff8aaebbc83800 R09: 20c49ba5e353f7cf R10: ffff8aaec85428f0 R11: ffffffffc05c41fc R12: ffff8aaf287d9f40 R13: ffff8aaf28692000 R14: 0000000000000004 R15: 0000000000000003 FS: 00007feb47a0d840(0000) GS:ffff8abc77b80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000034 CR3: 0000000323e94003 CR4: 00000000003706e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> ? show_trace_log_lvl+0x1c4/0x2df ? show_trace_log_lvl+0x1c4/0x2df ? ploop_put_delta+0x51/0x76 [ploop] ? __die_body.cold+0x8/0xd ? page_fault_oops+0x134/0x170 ? exc_page_fault+0x62/0x150 ? asm_exc_page_fault+0x22/0x30 ? fuse_generic_request_alloc+0x1c/0x80 [fuse] ? fput_many+0x7/0x90 ploop_put_delta+0x51/0x76 [ploop] ploop_destroy+0x17b/0x2a0 [ploop] dm_table_destroy+0x59/0x120 [dm_mod] dev_remove+0xdb/0x1a0 [dm_mod] ctl_ioctl+0x1a5/0x290 [dm_mod] dm_ctl_ioctl+0xa/0x20 [dm_mod] __x64_sys_ioctl+0x87/0xc0 do_syscall_64+0x59/0x90 ? syscall_exit_work+0x103/0x130 ? syscall_exit_to_user_mode+0x19/0x40 ? do_syscall_64+0x69/0x90 ? exit_to_user_mode_loop+0xd0/0x130 ? exit_to_user_mode_prepare+0xb6/0x100 ? syscall_exit_to_user_mode+0x19/0x40 ? do_syscall_64+0x69/0x90 ? do_user_addr_fault+0x1d6/0x6a0 ? syscall_exit_work+0x103/0x130 ? exc_page_fault+0x62/0x150 entry_SYSCALL_64_after_hwframe+0x77/0xe1 RIP: 0033:0x7feb4783ec6b RSP: 002b:00007ffdc8dc7c98 EFLAGS: 00000206 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 00007feb47c0fbd0 RCX: 00007feb4783ec6b RDX: 00005651f9b00c80 RSI: 00000000c138fd04 RDI: 0000000000000003 RBP: 00007feb47c4c316 R08: 00007feb47c4f2d8 R09: 00007ffdc8dc7af0 R10: 00007feb47c4c316 R11: 0000000000000206 R12: 00005651f9b00c80 R13: 00005651f9b00d30 R14: 00007feb47c4c316 R15: 00005651f9b00a00 </TASK> CR2: 0000000000000034 Fix by: 1. Using IS_ERR() to properly detect error pointers from file_clone_open() 2. Adding IS_ERR() checks in ploop_put_delta() to prevent cleanup crashes 3. Setting failed mtfile entries to NULL to avoid double-free issues https://virtuozzo.atlassian.net/browse/VSTOR-116965 Signed-off-by: Vasileios Almpanis <[email protected]> Feature: dm-ploop: ploop target driver --- drivers/md/dm-ploop-bat.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/md/dm-ploop-bat.c b/drivers/md/dm-ploop-bat.c index a0b678b30b97..65c74960f29c 100644 --- a/drivers/md/dm-ploop-bat.c +++ b/drivers/md/dm-ploop-bat.c @@ -511,8 +511,9 @@ int ploop_add_delta(struct ploop *ploop, u32 level, struct file *file, bool is_r } for (i = 0; i < ploop->nkt_runners; i++) { deltas[level].mtfile[i] = file_clone_open(file); - if (!deltas[level].mtfile[i]) { - ret = ENOMEM; + if (IS_ERR(deltas[level].mtfile[i])) { + ret = PTR_ERR(deltas[level].mtfile[i]); + deltas[level].mtfile[i] = NULL; goto out; } } @@ -531,14 +532,14 @@ void ploop_put_delta(struct ploop *ploop, struct ploop_delta *delta) { int i;- if (delta->file) {+ if (delta->file && !IS_ERR(delta->file)) { vfs_fsync(delta->file, 1); fput(delta->file); }if (delta->mtfile) {for (i = 0; i < ploop->nkt_runners; i++) { - if (delta->mtfile[i]) + if (delta->mtfile[i] && !IS_ERR(delta->mtfile[i])) fput(delta->mtfile[i]); } kfree(delta->mtfile);
for me this looks absolutely cumbersome. Please do not assign such values to the permanent storage. Keep NULL there. Assign the return code to the temporary variable and check the result. Den _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
