Opened 5 months ago
Closed 5 months ago
#1830 closed defect (fixed)
error message for missing argument segfaults (musl libc)
Reported by: | ncopa | Owned by: | Christian Franke |
---|---|---|---|
Priority: | major | Milestone: | Release 7.5 |
Component: | all | Version: | |
Keywords: | Cc: |
Description
running any of: smartctl -l
, smartctl -t
, smartctl -P
, smartctl -v
, smartctl -b
and smartctl -f
results in segfault on Alpine Linux (using musl libc).
Backtrace:
Program received signal SIGSEGV, Segmentation fault. parse_options (print_type_only=@0x7fffffffa556: false, nvmeopts=..., scsiopts=..., ataopts=..., type=@0x7fffffffa538: 0x0, argv=0x7fffffffe668, argc=2) at smartctl.cpp:1173 1173 if (arg[1] == '-' && optchar != 'h') {
Downstream report: https://gitlab.alpinelinux.org/alpine/aports/-/issues/15948
Change History (16)
comment:1 by , 5 months ago
comment:2 by , 5 months ago
Milestone: | → undecided |
---|---|
Priority: | minor → major |
Thanks for the report. Please check whether smartd
is also affected, for example with smartd -l
.
The related code is 21+ years old, see r369. I don't remember any similar problem report.
The fix does not work with getopt_long()
variants from glibc and Open/NetBSD. The latter is also used by Cygwin and Mingw-w64 CRT. All of these set optind = 2
on smartctl -l
such that arg
correctly points to -l
.
The fix suggests that musl libc sets optind = 3
. This either means that smartctl relies on an undocumented corner case or that there is a bug in musl libc itself. I'm not sure.
Perhaps we could get rid of this legacy arg
check. Leaving ticket open as undecided for now.
comment:3 by , 5 months ago
Summary: | error message for missing argument segfaults → error message for missing argument segfaults (musl libc) |
---|
comment:4 by , 5 months ago
I can confirm that:
/ # apk add smartmontools fetch http://dl-cdn.alpinelinux.org/alpine/v3.12/main/x86_64/APKINDEX.tar.gz fetch http://dl-cdn.alpinelinux.org/alpine/v3.12/community/x86_64/APKINDEX.tar.gz (1/3) Installing libgcc (9.3.0-r2) (2/3) Installing libstdc++ (9.3.0-r2) (3/3) Installing smartmontools (7.1-r2) Executing busybox-1.31.1-r19.trigger OK: 9 MiB in 17 packages / # smartctl -l smartctl 7.1 2019-12-30 r5022 [x86_64-linux-5.10.47-linuxkit] (local build) Copyright (C) 2002-19, Bruce Allen, Christian Franke, www.smartmontools.org Segmentation fault / # cat /etc/os-release NAME="Alpine Linux" ID=alpine VERSION_ID=3.12.3 PRETTY_NAME="Alpine Linux v3.12" HOME_URL="https://alpinelinux.org/" BUG_REPORT_URL="https://bugs.alpinelinux.org/"
Probably we do not have a lot of such reports because alpine is mostly popular in containers and not bone servers.
comment:5 by , 5 months ago
I could not reproduce the problem with a quick stand-alone test of musl-1.2.5/src/misc/getopt_long.c
. It sets optind = 2
on missing arguments as expected.
Which musl version is used on the affected systems?
comment:6 by , 5 months ago
Some data from gdb with smartctl -l
(gdb) print optind $37 = 3 (gdb) print argc $38 = 2 (gdb) print argv[0] $39 = 0x7fff95e8cf12 "/smartmontools-7.5/smartctl" (gdb) print argv[1] $40 = 0x7fff95e8cf2e "-l" (gdb) print argv[2] $41 = 0x0
I think we can change the logic to check if optind > argc and use argc instead, but we need to check if it works correctly.
comment:7 by , 5 months ago
I was trying different versions (using docker and run alpine
), now it is:
/smartmontools-7.5 # apk list --installed|grep -i musl musl-dbg-1.2.2-r9 x86_64 {musl} (MIT) [installed] musl-1.2.2-r9 x86_64 {musl} (MIT) [installed] musl-utils-1.2.2-r9 x86_64 {musl} (MIT BSD GPL2+) [installed] musl-dev-1.2.2-r9 x86_64 {musl} (MIT) [installed]
comment:8 by , 5 months ago
Also issue confirmed with alpine:edge which uses musl-1.2.5
/ # smartctl -l smartctl 7.4 2023-08-01 r5530 [x86_64-linux-5.10.47-linuxkit] (local build) Copyright (C) 2002-23, Bruce Allen, Christian Franke, www.smartmontools.org Segmentation fault / # apk list --installed|grep musl musl-1.2.5-r0 x86_64 {musl} (MIT) [installed] musl-utils-1.2.5-r0 x86_64 {musl} (MIT AND BSD-2-Clause AND GPL-2.0-or-later) [installed]
comment:9 by , 5 months ago
I could not reproduce the problem ...
I take that back as I missed that getopt_long()
calls getopt()
for short options. This actually returns optind = argc + 1
if the argument of the last option is missing.
This is IMO a bug as optind
should never point behind the argv[argc+1]
array.
comment:10 by , 5 months ago
Posix spec is not very strict about it, it tells that getopt should update optind after option is processed, however, there is no promise that it should be pointed to valid arg. So i think if we will add logic min(optind, argc) it will not make any issues.
comment:11 by , 5 months ago
https://github.com/JuliaLang/julia/pull/31946 looks like a very similar issue from another project
comment:12 by , 5 months ago
ah, yes
If the option was the last character in the string pointed to by an element of argv, then optarg shall contain the next element of argv, and optind shall be incremented by 2. If the resulting value of optind is greater than argc, this indicates a missing option-argument, and getopt() shall return an error indication.
comment:13 by , 5 months ago
probably something like that should always work:
arg = optind > argc ? argv[argc-1] : argv[optind-1];
P.S. same apply for smartd
, with a fix above working the same.
comment:14 by , 5 months ago
Milestone: | undecided → Release 7.5 |
---|---|
Owner: | set to |
Status: | new → accepted |
Proposed fix for smartctl.cpp and smartd.cpp:
- arg = argv[optind-1]; + arg = argv[optind <= argc ? optind - 1 : argc - 1];
This fixes it: