Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panic in ioctl triggered whenever a nvlist without NV_UNIQUE_NAME is passed to lzc_sync #11281

Closed
codyps opened this issue Dec 4, 2020 · 1 comment
Labels
Type: Defect Incorrect behavior (e.g. crash, hang)

Comments

@codyps
Copy link

codyps commented Dec 4, 2020

System information

Type Version/Name
Distribution Name Arch Linux
Linux Kernel 5.9.11-arch2-1
Architecture x86_64
ZFS Version 2.0.0-1
SPL Version 2.0.0-1

(This also occurs in older versions of openzfs. 0.8.5 was tested with the same issue)

Describe the problem you're observing

Calling lzc_sync() with a nvlist created without NV_UNIQUE_NAME results in the process calling lzc_sync() being stuck in the D state forever, and a panic is noted in dmesg:

[   32.343923] VERIFY3(0 == nvlist_lookup_boolean_value(nvl, name, &rv)) failed (0 == 95)
[   32.343926] PANIC at fnvpair.c:323:fnvlist_lookup_boolean_value()

(full panic below)

Hang-check notes that the task is hung.

Describe how to reproduce the problem

Compile and Run this snippet on a system with a pool named "testpool".

#include <libzfs_core.h>

#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>

void ret_h(int r, char *msg, ...) {
	if (r != 0) {
		va_list va;
		va_start(va, msg);
		vfprintf(stderr, msg, va);
		va_end(va);
		exit(EXIT_FAILURE);
	}
}

int main(void)
{
	int r;
	nvlist_t *nv;
	// NOTE: by omitting NV_UNIQUE_NAME, we trigger a panic in the ioctl
	r = nvlist_alloc(&nv, 0, 0);
	ret_h(r, "nvlist_alloc\n");
	r = nvlist_add_boolean_value(nv, "force", B_TRUE);
	ret_h(r, "nvlist_add_boolean\n");

	libzfs_core_init();
	r = lzc_sync("testpool", nv, NULL);
	ret_h(r, "lzc_sync\n");

	printf("COMPLETE\n");

	return 0;
}

Include any warning/errors/backtraces from the system logs

