11826 Fix resizing of audit event map

Review Request #2400 — Created Oct. 17, 2019 and submitted

mscheler
illumos-gate
11826
general

Fix obvious memory allocation errors (famous last words)

I've run a nightly and booted a system into the a BE with the new packages. I've made sure that the "newgrp" command (which is linked against "libbsm") still works.

I'm open to suggestions for better testing.

  • 0
  • 0
  • 4
  • 1
  • 5
Description From Last Updated
tsoome
  1. 
      
  2. (void *) should not need type cast, or if you feel like to keep those, you can move assignment out of if statement and win in readability:)

    1. This whole source file needs a cleanup:
      - Remove "register"
      - Fix double loading of the map when au_preselect() is called for the first time and AU_PRS_REREAD is used
      - Use "bool" (or "boolean_t") instead of integers
      etc.

      However I would prefer to limit the scope of this change to fixing the buffer overflow and memory leak.

  3. 
      
tsoome
  1. Ship It!
  2. 
      
citrus
  1. 
      
  2. Have you considered switching to reallocarray() here too to protect against overflows?

    1. I didn't even know that such a function exists. It doesn't exist in the old Illumos derivate that this fix is coming from.

  3. 
      
mscheler
citrus
  1. Ship It!
  2. 
      
rm
  1. 
      
  2. Since you're modifying this, would you mind putting braces around this if statement since it's body is a multi-line continuation.
    1. I've combined the two if blocks.

  3. Shouldn't this be '<=' rather than just '<'?
    1. This can only happen with a very very odd value for ALLOC_INC. But I'll change the check.

  4. usr/src/lib/libbsm/common/au_preselect.c (Diff revision 2)
     
     
     
     
    Should we consider using recallocarray as opposed to reallocarray so that way if other software ends up making the mistake of going beyond the valid counts, we end up with zeroed data rather than random heap garbage?
    1. I would prefer to put in some value which makes application crash in a predictable way (e.g. segmentation fault). But as both members are integers there isn't a good choice here.

      I'll change it to use recallocarray() for now.

  5. 
      
mscheler
rm
  1. Ship It!
  2. 
      
citrus
  1. Ship It!
  2. 
      
mscheler
mscheler
tsoome
  1. Ship It!
  2. 
      
citrus
  1. Ship It!
  2. 
      
rm
  1. Ship It!
  2. 
      
mscheler
Review request changed

Status: Closed (submitted)

Loading...