Project

General

Profile

Actions

Bug #16054

open

fmdump mishandles interleaved arguments with -A

Added by Thirteen Oxide 17 days ago. Updated 10 days ago.

Status:
New
Priority:
Normal
Category:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:
External Bug:

Description

We observe:

$ fmdump -A ./errlog -c ereport.io.pci.fabric
Segmentation Fault (core dumped)
$ fmdump -A -c ereport.io.pci.fabric ./errlog | wc -l
    2558

In the core, we see:
> ::walk thread | ::findstack -v
stack pointer for thread 1: 8037938
[ 08037938 libc.so.1`__lwp_park+0x19() ]
  08037978 libc.so.1`cond_wait_queue+0x5f(8cd7a98, 8cd7a78, 0)
  080379b8 libc.so.1`__cond_wait+0xaf(8cd7a98, 8cd7a78)
  080379d8 libc.so.1`cond_wait+0x2e(8cd7a98, 8cd7a78)
  080379f8 libc.so.1`pthread_cond_wait+0x26(8cd7a98, 8cd7a78)
  08037a78 aggregate+0x268(8cd7010, 3, 0, 8037ab0, 2, 0)
  08037c38 main+0x737(8037c3c, fe794484)
  08037c78 _start_crt+0x9a(5, 8037ca4, eff596e7, 0, 0, 0)
  08037c98 _start+0x1a(5, 8037da8, 8037daf, 8037db2, 8037dbb, 8037dbe)
stack pointer for thread 2: feb66ef8
[ feb66ef8 libc.so.1`__open64() ]
  feb66f38 libfmd_log.so.1`fmd_log_open+0x6f(2, 8cd7610, feb66f68)
  feb66fa8 pipeline_process+0x2b(8cd7a18, 8cd7610, 0)
  feb66fc8 pipeline_thr+0x5b(8cd7a18)
  feb66fe8 libc.so.1`_thrp_setup+0x7f(fec00240)
  feb66ff8 libc.so.1`_lwp_start(fec00240, 0, 0, 0, 0, 0)
stack pointer for thread 3: fed17ef8
[ fed17ef8 libc.so.1`strlen+0x30() ]
  fed17f38 libfmd_log.so.1`fmd_log_open+0x51(2, 0, fed17f68)
  fed17fa8 pipeline_process+0x2b(8cd7a78, 0, 0)
  fed17fc8 pipeline_thr+0x5b(8cd7a78)
  fed17fe8 libc.so.1`_thrp_setup+0x7f(fec00a40)
  fed17ff8 libc.so.1`_lwp_start(fec00a40, 0, 0, 0, 0, 0)
stack pointer for thread 4: fe91ef48
[ fe91ef48 libc.so.1`__lwp_park+0x19() ]
  fe91ef88 libc.so.1`mutex_lock_impl+0x13a(8cd7ad8, 0)
  fe91efa8 libc.so.1`mutex_lock+0x19(8cd7ad8)
  fe91efc8 pipeline_thr+0x11(8cd7ad8)
  fe91efe8 libc.so.1`_thrp_setup+0x7f(fec01240)
  fe91eff8 libc.so.1`_lwp_start(fec01240, 0, 0, 0, 0, 0)

We can see that we're starting 3 pipeline threads and that one of them has handed fmd_log_open a NULL filename. Why? All we do in aggregate is iterate over ifiles/n_ifiles. We can see that n_ifiles is 3, but what's in ifiles?
> 8cd7010/ppp
0x8cd7010:      0x8cd7610          0                  0
> 0x8cd7610/s
0x8cd7610:      ./errlog

Now that won't do. Why do we think we have 3 files when in fact only one additional argument was specified on the command line? The option-parsing logic here tries to accommodate arguments (i.e., filenames) interleaved with options. It mostly works, but the code that tries to do this looks like this:
»       »       if (optind < argc) {
»       »       »       char *dest;

»       »       »       if (ifiles == NULL) {
»       »       »       »       n_ifiles = argc - optind;
»       »       »       »       ifiles = calloc(n_ifiles, sizeof (char *));
»       »       »       »       if (ifiles == NULL) {
»       »       »       »       »       fmdump_fatal(
»       »       »       »       »       »   "failed to allocate memory for " 
»       »       »       »       »       »   "%d input file%s", n_ifiles,
»       »       »       »       »       »   n_ifiles > 1 ? "s" : "");
»       »       »       »       }
»       »       »       }

»       »       »       if (ifileidx > 0 && !opt_A)
»       »       »       »       fmdump_usage("illegal argument -- %s\n",
»       »       »       »       »   argv[optind]);

»       »       »       if ((dest = malloc(PATH_MAX)) == NULL)
»       »       »       »       fmdump_fatal("failed to allocate memory");

»       »       »       (void) strlcpy(dest, argv[optind++], PATH_MAX);
»       »       »       ifiles[ifileidx++] = dest;
»       »       }

In the case of interest, we're giving -A so multiple file arguments are allowed. The first time we encounter one, we set n_ifiles to the maximum number of such arguments we could have: the remaining number to be processed. But in this instance, all but one of those are actually options and/or option arguments that are going to be swallowed up. We end up with n_ifiles equal to 3 but ifileidx equal to 1 and only the single valid entry. We should be able to address this by setting n_ifiles to ifileidx when we're finished parsing all the arguments -- and while we're here, we probably ought to assert that ifileidx < n_ifiles before adding the entry to the array; if it isn't, this parsing code has to be broken and we're going to corrupt memory if we continue.

With this change:

$ ~/fmdump -A ./errlog -c ereport.io.pci.fabric | wc -l
    2558
$ fmdump -A -t 0 ./errlog ./fltlog | wc -l
    5738
$ fmdump -A ./errlog -t 0 ./fltlog | wc -l
       0
$ ~/fmdump -A ./errlog -t 0 ./fltlog | wc -l
    5738
$ ~/fmdump -A -t 0 ./errlog ./fltlog | wc -l
    5738


Related issues

Related to illumos gate - Bug #16051: fmdump -A, -a, -H, -j, and -p should be documentedNewThirteen Oxide

Actions
Actions #1

Updated by Thirteen Oxide 17 days ago

  • Assignee set to Thirteen Oxide
  • Difficulty changed from Medium to Bite-size
Actions #2

Updated by Thirteen Oxide 17 days ago

  • Related to Bug #16051: fmdump -A, -a, -H, -j, and -p should be documented added
Actions #3

Updated by Electric Monk 10 days ago

  • Gerrit CR set to 3150
Actions

Also available in: Atom PDF