[PATCH] Fix audispd crash on ARM 32-Bits

[Tia, Javier]

Hi Steve,

Understood. Thank you for all your comments and suggestions.

-Javier

On 12/13/20 10:34 PM, Steve Grubb wrote:
> On Saturday, December 12, 2020 3:21:25 PM EST Tia, Javier wrote:
>> Thank you for your prompt response and for pointing to a solution.
>>
>> Yes, this patch it's applied to audit v2.4.3. It's an embedded device,
>> and at the moment, we're unable to upgrade the audit to a higher audit
>> version.
> 
> That's a shame. But if you have a reproducer, it might be worth seeing if its
> fixed in 2.8.5 and bisecting back to find the official patch if it were fixed.
>   
>> If audit v2.4.y were still maintainable,
> 
> It's not
> 
>> would you accept this patch for audit v2.4.y?
> 
> That depends. You are zeroing out the path and then setting it to NULL.
> Setting the pointer to NULL should be enough. If not, setting the first byte
> to 0 should wipe out the whole string for any string function. But usually
> this kind of fixup is because it gets used again somewhere by accident. That
> would be a plugin lifecycle issue and would be the root cause. The plugin
> lifecycle was reworked sometime after the release you have.
> 
> So, my guess (and it's pure speculation without a reproducer) is this covers
> up whatever problem you are seeing. But there may be a deeper issue about a
> plugin not being fully decommissioned. It's a long way to say, I'd look
> deeper as to how this goes wrong.
> 
> -Steve
> 
>>
>> -Javier
>>
>> On 12/12/20 1:45 PM, Steve Grubb wrote:
>>
>>> Hello,
>>>
>>> Thanks for the patch. But if its true that this is against audit-2.4.3,
>>> then
>   there is a good chance this is fixed by 2.8.5. There were a number
>>> of fixes in this area that fixed various issues with plugins.
>>>
>>> Best Regards,
>>> -Steve
>>>
>>> On Friday, December 11, 2020 9:10:50 PM EST Javier Tiá wrote:
>>>
>>>> On ARM 32-Bits, audispd is crashing. Backtrace:
>>>>
>>>>
>>>>
>>>> (gdb) bt
>>>> 0  0xb6e20958 in __GI_raise (sig=sig at entry=6)
>>>>
>>>>      at
>>>>      /usr/src/debug/glibc/2.23-r0/git/sysdeps/unix/sysv/linux/raise.c:54
>>>>      
>>>>
>>>> 1  0xb6e21e58 in __GI_abort ()
>>>>
>>>>      at /usr/src/debug/glibc/2.23-r0/git/stdlib/abort.c:118
>>>>
>>>> 2  0xb6e59d64 in __libc_message (do_abort=do_abort at entry=2,
>>>>
>>>>      fmt=0xb6f1119c "*** Error in `%s': %s: 0x%s ***\n")
>>>>      at /usr/src/debug/glibc/2.23-r0/git/sysdeps/posix/libc_fatal.c:175
>>>>
>>>> 3  0xb6e60108 in malloc_printerr (action=<optimized out>,
>>>>
>>>>      str=0xb6f11354 "double free or corruption (fasttop)",
>>>>      ptr=<optimized
>>>>
>>>> out>, ar_ptr=<optimized out>)
>>>>
>>>>      at /usr/src/debug/glibc/2.23-r0/git/malloc/malloc.c:5007
>>>>
>>>> 4  0xb6e60a98 in _int_free (av=0xb6f2d79c <main_arena>, p=<optimized
>>>> out>,
>>>
>>>>      have_lock=<optimized out>)
>>>>      at /usr/src/debug/glibc/2.23-r0/git/malloc/malloc.c:3868
>>>>
>>>> 5  0x004234b8 in free_pconfig (config=0x43b398)
>>>>
>>>>      at
>>>>
>>>> /usr/src/debug/audit/2.4.3-r8/audit-2.4.3/audisp/audispd-pconfig.c:513
>>>> 6
>>>> 0x00421244 in main (argc=<optimized out>, argv=<optimized out>) at
>>>> /usr/src/debug/audit/2.4.3-r8/audit-2.4.3/audisp/audispd.c:464
>>>>
>>>>
>>>>
>>>> (gdb) f 5
>>>> (gdb) p config->path
>>>> $2 = 0x43b5f0 ""
>>>> (gdb) p config->name
>>>> $3 = 0x43b370 "h\264C
>>>>
>>>>
>>>>
>>>> Be paranoid and overwrite config->path with zero bytes before doing the
>>>> free().
>>>> ---
>>>>
>>>>    audisp/audispd-pconfig.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>>
>>>>
>>>> diff --git a/audisp/audispd-pconfig.c b/audisp/audispd-pconfig.c
>>>> index a8b7878..a13f681 100644
>>>> --- a/audisp/audispd-pconfig.c
>>>> +++ b/audisp/audispd-pconfig.c
>>>> @@ -510,7 +510,11 @@ void free_pconfig(plugin_conf_t *config)
>>>>
>>>>    		close(config->plug_pipe[0]);
>>>>    	
>>>>    	if (config->plug_pipe[1] >= 0)
>>>>    	
>>>>    		close(config->plug_pipe[1]);
>>>>
>>>> +	/* Be paranoid and overwrite config->path with zero bytes before
>>>> doing
>>>> the +	 * free() */
>>>> +	memset(config->path, 0, strlen(config->path));
>>>>
>>>>    	free((void *)config->path);
>>>>
>>>> +	config->path = NULL;
>>>>
>>>>    	free((void *)config->name);
>>>>    
>>>>    }
>>>
>>>
>>>
>>>
>>>
> 
> 
> 
>