dmesg panic
[   32.343923] VERIFY3(0 == nvlist_lookup_boolean_value(nvl, name, &rv)) failed (0 == 95)
[   32.343926] PANIC at fnvpair.c:323:fnvlist_lookup_boolean_value()
[   32.343927] Showing stack for process 3936
[   32.343930] CPU: 3 PID: 3936 Comm: sync-hang Tainted: P           OE     5.9.11-arch2-1 #1
[   32.343930] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[   32.343931] Call Trace:
[   32.344020]  dump_stack+0x6b/0x83
[   32.344028]  spl_panic+0xef/0x117 [spl]
[   32.344032]  ? _raw_spin_unlock+0x16/0x30
[   32.344075]  ? spa_open_common+0x2c7/0x4f0 [zfs]
[   32.344082]  fnvlist_lookup_boolean_value+0x6b/0x80 [znvpair]
[   32.344114]  zfs_ioc_pool_sync+0x51/0xe0 [zfs]
[   32.344147]  zfsdev_ioctl_common+0x3a0/0x880 [zfs]
[   32.344179]  zfsdev_ioctl+0x53/0xe0 [zfs]
[   32.344186]  __x64_sys_ioctl+0x83/0xb0
[   32.344194]  do_syscall_64+0x33/0x40
[   32.344196]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   32.344198] RIP: 0033:0x7f60d0079f6b
[   32.344200] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d d5 ae 0c 00 f7 d8 64 89 01 48
[   32.344200] RSP: 002b:00007fffff1115e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[   32.344202] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f60d0079f6b
[   32.344202] RDX: 00007fffff111610 RSI: 0000000000005a47 RDI: 0000000000000004
[   32.344203] RBP: 00007fffff114c00 R08: 0000000000000015 R09: 000055d7f0c713d0
[   32.344203] R10: 000055d7efd034eb R11: 0000000000000246 R12: 0000000000005a47
[   32.344204] R13: 00007fffff111610 R14: 0000000000005a47 R15: 000055d7f0c713d0
ongoing hang detection prints in dmesg
[  245.591859] INFO: task sync-hang:3936 blocked for more than 122 seconds.
[  245.591862]       Tainted: P           OE     5.9.11-arch2-1 #1
[  245.591862] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  245.591864] task:sync-hang       state:D stack:    0 pid: 3936 ppid:  3050 flags:0x00000080
[  245.591867] Call Trace:
[  245.591876]  __schedule+0x292/0x830
[  245.591882]  schedule+0x46/0xf0
[  245.591891]  spl_panic+0x115/0x117 [spl]
[  245.591896]  ? _raw_spin_unlock+0x16/0x30
[  245.591956]  ? spa_open_common+0x2c7/0x4f0 [zfs]
[  245.591965]  fnvlist_lookup_boolean_value+0x6b/0x80 [znvpair]
[  245.592013]  zfs_ioc_pool_sync+0x51/0xe0 [zfs]
[  245.592063]  zfsdev_ioctl_common+0x3a0/0x880 [zfs]
[  245.592112]  zfsdev_ioctl+0x53/0xe0 [zfs]
[  245.592117]  __x64_sys_ioctl+0x83/0xb0
[  245.592122]  do_syscall_64+0x33/0x40
[  245.592124]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  245.592126] RIP: 0033:0x7f60d0079f6b
[  245.592128] Code: Unable to access opcode bytes at RIP 0x7f60d0079f41.
[  245.592129] RSP: 002b:00007fffff1115e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  245.592130] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f60d0079f6b
[  245.592131] RDX: 00007fffff111610 RSI: 0000000000005a47 RDI: 0000000000000004
[  245.592132] RBP: 00007fffff114c00 R08: 0000000000000015 R09: 000055d7f0c713d0
[  245.592133] R10: 000055d7efd034eb R11: 0000000000000246 R12: 0000000000005a47
[  245.592134] R13: 00007fffff111610 R14: 0000000000005a47 R15: 000055d7f0c713d0
@codyps codyps added Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang) labels Dec 4, 2020
codyps added a commit to codyps/rust-libzfs that referenced this issue Dec 4, 2020
behlendorf added a commit to behlendorf/zfs that referenced this issue Dec 4, 2020
When checking if the "force" argument is set it is sufficient
to use nvlist_exists().  fnvlist_lookup_boolean_value() should
not be used in this case since it's never allowed to fail which
can happen when the provided nvlist does not set NV_UNIQUE_NAME.
This is consistent with the boolean option handling for the
other ioctls.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#11281
@behlendorf behlendorf removed the Status: Triage Needed New issue which needs to be triaged label Dec 4, 2020
@behlendorf
Copy link
Contributor

Thanks for reporting this. I've opened PR #11284 with a fix.

behlendorf added a commit to behlendorf/zfs that referenced this issue Dec 8, 2020
The fnvlist_lookup_boolean_value() function should not be used
to check the force argument since it's optional.  It may not be
provided or may have been created with the wrong flags.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#11281
ghost pushed a commit to zfsonfreebsd/ZoF that referenced this issue Dec 23, 2020
The fnvlist_lookup_boolean_value() function should not be used
to check the force argument since it's optional.  It may not be
provided or may have been created with the wrong flags.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#11281
Closes openzfs#11284
behlendorf added a commit that referenced this issue Dec 23, 2020
The fnvlist_lookup_boolean_value() function should not be used
to check the force argument since it's optional.  It may not be
provided or may have been created with the wrong flags.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #11281
Closes #11284
jsai20 pushed a commit to jsai20/zfs that referenced this issue Mar 30, 2021
The fnvlist_lookup_boolean_value() function should not be used
to check the force argument since it's optional.  It may not be
provided or may have been created with the wrong flags.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#11281
Closes openzfs#11284
sempervictus pushed a commit to sempervictus/zfs that referenced this issue May 31, 2021
The fnvlist_lookup_boolean_value() function should not be used
to check the force argument since it's optional.  It may not be
provided or may have been created with the wrong flags.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#11281
Closes openzfs#11284
amybones pushed a commit to amybones/zfs that referenced this issue Oct 24, 2023
Many of ioctls used in ZFS make use of the fnvlist_lookup_* family of
functions, which assert when the associated nvlist_lookup_* function
returns non-zero. On Linux, one consequence is that any program that
issues an invalid ioctl will hang until manually killed since the
kernel thread responsible has crashed. If such a program is run
repeatedly, system resources could eventually be exhausted, making this
a vector for a denial of service attack by unprivileged code.

