9645 scf_read_propvec segfaults on error

Review Request #1128 — Created July 10, 2018 and submitted

andy_js
illumos-gate
9645
general

Enabling the auditset service on a machine that doesn't have auditd installed causes it to core dump and enter maintenance mode. From looking at the stack trace we see it dumping in free, which is being called by scf_clean_propvec.

pstack /var/cores/core.svc-auditset.6995

core '/var/cores/core.svc-auditset.6995' of 6995: /lib/svc/method/svc-auditset
fee59f9c _free_unlocked (8062000, 8047c4c, 73, 8062000, fe9fd000, fe93d900) + 24
fee5a3df free (8062000, fe9fd000, 3eb, 80632a0, 8047ca0, fefbe010) + 45
fe9da6a7 scf_clean_propvec (fe93d900, fe9247f2, 0, 80623a8, 80623c8, 8062368) + 8c
fe9daada scf_read_propvec (fe9247f2, fe9248b0, 0, fe93d900, 8047d5c, 0) + 418
fe9199a0 get_val_scf (fe93d900, fe9248b0, 5, 8047dbc, 0, 8047e20) + 47
fe91a97c do_getflags_scf (8047dbc, 8047db8, 0, 8051820, 8062000, 805126e) + 5d
080516f9 main (1, 8047e20, 8047e28, 8051363, 0, 0) + 128
08051388 _start_crt (1, 8047e20, fefcf363, 0, 0, 0) + 97
0805125a _start (1, 8047ed0, 0, 8047eed, 8047f0f, 8047f20) + 1a
Looking at the source we can see that scf_clean_propvec is called unconditionally on error, and in some cases it is called before the property vector has been initialised. When this happens free is called on bogus values.

Moving the block of code that initialised the property vector further up causes the problem to go away and exposes the real problem. Now the service fails with "entity not found" while looking up auditd.

Prior to the patch auditset core dumps when auditd isn't installed.
After the patch auditset fails with "entity not found".

andy_js
vgusev
  1. 
      
  2. usr/src/lib/libscf/common/midlevel.c (Diff revision 2)
     
     
    Could you add extra comment like "do initialization"? ?
    1. See the comment before the function.

    2. Indeed. Thanks)

  3. 
      
vgusev
  1. Ship It!
  2. 
      
tsoome
  1. Ship It!
  2. 
      
yuripv
  1. Ship It!
  2. 
      
andy_js
andy_js
andy_js
andy_js
Review request changed

Status: Closed (submitted)

Loading...