[PATCH 1/1] Added exe field to audit core dump signal log

[William Roberts]

On Wed, Nov 20, 2013 at 2:03 PM, William Roberts
<bill.c.roberts at gmail.com> wrote:
> On Wed, Nov 20, 2013 at 1:47 PM, Richard Guy Briggs <rgb at redhat.com> wrote:
>> On Thu, Nov 14, 2013 at 08:56:57AM +0530, Paul Davies C wrote:
>>> Currently when the coredump signals are logged by the audit system , the
>>> actual path to the executable is not logged. Without details of exe , the
>>> system admin may not have an exact idea on what program failed.
>>>
>>> This patch changes the audit_log_task() so that the path to the exe is also
>>> logged.
>>
>> Out of curiosity, on which platform are you observing this?  This sounds
>> related to Bill Roberts' recent cmdline patches.
>
> I don't think this is related, looks to me like he want the exe file that
> started it. Where as I want some abstract field that was set by the VM
> at application invocation.
>
>>
>> I also wonder how reliable this is, or whether it could have been
>> changed from under us by deletion or rename after invocation.
>
> I don't think paths can ever be trusted 100%... I'm thinking chroots and
> namespaces might alter what the path is.
>
>>
>> This BZ sounds related:
>>         https://bugzilla.redhat.com/show_bug.cgi?id=837856
>>         https://bugzilla.redhat.com/show_bug.cgi?id=831684
>>
>>> Signed-off-by: Paul Davies C <pauldaviesc at gmail.com>
>>> ---
>>>  kernel/auditsc.c |    7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>>> index 9845cb3..988de72 100644
>>> --- a/kernel/auditsc.c
>>> +++ b/kernel/auditsc.c
>>> @@ -2353,6 +2353,7 @@ static void audit_log_task(struct audit_buffer *ab)
>>>       kuid_t auid, uid;
>>>       kgid_t gid;
>>>       unsigned int sessionid;
>>> +     struct mm_struct *mm = current->mm;
> Why wouldn't we use:
> get_task_mm(current)
>>>
>>>       auid = audit_get_loginuid(current);
>>>       sessionid = audit_get_sessionid(current);
>>> @@ -2366,6 +2367,12 @@ static void audit_log_task(struct audit_buffer *ab)
>>>       audit_log_task_context(ab);
>>>       audit_log_format(ab, " pid=%d comm=", current->pid);
>>>       audit_log_untrustedstring(ab, current->comm);
>>> +     if (mm) {
>
> if using get_task_mm() drop below
>
>>> +             down_read(&mm->mmap_sem);
>>> +             if (mm->exe_file)
>>> +                     audit_log_d_path(ab, " exe=", &mm->exe_file->f_path);
>
> if using get_task_mm() change below to mmput(mm);
>
>>> +             up_read(&mm->mmap_sem);
>>> +     }
>>>  }
>>>

<snip>
One other thing that I know Steve Grubb is picky on, is the field
still needs to be there even if mm is null. We can't have
disappearing fields. On error conditions, I've been doing
<fieldname>=(null) on my patches.