11826 Fix resizing of audit event map

Review Request #2400 — Created Oct. 17, 2019 and submitted Oct. 25, 2019, 8:23 a.m.

Information
Owner:	mscheler
Repository:	illumos-gate
Branch:
Bugs:	11826
Depends On:
Reviewers
Groups:	general
People:

Description

Fix obvious memory allocation errors (famous last words)

Testing Done

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.

Issues

0
0
4
1
All Issues: 5

Description	From	Last Updated
(void *) should not need type cast, or if you feel like to keep those, you can move assignment out ...	tsoome	Oct. 17, 2019, 1:05 p.m.
Have you considered switching to reallocarray() here too to protect against overflows?	citrus	Oct. 17, 2019, 2:53 p.m.
Since you're modifying this, would you mind putting braces around this if statement since it's body is a multi-line continuation.	rm	Oct. 18, 2019, 8:50 a.m.
Shouldn't this be '<=' rather than just '<'?	rm	Oct. 18, 2019, 8:52 a.m.
Should we consider using recallocarray as opposed to reallocarray so that way if other software ends up making the mistake ...	rm	Oct. 18, 2019, 8:53 a.m.

usr/src/lib/libbsm/common/au_preselect.c (Diff revision 1)

Show all issues

(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:)

mscheler Oct. 17, 2019, 1:05 p.m.

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.

Ship it!

```
Ship It!
```

Ship it!

usr/src/lib/libbsm/common/au_preselect.c (Diff revision 1)

Show all issues

Have you considered switching to reallocarray() here too to protect against overflows?

mscheler Oct. 17, 2019, 2:42 p.m.

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.

Change Summary:

Addressed review feedback

Diff:

Revision 2 (+15 -6)

Show changes

usr/src/lib/libbsm/common/au_preselect.c

Ship it!

```
Ship It!
```

usr/src/lib/libbsm/common/au_preselect.c (Diff revision 2)

Show all issues

Since you're modifying this, would you mind putting braces around this if statement since it's body is a multi-line continuation.

mscheler Oct. 18, 2019, 8:56 a.m.
```
I've combined the two if blocks.
```

usr/src/lib/libbsm/common/au_preselect.c (Diff revision 2)

Show all issues

Shouldn't this be '<=' rather than just '<'?

mscheler Oct. 18, 2019, 8:56 a.m.

This can only happen with a very very odd value for ALLOC_INC. But I'll change the check.

usr/src/lib/libbsm/common/au_preselect.c (Diff revision 2)

Show all issues

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?

mscheler Oct. 18, 2019, 8:56 a.m.

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.