This issue was previously raised in openzfs#11281, and a fix applied was to the
single ioctl that was reported as being problematic in that issue. To
further improve the situation, this change introduces a simple check in
zfs_check_input_nvpairs for the presence of either NV_UNIQUE_NAME or
NV_UNIQUE_NAME_TYPE flags and returns EINVAL if not present, which
prevents this issue from occuring for any of the non-legacy ioctls.
amybones added a commit to amybones/zfs that referenced this issue Oct 24, 2023
Many of the ioctls in ZFS make use of the fnvlist_lookup_* family of functions,
which assert when the associated nvlist_lookup_* function returns non-zero. On
Linux, one consequence of this is that any program that issues an ioctl with a
misconfigured nvlist will hang until manually killed since the kernel thread
responsible has crashed. If such a program is run repeatedly, system resources
could eventually be exhausted, making this a vector for a denial of service
attack by unprivileged code.

This issue was previously raised in openzfs#11281, and a fix applied was to the
single ioctl that was reported as being problematic in that issue. To
further improve the situation, this change introduces a simple check in
zfs_check_input_nvpairs for the presence of either NV_UNIQUE_NAME or
NV_UNIQUE_NAME_TYPE flags and returns EINVAL if not present, which
prevents this issue from occuring for any of the non-legacy ioctls.

Signed-off-by: amy bones <amy@spookygirl.boo>
amybones added a commit to amybones/zfs that referenced this issue Oct 24, 2023
Many of the ioctls in ZFS make use of the fnvlist_lookup_* family of functions,
which assert when the associated nvlist_lookup_* function returns non-zero. On
Linux, one consequence of this is that any program that issues an ioctl with a
misconfigured nvlist will hang until manually killed since the kernel thread
responsible has crashed. If such a program is run repeatedly, system resources
could eventually be exhausted, making this a vector for a denial of service
attack by unprivileged code.

This issue was previously raised in openzfs#11281, and a fix applied was to the
single ioctl that was reported as being problematic in that issue. To
further improve the situation, this change introduces a simple check in
zfs_check_input_nvpairs for the presence of either NV_UNIQUE_NAME or
NV_UNIQUE_NAME_TYPE flags and returns EINVAL if not present, which
prevents this issue from occuring for any of the non-legacy ioctls.

Signed-off-by: amy bones <amy@spookygirl.boo>
amybones added a commit to amybones/zfs that referenced this issue Oct 26, 2023
Many of the ioctls in ZFS make use of the fnvlist_lookup_* family of functions,
which assert when the associated nvlist_lookup_* function returns non-zero. On
Linux, one consequence of this is that any program that issues an ioctl with a
misconfigured nvlist will hang until manually killed since the kernel thread
responsible has crashed. If such a program is run repeatedly, system resources
could eventually be exhausted, making this a vector for a denial of service
attack by unprivileged code.

This issue was previously raised in openzfs#11281, and a fix applied was to the
single ioctl that was reported as being problematic in that issue. To
further improve the situation, this change introduces a simple check in
zfs_check_input_nvpairs for the presence of either NV_UNIQUE_NAME or
NV_UNIQUE_NAME_TYPE flags and returns EINVAL if not present, which
prevents this issue from occuring for any of the non-legacy ioctls.

Signed-off-by: amy bones <amy@spookygirl.boo>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Development

No branches or pull requests

2 participants