Skip to content

'zpool get' segfaults with two vdevs #15972

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

Closed
tonyhutter opened this issue Mar 6, 2024 · 4 comments
Closed

'zpool get' segfaults with two vdevs #15972

tonyhutter opened this issue Mar 6, 2024 · 4 comments
Labels
good first issue Indicates a good issue for first-time contributors Type: Defect Incorrect behavior (e.g. crash, hang)

Comments

@tonyhutter
Copy link
Contributor

tonyhutter commented Mar 6, 2024

System information

Type Version/Name
Distribution Name Fedora
Distribution Version 39
Kernel Version 6.6.13
Architecture x86-64
OpenZFS Version 8f2f6cd (master)

Describe the problem you're observing

If you pass two vdevs to zpool get, it will segfault. Both vdevs need to be part of the pool.

Describe how to reproduce the problem

zpool get state <pool> <vdev1> <vdev2>

Include any warning/errors/backtraces from the system logs

#0  0x00007ffff76de7ff in ____strtoull_l_internal () from /lib64/libc.so.6
#1  0x00007ffff7f70e17 in zpool_find_vdev (zhp=zhp@entry=0x4471f0, path=path@entry=0x0, avail_spare=avail_spare@entry=0x7fffffff81c8, l2cache=l2cache@entry=0x7fffffff81cc, log=log@entry=0x0)
    at lib/libzfs/libzfs_pool.c:3053
#2  0x00007ffff7f70f4b in zpool_vdev_guid (zhp=zhp@entry=0x4471f0, vdevname=vdevname@entry=0x0, vdev_guid=vdev_guid@entry=0x7fffffff8628) at lib/libzfs/libzfs_pool.c:5147
#3  0x00007ffff7f777ac in zpool_get_vdev_prop (zhp=zhp@entry=0x4471f0, vdevname=vdevname@entry=0x0, prop=VDEV_PROP_NAME, prop_name=0x0, buf=buf@entry=0x7fffffff8aa0 "/file1", len=len@entry=4096, 
    srctype=0x0, literal=B_FALSE) at lib/libzfs/libzfs_pool.c:5340
#4  0x00007ffff7f77b45 in vdev_expand_proplist (zhp=zhp@entry=0x4471f0, vdevname=vdevname@entry=0x0, plp=plp@entry=0x7fffffffac70) at lib/libzfs/libzfs_pool.c:1044
#5  0x000000000040a89b in get_callback (zhp=0x4471f0, data=0x7fffffffac30) at cmd/zpool/zpool_main.c:10550
#6  0x00000000004090ef in pool_list_iter (zlp=0x4496b0, unavail=<optimized out>, func=0x40a720 <get_callback>, data=0x7fffffffac30) at cmd/zpool/zpool_iter.c:186
#7  0x000000000040929a in for_each_pool (argc=argc@entry=1, argv=argv@entry=0x4498e8, unavail=unavail@entry=B_TRUE, proplist=proplist@entry=0x7fffffffac70, type=<optimized out>, literal=<optimized out>, 
    func=0x40a720 <get_callback>, data=0x7fffffffac30) at cmd/zpool/zpool_iter.c:262
#8  0x000000000041d796 in zpool_do_get (argc=1, argv=0x4498e8) at cmd/zpool/zpool_main.c:10777
#9  0x000000000040895e in main (argc=6, argv=0x7fffffffe408) at cmd/zpool/zpool_main.c:11545

@tonyhutter tonyhutter added good first issue Indicates a good issue for first-time contributors Type: Defect Incorrect behavior (e.g. crash, hang) labels Mar 6, 2024
@madwizard
Copy link
Contributor

I did a test on FreeBSD:
FreeBSD version: FreeBSD 14.0-RELEASE FreeBSD 14.0-RELEASE #0 releng/14.0-n265380-f9716eee8ab4: Fri Nov 10 05:57:23 UTC 2023
Zpool version: zpool version zfs-2.2.0-FreeBSD_g95785196f zfs-kmod-2.2.0-FreeBSD_g95785196f
running this command gives me the same result: segfault.

I've run this through gdb, and se segfault happens within zpool_find_vdev in this line:

guid = strtoull(path, &end, 0);

`__GI_____strtoul_l_internal (nptr=0x0, endptr=0x7fffffff8250, base=0, group=0, bin_cst=true, loc=0x7ffff7da33c0 <_nl_global_locale>) at ../stdlib/strtol_l.c:238
238 {
(gdb) s
252 struct __locale_data *current = loc->__locales[LC_NUMERIC];
(gdb) s
264 if (__glibc_unlikely (group))
(gdb) s
295 if (base < 0 || base == 1 || base > 36)
(gdb) s
304 while (ISSPACE (*s))
(gdb) print s
$4 = 0x0
(gdb) s

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7c197ff in __GI_____strtoul_l_internal (nptr=0x0, endptr=0x7fffffff8250, base=0, group=, bin_cst=true, loc=0x7ffff7da33c0 <_nl_global_locale>) at ../stdlib/strtol_l.c:304
304 while (ISSPACE (*s))`

The ISSPACE receives 0x0.

@justinpryzby
Copy link

commit 2a673e7 (@allanjude @mmaybee) added vdev properties, and zpool_do_get() was changed to say:

+                       cb.cb_vdevs.cb_names = argv + 1;
+                       cb.cb_vdevs.cb_names_count = argc - 1;

commit 3e4ed42 (@rob-wing) was about "Create zap for root vdev", and changed it to say:

+                       cb.cb_vdevs.cb_names = &vdev;

@Egor-OSSRevival
Copy link

