public inbox for linux-audit@redhat.com
 help / color / mirror / Atom feed
* BUG: possible memory leak in  userspace libauparse
@ 2019-03-26  2:38 zhangqi (DI)
  2019-03-26 21:31 ` Steve Grubb
  0 siblings, 1 reply; 2+ messages in thread
From: zhangqi (DI) @ 2019-03-26  2:38 UTC (permalink / raw)
  To: linux-audit@redhat.com; +Cc: Shanshishi


[-- Attachment #1.1: Type: text/plain, Size: 3229 bytes --]

Hi all
       I think there is a memory leak bug in userspace audit, correct me if I'm wrong.   Audit-2.8.5 has introduced a performance improvement for lol operations(see the following commits for details:3ecf7a212c53e439109163eef79e3bbe4c00dd99, 270c39f1f0dd783a32aa0f9a73214cf15e1c19b4).  The improvement code snippet is repeated here for your convenience:

auparse/auparse.c:

260         if (lowest && lowest->status == EBS_COMPLETE) {
261                 lowest->status = EBS_EMPTY;
262                 au->au_ready--;
263                 // Try to consolidate the array so that we iterate
264                 // over a smaller portion next time
265                 if (lowest == &lol->array[lol->maxi]) {
266                         au_lolnode *ptr = lowest;
267                         while (ptr->status == EBS_EMPTY && lol->maxi > 0) {
268                                 lol->maxi--;
269                                 ptr = &lol->array[lol->maxi];
270                         }
271                 }
272                 return lowest->l;
273         }

The problem is that after shrinking lol-maxi, the EBS_EMPTY lolnodes are effectively denied chances of being freed, as only entries below lol-maxi are freed:
1405         for (i = 0; i <= au->au_lo->maxi; i++) {
1406                 au_lolnode *cur = &au->au_lo->array[i];
1407                 if (cur->status == EBS_EMPTY && cur->l) {
1408 #ifdef  LOL_EVENTS_DEBUG01
1409                         if (debug) {printf("Freeing at start "); print_list_t(cur->l);}
1410 #endif  /* LOL_EVENTS_DEBUG01 */
1411                         aup_list_clear(cur->l);
1412                         free(cur->l);
1413                         au->le = NULL;  // this should crash any usage
1414                                         // of au->le until reset
1415                         cur->l = NULL;
1416                 }
1417         }


The problem is further confirmed when later insertions can make the cut out entries completely lost to the wild, since it doesn't check cur->l:

199         for (i = 0; i < lol->limit; i++) {
200                 au_lolnode *cur = &lol->array[i];
201                 if (cur->status == EBS_EMPTY) {
202                         cur->l = l;
203                         cur->status = EBS_BUILDING;
204                         if (i > lol->maxi)
205                                 lol->maxi = i;
206                         return cur;
207                 }
208         }

---------------------------------------------Some blackbox tests on sedispatch:-------------------------------------------------------

Valgrind check reports memory leak problem:
==30536== LEAK SUMMARY:
==30536==    definitely lost: 14,848 bytes in 232 blocks
==30536==    indirectly lost: 781,160 bytes in 29,837 blocks
==30536==      possibly lost: 0 bytes in 0 blocks
==30536==    still reachable: 11,851 bytes in 81 blocks
==30536==         suppressed: 0 bytes in 0 blocks
==30536== Reachable blocks (those to which a pointer was found) are not shown

And a dummy test program generating  floods of AVC events can blow the sedispatch daemon to some hundreds of megabytes after running for several days.

[-- Attachment #1.2: Type: text/html, Size: 12973 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: BUG: possible memory leak in  userspace libauparse
  2019-03-26  2:38 BUG: possible memory leak in userspace libauparse zhangqi (DI)
@ 2019-03-26 21:31 ` Steve Grubb
  0 siblings, 0 replies; 2+ messages in thread
From: Steve Grubb @ 2019-03-26 21:31 UTC (permalink / raw)
  To: linux-audit; +Cc: Shanshishi

On Monday, March 25, 2019 10:38:02 PM EDT zhangqi (DI) wrote:
>        I think there is a memory leak bug in userspace audit, correct me if
> I'm wrong.  

Thanks for reporting this. Upstream commits 1af601f and a4ed200 fix this.

-Steve

> Audit-2.8.5 has introduced a performance improvement for lol
> operations(see the following commits for
> details:3ecf7a212c53e439109163eef79e3bbe4c00dd99,
> 270c39f1f0dd783a32aa0f9a73214cf15e1c19b4).  The improvement code snippet
> is repeated here for your convenience:
> 
> auparse/auparse.c:
> 
> 260         if (lowest && lowest->status == EBS_COMPLETE) {
> 261                 lowest->status = EBS_EMPTY;
> 262                 au->au_ready--;
> 263                 // Try to consolidate the array so that we iterate
> 264                 // over a smaller portion next time
> 265                 if (lowest == &lol->array[lol->maxi]) {
> 266                         au_lolnode *ptr = lowest;
> 267                         while (ptr->status == EBS_EMPTY && lol->maxi >
> 0) { 268                                 lol->maxi--;
> 269                                 ptr = &lol->array[lol->maxi];
> 270                         }
> 271                 }
> 272                 return lowest->l;
> 273         }
> 
> The problem is that after shrinking lol-maxi, the EBS_EMPTY lolnodes are
> effectively denied chances of being freed, as only entries below lol-maxi
> are freed: 1405         for (i = 0; i <= au->au_lo->maxi; i++) {
> 1406                 au_lolnode *cur = &au->au_lo->array[i];
> 1407                 if (cur->status == EBS_EMPTY && cur->l) {
> 1408 #ifdef  LOL_EVENTS_DEBUG01
> 1409                         if (debug) {printf("Freeing at start ");
> print_list_t(cur->l);} 1410 #endif  /* LOL_EVENTS_DEBUG01 */
> 1411                         aup_list_clear(cur->l);
> 1412                         free(cur->l);
> 1413                         au->le = NULL;  // this should crash any usage
> 1414                                         // of au->le until reset 1415
>                         cur->l = NULL;
> 1416                 }
> 1417         }
> 
> 
> The problem is further confirmed when later insertions can make the cut out
> entries completely lost to the wild, since it doesn't check cur->l:
> 
> 199         for (i = 0; i < lol->limit; i++) {
> 200                 au_lolnode *cur = &lol->array[i];
> 201                 if (cur->status == EBS_EMPTY) {
> 202                         cur->l = l;
> 203                         cur->status = EBS_BUILDING;
> 204                         if (i > lol->maxi)
> 205                                 lol->maxi = i;
> 206                         return cur;
> 207                 }
> 208         }
> 
> ---------------------------------------------Some blackbox tests on
> sedispatch:-------------------------------------------------------
> 
> Valgrind check reports memory leak problem:
> ==30536== LEAK SUMMARY:
> ==30536==    definitely lost: 14,848 bytes in 232 blocks
> ==30536==    indirectly lost: 781,160 bytes in 29,837 blocks
> ==30536==      possibly lost: 0 bytes in 0 blocks
> ==30536==    still reachable: 11,851 bytes in 81 blocks
> ==30536==         suppressed: 0 bytes in 0 blocks
> ==30536== Reachable blocks (those to which a pointer was found) are not
> shown
> 
> And a dummy test program generating  floods of AVC events can blow the
> sedispatch daemon to some hundreds of megabytes after running for several
> days.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-03-26 21:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-26  2:38 BUG: possible memory leak in userspace libauparse zhangqi (DI)
2019-03-26 21:31 ` Steve Grubb

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox