[Devel] [PATCH vz9/vz10] dm-ploop: null pointer dereference in ploop_put_delta()

[Denis V. Lunev]

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 <vasileios.almpanis at virtuozzo.com>
>
> 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