Hi, I’m Egor from OSS Revival. We specialize in revitalizing and maintaining open-source projects for long-term stability. I’ll work on this issue, ensuring it stays aligned with the project’s original vision.

Syed-Shahrukh-OSSRevival added a commit to Project-OSS-Revival/zfs that referenced this issue Apr 3, 2025
Syed-Shahrukh-OSSRevival added a commit to Project-OSS-Revival/zfs that referenced this issue Apr 3, 2025
…).

This change fixes the zpool get command segfault if you pass
two or vdevs to zpool get. The problem was identified in handling
of the zpool get command line arguments. A pointer vdev was used
to point to the argv[1], and its address set to cb.cb_vdevs.cb_names
(pointer to array of strings) so any increment to cb_names resulted
in a segfault. Fix covers a special case of root parameter at argv[1]
and remaining cases are handled by passing in the argv + 1, which
allows cb_names iteration of next command line arguments (vdevs).

Signed-off-by: Syed Shahrukh Hussain <[email protected]>
Syed-Shahrukh-OSSRevival added a commit to Project-OSS-Revival/zfs that referenced this issue Apr 3, 2025
…fs#15972).

The problem was identified in handling of the zpool get state command
line arguments. A pointer vdev was used to point to the argv[1], and
its address set to cb.cb_vdevs.cb_names(pointer to array of strings)
so any increment to cb_names resulted in a segfault. Fix covers a
special case of root parameter at argv[1] and remaining cases are
handled by passing in the argv + 1, which allows cb_names iteration
of next command line arguments (vdevs).

Signed-off-by: Syed Shahrukh Hussain <[email protected]>
tonyhutter pushed a commit that referenced this issue Apr 4, 2025
…. (#17213)

The problem was identified in handling of the zpool get state command
line arguments. A pointer vdev was used to point to the argv[1], and
its address set to cb.cb_vdevs.cb_names(pointer to array of strings)
so any increment to cb_names resulted in a segfault. Fix covers a
special case of root parameter at argv[1] and remaining cases are
handled by passing in the argv + 1, which allows cb_names iteration
of next command line arguments (vdevs).

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Attila Fülöp <[email protected]>

Signed-off-by: Syed Shahrukh Hussain <[email protected]>
@tonyhutter
Copy link
Contributor Author

This has been fixed

fuporovvStack pushed a commit to fuporovvStack/zfs that referenced this issue Apr 11, 2025
…fs#15972). (openzfs#17213)

The problem was identified in handling of the zpool get state command
line arguments. A pointer vdev was used to point to the argv[1], and
its address set to cb.cb_vdevs.cb_names(pointer to array of strings)
so any increment to cb_names resulted in a segfault. Fix covers a
special case of root parameter at argv[1] and remaining cases are
handled by passing in the argv + 1, which allows cb_names iteration
of next command line arguments (vdevs).

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Attila Fülöp <[email protected]>

Signed-off-by: Syed Shahrukh Hussain <[email protected]>
fuporovvStack pushed a commit to fuporovvStack/zfs that referenced this issue Apr 11, 2025
…fs#15972). (openzfs#17213)

The problem was identified in handling of the zpool get state command
line arguments. A pointer vdev was used to point to the argv[1], and
its address set to cb.cb_vdevs.cb_names(pointer to array of strings)
so any increment to cb_names resulted in a segfault. Fix covers a
special case of root parameter at argv[1] and remaining cases are
handled by passing in the argv + 1, which allows cb_names iteration
of next command line arguments (vdevs).

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Attila Fülöp <[email protected]>

Signed-off-by: Syed Shahrukh Hussain <[email protected]>
tonyhutter pushed a commit to tonyhutter/zfs that referenced this issue Apr 15, 2025
…fs#15972). (openzfs#17213)

The problem was identified in handling of the zpool get state command
line arguments. A pointer vdev was used to point to the argv[1], and
its address set to cb.cb_vdevs.cb_names(pointer to array of strings)
so any increment to cb_names resulted in a segfault. Fix covers a
special case of root parameter at argv[1] and remaining cases are
handled by passing in the argv + 1, which allows cb_names iteration
of next command line arguments (vdevs).

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Attila Fülöp <[email protected]>

Signed-off-by: Syed Shahrukh Hussain <[email protected]>
tonyhutter pushed a commit to tonyhutter/zfs that referenced this issue Apr 16, 2025
…fs#15972). (openzfs#17213)

The problem was identified in handling of the zpool get state command
line arguments. A pointer vdev was used to point to the argv[1], and
its address set to cb.cb_vdevs.cb_names(pointer to array of strings)
so any increment to cb_names resulted in a segfault. Fix covers a
special case of root parameter at argv[1] and remaining cases are
handled by passing in the argv + 1, which allows cb_names iteration
of next command line arguments (vdevs).

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Attila Fülöp <[email protected]>

Signed-off-by: Syed Shahrukh Hussain <[email protected]>
snajpa pushed a commit to vpsfreecz/zfs that referenced this issue May 2, 2025
…fs#15972). (openzfs#17213)

The problem was identified in handling of the zpool get state command
line arguments. A pointer vdev was used to point to the argv[1], and
its address set to cb.cb_vdevs.cb_names(pointer to array of strings)
so any increment to cb_names resulted in a segfault. Fix covers a
special case of root parameter at argv[1] and remaining cases are
handled by passing in the argv + 1, which allows cb_names iteration
of next command line arguments (vdevs).

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Attila Fülöp <[email protected]>

Signed-off-by: Syed Shahrukh Hussain <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Indicates a good issue for first-time contributors Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Development

No branches or pull requests

4 participants