linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Richard Guy Briggs <rgb@redhat.com>
To: William Roberts <bill.c.roberts@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	William Roberts <wroberts@tresys.com>,
	linux-audit@redhat.com, viro@zeniv.linux.org.uk,
	akpm@linux-foundation.org
Subject: Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
Date: Tue, 14 Jan 2014 22:44:18 -0500	[thread overview]
Message-ID: <20140115034418.GH23577@madcap2.tricolour.ca> (raw)
In-Reply-To: <CAFftDdpSm=LyWBaMJban+0ZTxR0iS-rvuLELA9Xj936XjL4zLA@mail.gmail.com>

On 14/01/14, William Roberts wrote:
> The race was non existent. I had the VMA locked. I switched to this to keep
> the code that gets the cmdline value almost unchanged to try and reduce
> bugs. I can still author a patch on top of this later to optimize. However
> the buffer is smaller. Before it was page size, now its path max....iirc is
> smaller.

Both are 4K on what I'm looking at.

> On Jan 14, 2014 5:45 PM, "Richard Guy Briggs" <rgb@redhat.com> wrote:
> 
> > On 14/01/06, William Roberts wrote:
> > > During an audit event, cache and print the value of the process's
> > > cmdline value (proc/<pid>/cmdline). This is useful in situations
> > > where processes are started via fork'd virtual machines where the
> > > comm field is incorrect. Often times, setting the comm field still
> > > is insufficient as the comm width is not very wide and most
> > > virtual machine "package names" do not fit. Also, during execution,
> > > many threads have their comm field set as well. By tying it back to
> > > the global cmdline value for the process, audit records will be more
> > > complete in systems with these properties. An example of where this
> > > is useful and applicable is in the realm of Android. With Android,
> > > their is no fork/exec for VM instances. The bare, preloaded Dalvik
> > > VM listens for a fork and specialize request. When this request comes
> > > in, the VM forks, and the loads the specific application (specializing).
> > > This was done to take advantage of COW and to not require a load of
> > > basic packages by the VM on very app spawn. When this spawn occurs,
> > > the package name is set via setproctitle() and shows up in procfs.
> > > Many of these package names are longer then 16 bytes, the historical
> > > width of task->comm. Having the cmdline in the audit records will
> > > couple the application back to the record directly. Also, on my
> > > Debian development box, some audit records were more useful then
> > > what was printed under comm.
> >
> > So...  What happenned to allocating only what you need instead of the
> > full 4k buffer?  Your test results showed promise with only 64 or 128
> > bytes allocated.  I recall seeing some discussion about a race between
> > testing for the size needed and actually filling the buffer, but was
> > hoping that would be worked on rather than reverting back to the full
> > 4k.
> >
> > > The cached cmdline is tied to the life-cycle of the audit_context
> > > structure and is built on demand.
> > >
> > > Example denial prior to patch (Ubuntu):
> > > CALL msg=audit(1387828084.070:361): arch=c000003e syscall=82 success=yes
> > exit=0 a0=4184bf a1=418547 a2=0 a3=0 items=0 ppid=1 pid=1329
> > auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0
> > ses=4294967295 tty=(none) comm="console-kit-dae"
> > exe="/usr/sbin/console-kit-daemon"
> > subj=system_u:system_r:consolekit_t:s0-s0:c0.c255 key=(null)
> > >
> > > After Patches (Ubuntu):
> > > type=SYSCALL msg=audit(1387828084.070:361): arch=c000003e syscall=82
> > success=yes exit=0 a0=4184bf a1=418547 a2=0 a3=0 items=0 ppid=1 pid=1329
> > auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0
> > ses=4294967295 tty=(none) comm="console-kit-dae"
> > exe="/usr/sbin/console-kit-daemon"
> > subj=system_u:system_r:consolekit_t:s0-s0:c0.c255
> > cmdline="/usr/lib/dbus-1.0/dbus-daemon-launch-helper" key=(null)
> > >
> > > Example denial prior to patch (Android):
> > > type=1300 msg=audit(248323.940:247): arch=40000028 syscall=54 per=840000
> > success=yes exit=0 a0=39 a1=540b a2=2 a3=750eecec items=0 ppid=224 pid=1858
> > auid=4294967295 uid=1002 gid=1002 euid=1002 suid=1002 fsuid=1002 egid=1002
> > sgid=1002 fsgid=1002 tty=(none) ses=4294967295 comm="bt_hc_worker"
> > exe="/system/bin/app_process" subj=u:r:bluetooth:s0 key=(null)
> > >
> > > After Patches (Android):
> > > type=1300 msg=audit(248323.940:247): arch=40000028 syscall=54 per=840000
> > success=yes exit=0 a0=39 a1=540b a2=2 a3=750eecec items=0 ppid=224 pid=1858
> > auid=4294967295 uid=1002 gid=1002 euid=1002 suid=1002 fsuid=1002 egid=1002
> > sgid=1002 fsgid=1002 tty=(none) ses=4294967295 comm="bt_hc_worker"
> > exe="/system/bin/app_process" cmdline="com.android.bluetooth"
> > subj=u:r:bluetooth:s0 key=(null)
> > >
> > > Signed-off-by: William Roberts <wroberts@tresys.com>
> > > ---
> > >  kernel/audit.h   |    1 +
> > >  kernel/auditsc.c |   32 ++++++++++++++++++++++++++++++++
> > >  2 files changed, 33 insertions(+)
> > >
> > > diff --git a/kernel/audit.h b/kernel/audit.h
> > > index b779642..bd6211f 100644
> > > --- a/kernel/audit.h
> > > +++ b/kernel/audit.h
> > > @@ -202,6 +202,7 @@ struct audit_context {
> > >               } execve;
> > >       };
> > >       int fds[2];
> > > +     char *cmdline;
> > >
> > >  #if AUDIT_DEBUG
> > >       int                 put_count;
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index 90594c9..a4c2003 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -842,6 +842,12 @@ static inline struct audit_context
> > *audit_get_context(struct task_struct *tsk,
> > >       return context;
> > >  }
> > >
> > > +static inline void audit_cmdline_free(struct audit_context *context)
> > > +{
> > > +     kfree(context->cmdline);
> > > +     context->cmdline = NULL;
> > > +}
> > > +
> > >  static inline void audit_free_names(struct audit_context *context)
> > >  {
> > >       struct audit_names *n, *next;
> > > @@ -955,6 +961,7 @@ static inline void audit_free_context(struct
> > audit_context *context)
> > >       audit_free_aux(context);
> > >       kfree(context->filterkey);
> > >       kfree(context->sockaddr);
> > > +     audit_cmdline_free(context);
> > >       kfree(context);
> > >  }
> > >
> > > @@ -1271,6 +1278,30 @@ static void show_special(struct audit_context
> > *context, int *call_panic)
> > >       audit_log_end(ab);
> > >  }
> > >
> > > +static void audit_log_cmdline(struct audit_buffer *ab, struct
> > task_struct *tsk,
> > > +                      struct audit_context *context)
> > > +{
> > > +     int res;
> > > +     char *buf;
> > > +     char *msg = "(null)";
> > > +     audit_log_format(ab, " cmdline=");
> > > +
> > > +     /* Not  cached */
> > > +     if (!context->cmdline) {
> > > +             buf = kmalloc(PATH_MAX, GFP_KERNEL);
> > > +             if (!buf)
> > > +                     goto out;
> > > +             res = get_cmdline(tsk, buf, PATH_MAX);
> > > +             /* Ensure NULL terminated */
> > > +             if (buf[res-1] != '\0')
> > > +                     buf[res-1] = '\0';
> > > +             context->cmdline = buf;
> > > +     }
> > > +     msg = context->cmdline;
> > > +out:
> > > +     audit_log_untrustedstring(ab, msg);
> > > +}
> > > +
> > >  static void audit_log_exit(struct audit_context *context, struct
> > task_struct *tsk)
> > >  {
> > >       int i, call_panic = 0;
> > > @@ -1302,6 +1333,7 @@ static void audit_log_exit(struct audit_context
> > *context, struct task_struct *ts
> > >                        context->name_count);
> > >
> > >       audit_log_task_info(ab, tsk);
> > > +     audit_log_cmdline(ab, tsk, context);
> > >       audit_log_key(ab, context->filterkey);
> > >       audit_log_end(ab);
> > >
> > > --
> > > 1.7.9.5
> > >
> > > --
> > > Linux-audit mailing list
> > > Linux-audit@redhat.com
> > > https://www.redhat.com/mailman/listinfo/linux-audit
> >
> > - RGB
> >
> > --
> > Richard Guy Briggs <rbriggs@redhat.com>
> > Senior Software Engineer, Kernel Security, AMER ENG Base Operating
> > Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
> >

> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit


- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2014-01-15  3:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-06 15:30 [RFC][PATCH 1/3] mm: Create utility function for accessing a tasks commandline value William Roberts
2014-01-06 15:30 ` [RFC][PATCH 2/3] proc: Update get proc_pid_cmdline() to use mm.h helpers William Roberts
2014-01-06 15:30 ` [RFC][PATCH 3/3] audit: Audit proc cmdline value William Roberts
2014-01-06 17:08   ` Mateusz Guzik
2014-01-06 17:26     ` William Roberts
2014-01-06 17:30     ` William Roberts
2014-01-14 22:45   ` Richard Guy Briggs
2014-01-15  0:50     ` William Roberts
2014-01-15  0:56       ` William Roberts
2014-01-15  3:44       ` Richard Guy Briggs [this message]
2014-01-15 12:40   ` Paul Davies C
  -- strict thread matches above, loose matches on Subject: below --
2013-12-23 21:01 [RFC][PATCH 1/3] mm: Create utility function for accessing a tasks commandline value William Roberts
2013-12-23 21:01 ` [RFC][PATCH 3/3] audit: Audit proc cmdline value William Roberts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140115034418.GH23577@madcap2.tricolour.ca \
    --to=rgb@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bill.c.roberts@gmail.com \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=wroberts@tresys.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox