Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
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 maxiirc is
> smaller.
Both are 4K on what I'm looking at.
> On Jan 14, 2014 5:45 PM, "Richard Guy Briggs" wrote:
>
> > On 14/01/06, William Roberts wrote:
> > > During an audit event, cache and print the value of the process's
> > > cmdline value (proc//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=c03e 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=c03e 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=4028 syscall=54 per=84
> > 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=4028 syscall=54 per=84
> > 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
> > > ---
> > > 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 @@
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
This bounced LKML, re-sending. My phone sent it as HTML
On Tue, Jan 14, 2014 at 7:50 PM, 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 maxiirc is
> smaller.
>
> On Jan 14, 2014 5:45 PM, "Richard Guy Briggs" wrote:
>>
>> On 14/01/06, William Roberts wrote:
>> > During an audit event, cache and print the value of the process's
>> > cmdline value (proc//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=c03e 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=c03e 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=4028 syscall=54 per=84
>> > 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=4028 syscall=54 per=84
>> > 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
>> > ---
>> > 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 @@
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
On 14/01/06, William Roberts wrote:
> During an audit event, cache and print the value of the process's
> cmdline value (proc//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=c03e 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=c03e 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=4028 syscall=54 per=84
> 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=4028 syscall=54 per=84
> 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
> ---
> 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
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
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=c03e 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=c03e 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=4028 syscall=54 per=84
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=4028 syscall=54 per=84
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 wrobe...@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
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
This bounced LKML, re-sending. My phone sent it as HTML
On Tue, Jan 14, 2014 at 7:50 PM, William Roberts
bill.c.robe...@gmail.com 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 maxiirc is
smaller.
On Jan 14, 2014 5:45 PM, Richard Guy Briggs r...@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=c03e 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=c03e 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=4028 syscall=54 per=84
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=4028 syscall=54 per=84
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 wrobe...@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);
+
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
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 maxiirc is
smaller.
Both are 4K on what I'm looking at.
On Jan 14, 2014 5:45 PM, Richard Guy Briggs r...@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=c03e 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=c03e 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=4028 syscall=54 per=84
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=4028 syscall=54 per=84
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 wrobe...@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);
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
On Mon, Jan 6, 2014 at 9:08 AM, Mateusz Guzik wrote:
> I can't comment on the concept, but have one nit.
FYI: The concept is something that has been in the works and at least ackd on
by the current maintainer of audit:
http://marc.info/?l=linux-kernel=138660320704580=2
>
> On Mon, Jan 06, 2014 at 07:30:30AM -0800, William Roberts wrote:
>> +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';
>
> This accesses memory below the buffer if get_cmdline returned 0, which I
> believe will be the case when someone jokingly unmaps the area (all
> maybe when it is swapped out but can't be swapped in due to I/O errors).
>
Yeah that's not a nit, that's a serious issue and I will correct. Thanks.
> Also since you are just putting 0 in there anyway I don't see much point
> in testing for it.
>
>> + context->cmdline = buf;
>> + }
>> + msg = context->cmdline;
>> +out:
>> + audit_log_untrustedstring(ab, msg);
>> +}
>> +
>
>
>
> --
> Mateusz Guzik
--
Respectfully,
William C Roberts
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC][PATCH 3/3] audit: Audit proc cmdline value
-Original Message-
From: Mateusz Guzik [mailto:mgu...@redhat.com]
Sent: Monday, January 06, 2014 9:09 AM
To: William Roberts
Cc: linux-au...@redhat.com; linux...@kvack.org; linux-kernel@vger.kernel.org;
r...@redhat.com; v...@zeniv.linux.org.uk; a...@linux-foundation.org;
s...@tycho.nsa.gov; William Roberts
Subject: Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
I can't comment on the concept, but have one nit.
On Mon, Jan 06, 2014 at 07:30:30AM -0800, William Roberts wrote:
> +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';
This accesses memory below the buffer if get_cmdline returned 0, which I
believe will be the case when someone jokingly unmaps the area (all maybe when
it is swapped out but can't be swapped in due to I/O errors).
[William Roberts]
Sorry for the weird inline posting (Thanks MS Outlook of doom). Anyways, this
isn’t a nit. This is a major issue that should be dealt with. Thanks.
Also since you are just putting 0 in there anyway I don't see much point in
testing for it.
> + context->cmdline = buf;
> + }
> + msg = context->cmdline;
> +out:
> + audit_log_untrustedstring(ab, msg);
> +}
> +
--
Mateusz Guzik
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
I can't comment on the concept, but have one nit.
On Mon, Jan 06, 2014 at 07:30:30AM -0800, William Roberts wrote:
> +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';
This accesses memory below the buffer if get_cmdline returned 0, which I
believe will be the case when someone jokingly unmaps the area (all
maybe when it is swapped out but can't be swapped in due to I/O errors).
Also since you are just putting 0 in there anyway I don't see much point
in testing for it.
> + context->cmdline = buf;
> + }
> + msg = context->cmdline;
> +out:
> + audit_log_untrustedstring(ab, msg);
> +}
> +
--
Mateusz Guzik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH 3/3] audit: Audit proc cmdline value
During an audit event, cache and print the value of the process's
cmdline value (proc//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.
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=c03e 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=c03e 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=4028 syscall=54 per=84
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=4028 syscall=54 per=84
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
---
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 */
+
[RFC][PATCH 3/3] audit: Audit proc cmdline value
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.
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=c03e 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=c03e 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=4028 syscall=54 per=84
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=4028 syscall=54 per=84
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 wrobe...@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 */
+
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
I can't comment on the concept, but have one nit.
On Mon, Jan 06, 2014 at 07:30:30AM -0800, William Roberts wrote:
+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';
This accesses memory below the buffer if get_cmdline returned 0, which I
believe will be the case when someone jokingly unmaps the area (all
maybe when it is swapped out but can't be swapped in due to I/O errors).
Also since you are just putting 0 in there anyway I don't see much point
in testing for it.
+ context-cmdline = buf;
+ }
+ msg = context-cmdline;
+out:
+ audit_log_untrustedstring(ab, msg);
+}
+
--
Mateusz Guzik
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC][PATCH 3/3] audit: Audit proc cmdline value
-Original Message-
From: Mateusz Guzik [mailto:mgu...@redhat.com]
Sent: Monday, January 06, 2014 9:09 AM
To: William Roberts
Cc: linux-au...@redhat.com; linux...@kvack.org; linux-kernel@vger.kernel.org;
r...@redhat.com; v...@zeniv.linux.org.uk; a...@linux-foundation.org;
s...@tycho.nsa.gov; William Roberts
Subject: Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
I can't comment on the concept, but have one nit.
On Mon, Jan 06, 2014 at 07:30:30AM -0800, William Roberts wrote:
+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';
This accesses memory below the buffer if get_cmdline returned 0, which I
believe will be the case when someone jokingly unmaps the area (all maybe when
it is swapped out but can't be swapped in due to I/O errors).
[William Roberts]
Sorry for the weird inline posting (Thanks MS Outlook of doom). Anyways, this
isn’t a nit. This is a major issue that should be dealt with. Thanks.
Also since you are just putting 0 in there anyway I don't see much point in
testing for it.
+ context-cmdline = buf;
+ }
+ msg = context-cmdline;
+out:
+ audit_log_untrustedstring(ab, msg);
+}
+
--
Mateusz Guzik
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [RFC][PATCH 3/3] audit: Audit proc cmdline value
On Mon, Jan 6, 2014 at 9:08 AM, Mateusz Guzik mgu...@redhat.com wrote:
I can't comment on the concept, but have one nit.
FYI: The concept is something that has been in the works and at least ackd on
by the current maintainer of audit:
http://marc.info/?l=linux-kernelm=138660320704580w=2
On Mon, Jan 06, 2014 at 07:30:30AM -0800, William Roberts wrote:
+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';
This accesses memory below the buffer if get_cmdline returned 0, which I
believe will be the case when someone jokingly unmaps the area (all
maybe when it is swapped out but can't be swapped in due to I/O errors).
Yeah that's not a nit, that's a serious issue and I will correct. Thanks.
Also since you are just putting 0 in there anyway I don't see much point
in testing for it.
+ context-cmdline = buf;
+ }
+ msg = context-cmdline;
+out:
+ audit_log_untrustedstring(ab, msg);
+}
+
--
Mateusz Guzik
--
Respectfully,
William C Roberts
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH 3/3] audit: Audit proc cmdline value
During an audit event, cache and print the value of the process's
cmdline value (proc//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.
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=c03e 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=c03e 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=4028 syscall=54 per=84
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=4028 syscall=54 per=84
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
---
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 */
+
[RFC][PATCH 3/3] audit: Audit proc cmdline value
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.
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=c03e 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=c03e 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=4028 syscall=54 per=84
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=4028 syscall=54 per=84
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 wrobe...@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 */
+
