On Wed, Nov 20, 2013 at 2:03 PM, William Roberts <[email protected]> wrote: > On Wed, Nov 20, 2013 at 1:47 PM, Richard Guy Briggs <[email protected]> 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 <[email protected]> >>> --- >>> 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. -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
