[PATCH 1/1] Added exe field to audit core dump signal log
by Paul Davies C
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.
Signed-off-by: Paul Davies C <pauldaviesc(a)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;
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) {
+ down_read(&mm->mmap_sem);
+ if (mm->exe_file)
+ audit_log_d_path(ab, " exe=", &mm->exe_file->f_path);
+ up_read(&mm->mmap_sem);
+ }
}
static void audit_log_abend(struct audit_buffer *ab, char *reason, long signr)
--
1.7.9.5
9 years, 3 months
[PATCH 1/7] audit: implement generic feature setting and retrieving
by Eric Paris
The audit_status structure was not designed with extensibility in mind.
Define a new AUDIT_SET_FEATURE message type which takes a new structure
of bits where things can be enabled/disabled/locked one at a time. This
structure should be able to grow in the future while maintaining forward
and backward compatibility (based loosly on the ideas from capabilities
and prctl)
This does not actually add any features, but is just infrastructure to
allow new on/off types of audit system features.
Signed-off-by: Eric Paris <eparis(a)redhat.com>
---
include/linux/audit.h | 2 +
include/uapi/linux/audit.h | 16 +++++++
kernel/audit.c | 110 ++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 127 insertions(+), 1 deletion(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 729a4d1..7b31bec 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -73,6 +73,8 @@ struct audit_field {
void *lsm_rule;
};
+extern int is_audit_feature_set(int which);
+
extern int __init audit_register_class(int class, unsigned *list);
extern int audit_classify_syscall(int abi, unsigned syscall);
extern int audit_classify_arch(int arch);
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index b7cb978..a053243 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -68,6 +68,9 @@
#define AUDIT_MAKE_EQUIV 1015 /* Append to watched tree */
#define AUDIT_TTY_GET 1016 /* Get TTY auditing status */
#define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
+#define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off */
+#define AUDIT_GET_FEATURE 1019 /* Get which features are enabled */
+#define AUDIT_FEATURE_CHANGE 1020 /* audit log listing feature changes */
#define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly uninteresting to kernel */
#define AUDIT_USER_AVC 1107 /* We filter this differently */
@@ -369,6 +372,19 @@ struct audit_status {
__u32 backlog; /* messages waiting in queue */
};
+struct audit_features {
+#define AUDIT_FEATURE_VERSION 1
+ __u32 vers;
+ __u32 mask; /* which bits we are dealing with */
+ __u32 features; /* which feature to enable/disable */
+ __u32 lock; /* which features to lock */
+};
+
+#define AUDIT_LAST_FEATURE -1
+
+#define audit_feature_valid(x) ((x) >= 0 && (x) <= AUDIT_LAST_FEATURE)
+#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & 31)) /* mask for __u32 */
+
struct audit_tty_status {
__u32 enabled; /* 1 = enabled, 0 = disabled */
__u32 log_passwd; /* 1 = enabled, 0 = disabled */
diff --git a/kernel/audit.c b/kernel/audit.c
index f2f4666..3acbbc8 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -140,6 +140,15 @@ static struct task_struct *kauditd_task;
static DECLARE_WAIT_QUEUE_HEAD(kauditd_wait);
static DECLARE_WAIT_QUEUE_HEAD(audit_backlog_wait);
+static struct audit_features af = {.vers = AUDIT_FEATURE_VERSION,
+ .mask = -1,
+ .features = 0,
+ .lock = 0,};
+
+static char *audit_feature_names[0] = {
+};
+
+
/* Serialize requests from userspace. */
DEFINE_MUTEX(audit_cmd_mutex);
@@ -584,6 +593,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
return -EOPNOTSUPP;
case AUDIT_GET:
case AUDIT_SET:
+ case AUDIT_GET_FEATURE:
+ case AUDIT_SET_FEATURE:
case AUDIT_LIST_RULES:
case AUDIT_ADD_RULE:
case AUDIT_DEL_RULE:
@@ -628,6 +639,94 @@ static int audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
return rc;
}
+int is_audit_feature_set(int i)
+{
+ return af.features & AUDIT_FEATURE_TO_MASK(i);
+}
+
+
+static int audit_get_feature(struct sk_buff *skb)
+{
+ u32 seq;
+
+ seq = nlmsg_hdr(skb)->nlmsg_seq;
+
+ audit_send_reply(NETLINK_CB(skb).portid, seq, AUDIT_GET, 0, 0,
+ &af, sizeof(af));
+
+ return 0;
+}
+
+static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature,
+ u32 old_lock, u32 new_lock, int res)
+{
+ struct audit_buffer *ab;
+
+ ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE);
+ audit_log_format(ab, "feature=%s new=%d old=%d old_lock=%d new_lock=%d res=%d",
+ audit_feature_names[which], !!old_feature, !!new_feature,
+ !!old_lock, !!new_lock, res);
+ audit_log_end(ab);
+}
+
+static int audit_set_feature(struct sk_buff *skb)
+{
+ struct audit_features *uaf;
+ int i;
+
+ BUILD_BUG_ON(AUDIT_LAST_FEATURE + 1 > sizeof(audit_feature_names)/sizeof(audit_feature_names[0]));
+ uaf = nlmsg_data(nlmsg_hdr(skb));
+
+ /* if there is ever a version 2 we should handle that here */
+
+ for (i = 0; i <= AUDIT_LAST_FEATURE; i++) {
+ u32 feature = AUDIT_FEATURE_TO_MASK(i);
+ u32 old_feature, new_feature, old_lock, new_lock;
+
+ /* if we are not changing this feature, move along */
+ if (!(feature & uaf->mask))
+ continue;
+
+ old_feature = af.features & feature;
+ new_feature = uaf->features & feature;
+ new_lock = (uaf->lock | af.lock) & feature;
+ old_lock = af.lock & feature;
+
+ /* are we changing a locked feature? */
+ if ((af.lock & feature) && (new_feature != old_feature)) {
+ audit_log_feature_change(i, old_feature, new_feature,
+ old_lock, new_lock, 0);
+ return -EPERM;
+ }
+ }
+ /* nothing invalid, do the changes */
+ for (i = 0; i <= AUDIT_LAST_FEATURE; i++) {
+ u32 feature = AUDIT_FEATURE_TO_MASK(i);
+ u32 old_feature, new_feature, old_lock, new_lock;
+
+ /* if we are not changing this feature, move along */
+ if (!(feature & uaf->mask))
+ continue;
+
+ old_feature = af.features & feature;
+ new_feature = uaf->features & feature;
+ old_lock = af.lock & feature;
+ new_lock = (uaf->lock | af.lock) & feature;
+
+ if (new_feature != old_feature)
+ audit_log_feature_change(i, old_feature, new_feature,
+ old_lock, new_lock, 1);
+
+ if (new_feature)
+ af.features |= feature;
+ else
+ af.features &= ~feature;
+ af.lock |= new_lock;
+ }
+
+ return 0;
+}
+
static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
{
u32 seq;
@@ -699,7 +798,16 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
if (status_get->mask & AUDIT_STATUS_BACKLOG_LIMIT)
err = audit_set_backlog_limit(status_get->backlog_limit);
break;
- case AUDIT_USER:
+ case AUDIT_GET_FEATURE:
+ err = audit_get_feature(skb);
+ if (err)
+ return err;
+ break;
+ case AUDIT_SET_FEATURE:
+ err = audit_set_feature(skb);
+ if (err)
+ return err;
+ break;
case AUDIT_FIRST_USER_MSG ... AUDIT_LAST_USER_MSG:
case AUDIT_FIRST_USER_MSG2 ... AUDIT_LAST_USER_MSG2:
if (!audit_enabled && msg_type != AUDIT_USER_AVC)
--
1.8.2.1
10 years, 2 months
[PATCH 0/4] arm64: Add audit support
by AKASHI Takahiro
This patchset adds audit support on arm64.
The implementation is just like in other architectures,
and so I think little explanation is needed.
I verified this patch with some commands on both 64-bit rootfs
and 32-bit rootfs(, but only in little-endian):
# auditctl -a exit,always -S openat -F path=/etc/inittab
# auditctl -a exit,always -F dir=/tmp -F perm=rw
# auditctl -a task,always
# autrace /bin/ls
What else?
(Thanks to Clayton for his cross-compiling patch)
I'd like to discuss about the following issues:
(issues)
* AUDIT_ARCH_*
Why do we need to distiguish big-endian and little-endian? [2/4]
* AArch32
We need to add a check for identifying the endian in 32-bit tasks. [3/4]
* syscall no in AArch32
Currently all the definitions are added in unistd32.h with
"ifdef __AARCH32_AUDITSYSCALL" to use asm-generic/audit_*.h. [3/4]
"ifdef" is necessary to avoid a conflict with 64-bit definitions.
Do we need a more sophisticated way?
* TIF_AUDITSYSCALL
Most architectures, except x86, do not check TIF_AUDITSYSCALL. Why not? [4/4]
* Userspace audit package
There are some missing syscall definitions in lib/aarch64_table.h.
There is no support for AUDIT_ARCH_ARM (I mean LE. armeb is BE).
AKASHI Takahiro (4):
audit: Enable arm64 support
arm64: Add audit support
arm64: audit: Add AArch32 support
arm64: audit: Add audit hook in ptrace/syscall_trace
arch/arm64/Kconfig | 3 +
arch/arm64/include/asm/audit32.h | 12 ++
arch/arm64/include/asm/ptrace.h | 5 +
arch/arm64/include/asm/syscall.h | 18 ++
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/include/asm/unistd32.h | 387 ++++++++++++++++++++++++++++++++++
arch/arm64/kernel/Makefile | 4 +
arch/arm64/kernel/audit.c | 77 +++++++
arch/arm64/kernel/audit32.c | 46 ++++
arch/arm64/kernel/entry.S | 3 +
arch/arm64/kernel/ptrace.c | 12 ++
include/uapi/linux/audit.h | 2 +
init/Kconfig | 2 +-
13 files changed, 571 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/audit32.h
create mode 100644 arch/arm64/kernel/audit.c
create mode 100644 arch/arm64/kernel/audit32.c
--
1.7.9.5
10 years, 5 months
[PATCH] Support for auditing on the actions of a not-yet-executed process.
by Peter Moody
eg:
-a exit,always -F arch=b64 -S socket -F 'a0!=1' -F exe=/bin/bash -F success=1
to see instances of /bin/bash opening a non-local socket. Or
-a exit,always -F arch=b64 -S socket -F 'a0!=1' -F exe_children=/bin/bash -F success=1
to instances of /bin/bash, and any descendant processes, opening a non local socket.
proposed https://www.redhat.com/archives/linux-audit/2012-June/msg00002.html
and it seemed like there was interest.
Signed-off-by: Peter Moody <pmoody(a)google.com>
---
trunk/lib/errormsg.h | 2 +-
trunk/lib/fieldtab.h | 2 ++
trunk/lib/libaudit.c | 11 +++++++++++
trunk/lib/libaudit.h | 7 ++++++-
4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/trunk/lib/errormsg.h b/trunk/lib/errormsg.h
index 4d996d5..cd595ec 100644
--- a/trunk/lib/errormsg.h
+++ b/trunk/lib/errormsg.h
@@ -51,7 +51,7 @@ static const struct msg_tab err_msgtab[] = {
{ -15, 2, "-F unknown errno -"},
{ -16, 2, "-F unknown file type - " },
{ -17, 1, "can only be used with exit and entry filter list" },
- { -18, 1, "" }, // Unused
+ { -18, 1, "only takes = operator" },
{ -19, 0, "Key field needs a watch or syscall given prior to it" },
{ -20, 2, "-F missing value after operation for" },
{ -21, 2, "-F value should be number for" },
diff --git a/trunk/lib/fieldtab.h b/trunk/lib/fieldtab.h
index c0432cc..245b541 100644
--- a/trunk/lib/fieldtab.h
+++ b/trunk/lib/fieldtab.h
@@ -66,3 +66,5 @@ _S(AUDIT_ARG3, "a3" )
_S(AUDIT_FILTERKEY, "key" )
_S(AUDIT_FIELD_COMPARE, "field_compare" )
+_S(AUDIT_EXE, "exe" )
+_S(AUDIT_EXE_CHILDREN, "exe_children" )
diff --git a/trunk/lib/libaudit.c b/trunk/lib/libaudit.c
index 20eaf5f..06eed86 100644
--- a/trunk/lib/libaudit.c
+++ b/trunk/lib/libaudit.c
@@ -1400,6 +1400,17 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
else
return -21;
break;
+ case AUDIT_EXE_CHILDREN:
+ case AUDIT_EXE:
+ {
+ struct stat buf;
+ if ((stat(v, &buf)) < 0)
+ return -2;
+ if (op != AUDIT_EQUAL)
+ return -18;
+ rule->values[rule->field_count] = (unsigned long)buf.st_ino;
+ }
+ break;
case AUDIT_DEVMAJOR...AUDIT_INODE:
case AUDIT_SUCCESS:
if (flags != AUDIT_FILTER_EXIT)
diff --git a/trunk/lib/libaudit.h b/trunk/lib/libaudit.h
index 89dd588..2c8a802 100644
--- a/trunk/lib/libaudit.h
+++ b/trunk/lib/libaudit.h
@@ -243,6 +243,12 @@ extern "C" {
#ifndef AUDIT_FIELD_COMPARE
#define AUDIT_FIELD_COMPARE 111
#endif
+#ifndef AUDIT_EXE
+#define AUDIT_EXE 112
+#endif
+#ifndef AUDIT_EXE_CHILDREN
+#define AUDIT_EXE_CHILDREN 113
+#endif
#ifndef AUDIT_COMPARE_UID_TO_OBJ_UID
#define AUDIT_COMPARE_UID_TO_OBJ_UID 1
@@ -524,4 +530,3 @@ extern void audit_rule_free_data(struct audit_rule_data *rule);
#endif
#endif
-
--
1.7.7.3
10 years, 6 months
[PATCH] ausearch: Add checkpoint capability and have incomplete logs carry forward when processing multiple audit.log files
by Burn Alting
All,
Attached is a patch for review.
It is against revision 829 within http://svn.fedorahosted.org/svn/audit
This patch
- allows ausearch to checkpoint itself, in that, successive invocations
will only display new events. This is enabled via the --checkpoint fn
option. The mods to ausearch.8 describe the method of achieving this.
- fixes a minor annoyance/bug in that, when ausearch processes events
from multiple audit.log files, incomplete events are considered as
complete (and hence printed) when ausearch encounters an EOF on input
from all the log files being processed. Now, ausearch only flushes
incomplete events on the last log file being processed.
Regards
Burn Alting
10 years, 7 months
[PATCH 00/12] RFC: steps to make audit pid namespace-safe
by Richard Guy Briggs
This patchset is a revival of some of Eric Biederman's work to make audit
pid-namespace-safe.
In a couple of places, audit was printing PIDs in the task's pid namespace
rather than relative to the audit daemon's pid namespace, which currently is
init_pid_ns.
It also allows processes to log audit user messages in their own pid
namespaces, which was not previously permitted. Please see:
https://bugzilla.redhat.com/show_bug.cgi?id=947530
https://bugs.launchpad.net/ubuntu/+source/vsftpd/+bug/1160372
https://bugzilla.novell.com/show_bug.cgi?id=786024
Part of the cleanup here involves deprecating task->pid and task->tgid, which
are error-prone duplicates of the task->pids structure
The next step which I hope to add to this patchset will be to purge task->pid
and task->tgid from the rest of the kernel if possible. Once that is done,
task_pid_nr_init_ns() and task_tgid_nr_init_ns() that were introduced in patch
05/12 and used in patches 06/12 and 08/12 could be replaced with task_pid_nr()
and task_tgid_nr(). Eric B. did take a stab at that, but checking all the
subtleties will be non-trivial.
Does anyone have any opinions or better yet hard data on cache line misses
between pid_nr(struct pid*) and pid_nr_ns(struct pid*, &init_pid_ns)? I'd
like to see pid_nr() use pid_nr_ns(struct pid*, &init_pid_ns), or
pid_nr_init_ns() eliminated in favour of the original pid_nr(). pid_nr()
currently accesses the first level of the pid structure without having to
dereference the level number. If there is an actual speed difference, it could
be worth keeping, otherwise, I'd prefer to simplify that code.
Eric also had a patch to add a printk option to format a struct pid pointer
which was PID namespace-aware. I don't see the point, but I'll let him explain
it.
Discuss.
Eric W. Biederman (5):
audit: Kill the unused struct audit_aux_data_capset
audit: Simplify and correct audit_log_capset
Richard Guy Briggs (7):
audit: fix netlink portid naming and types
pid: get ppid pid_t of task in init_pid_ns safely
audit: convert PPIDs to the inital PID namespace.
pid: get pid_t of task in init_pid_ns correctly
audit: store audit_pid as a struct pid pointer
audit: anchor all pid references in the initial pid namespace
pid: modify task_pid_nr to work without task->pid.
pid: modify task_tgid_nr to work without task->tgid.
pid: rewrite task helper functions avoiding task->pid and task->tgid
pid: mark struct task const in helper functions
drivers/tty/tty_audit.c | 3 +-
include/linux/audit.h | 8 ++--
include/linux/pid.h | 6 +++
include/linux/sched.h | 81 ++++++++++++++++++++++++----------
kernel/audit.c | 76 +++++++++++++++++++------------
kernel/audit.h | 12 +++---
kernel/auditfilter.c | 35 +++++++++++----
kernel/auditsc.c | 36 ++++++---------
kernel/capability.c | 2 +-
kernel/pid.c | 4 +-
security/apparmor/audit.c | 7 +--
security/integrity/integrity_audit.c | 2 +-
security/lsm_audit.c | 11 +++--
security/tomoyo/audit.c | 2 +-
14 files changed, 177 insertions(+), 108 deletions(-)
10 years, 7 months
[RFC PATCH] audit: generic compat system call support
by AKASHI Takahiro
Arm64 supports 32-bit mode(AArch32) and 64-bit mode(AArch64).
To enable audit support, we want to avoid duplicating lib/audit.c
as other arch's do, and instead to use lib/audit.c and extend/re-work it
in order to support compat system calls as well.
Changes are nothing fancy, just copying lib/audit.c and adding hooks
for compat system calls as done in other arch's.
Once this patch is accepted, my aarch64 patch will be rebased on top
of this.
(If you want, I can submit it immediately because it is already working.)
AKASHI Takahiro (1):
audit: Add generic compat syscall support
include/linux/audit.h | 3 +++
lib/Makefile | 3 +++
lib/audit.c | 10 ++++++++
lib/compat_audit.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 80 insertions(+)
create mode 100644 lib/compat_audit.c
--
1.7.9.5
10 years, 7 months
Re: [PATCH] lib/vsprintf: add %pT format specifier
by Tetsuo Handa
Tetsuo Handa wrote:
> Geert Uytterhoeven wrote:
> > On Sat, Jan 11, 2014 at 12:59 AM, Andrew Morton
> > <akpm(a)linux-foundation.org> wrote:
> > >> +char *comm_name(char *buf, char *end, struct task_struct *tsk,
> > >> + struct printf_spec spec, const char *fmt)
> > >> +{
> > >> + char name[TASK_COMM_LEN];
> > >> +
> > >> + /* Caller can pass NULL instead of current. */
> > >> + if (!tsk)
> > >> + tsk = current;
> > >> + /* Not using get_task_comm() in case I'm in IRQ context. */
> > >> + memcpy(name, tsk->comm, TASK_COMM_LEN);
> >
> > So this may copy more bytes than the actual string length of tsk->comm.
> > As this is a temporary buffer, that just wastes cycles.
>
> For example, strncpy() in arch/x86/lib/string_32.c is
>
> char *strncpy(char *dest, const char *src, size_t count)
> {
> int d0, d1, d2, d3;
> asm volatile("1:\tdecl %2\n\t"
> "js 2f\n\t"
> "lodsb\n\t"
> "stosb\n\t"
> "testb %%al,%%al\n\t"
> "jne 1b\n\t"
> "rep\n\t"
> "stosb\n"
> "2:"
> : "=&S" (d0), "=&D" (d1), "=&c" (d2), "=&a" (d3)
> : "" (src), "1" (dest), "2" (count) : "memory");
> return dest;
> }
>
> and strncpy() in lib/string.c is
>
> char *strncpy(char *dest, const char *src, size_t count)
> {
> char *tmp = dest;
>
> while (count) {
> if ((*tmp = *src) != 0)
> src++;
> tmp++;
> count--;
> }
> return dest;
> }
>
> while memcpy(name, tsk->comm, TASK_COMM_LEN) is
>
> u64 *dest = (u64 *) name;
> u64 *src = (u64 *) tsk->comm;
> *dest++ = *src++;
> *dest = *src;
>
> if sizeof(long) == 64. I can't understand why unconditionally copying 8 bytes *
> 2 consumes more cycles than conditionally copying up to 16 bytes...
>
> Also, strncpy() in lib/string.c is not safe for copying task_struct->comm, for
> task_struct->comm can change at any moment.
>
> Initial state:
>
> p->comm contains "secret_commname\0"
>
> A reader calls strncpy(buf, p->comm, 16)
> In strncpy() does
>
> char *dest = buf
> char *src = tsk->comm
> char *tmp = dest
> while (16)
> if ((buf[0] = 's') != 0)
> src++
> tmp++;
> 15
> while (15)
> if ((buf[1] = 'e') != 0)
> src++
> tmp++
> 14
>
> At this moment preemption happens, and a writer jumps in.
> The writer calls set_task_comm(p, "x").
> Now p->comm contains "x\0cret_commname\0".
> The preemption ends and the reader continues the loop.
> Now *src == '\0' but continues copying.
>
> while (14)
> if ((buf[2] = 'c') != 0)
> src++
> tmp++
> 13
> (...snipped...)
> while (1)
> if ((buf[15] = '\0') != 0)
> tmp++
> 0
> return dest
>
> and gets "xecret_commname\0" in the buf.
Oops, my example was bad, though the conclusion does not changte.
Start with "Here We Go\0\0\0\0\0\0", and a preempted writer changes it to
"Let's Go\0\0\0\0\0\0\0\0" when a reader has copied 'H' 'e' 'r' 'e'. Then,
the reader gets "Heres Go\0\0\0\0\0\0\0\0" in the buf.
What I wanted to say is: Do not use strncpy() or strlcpy() for copying
task_struct->comm to temporary buffer, for it can be changed while reading it.
Hello, audit subsystem users.
Below are patches for avoiding racing in audit logs.
----------------------------------------
>From de04a5b08b611293b05b4b4fcc82dc1cd1b89ac3 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp>
Date: Sun, 12 Jan 2014 16:28:12 +0900
Subject: [PATCH 1/4] exec: Add wrapper function for reading task_struct->comm.
Since task_struct->comm can be modified by other threads while the current
thread is reading it, it is recommended to use get_task_comm() for reading it.
However, since get_task_comm() holds task_struct->alloc_lock spinlock,
some users cannot use get_task_comm(). Also, a lot of users are directly
reading from task_struct->comm even if they can use get_task_comm().
Such users might obtain inconsistent result.
This patch introduces a wrapper function for reading task_struct->comm .
Currently this function does not provide consistency. I'm planning to change to
use RCU in the future. By using RCU, the comm name read from task_struct->comm
will be guaranteed to be consistent. But before modifying set_task_comm() to
use RCU, we need to kill direct ->comm users who do not use get_task_comm().
Users directly reading from task_struct->comm for printing purpose can use
%pT format specifier rather than this wrapper function.
Signed-off-by: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp>
---
include/linux/sched.h | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 53f97eb..a31e148 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1665,6 +1665,24 @@ static inline cputime_t task_gtime(struct task_struct *t)
extern void task_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st);
extern void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime_t *st);
+/**
+ * commcpy - Copy task_struct->comm to buffer.
+ *
+ * @buf: Buffer to copy @tsk->comm which must be at least TASK_COMM_LEN bytes.
+ * @tsk: Pointer to "struct task_struct".
+ *
+ * Returns @buf .
+ *
+ * Please use this wrapper function which will be updated in the future to read
+ * @tsk->comm in a consistent way using RCU.
+ */
+static inline char *commcpy(char *buf, const struct task_struct *tsk)
+{
+ memcpy(buf, tsk->comm, TASK_COMM_LEN);
+ buf[TASK_COMM_LEN - 1] = '\0';
+ return buf;
+}
+
/*
* Per process flags
*/
--
1.7.1
----------------------------------------
>From a09631ee2536d581b3c713690cf134cc84c8cce9 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp>
Date: Sun, 12 Jan 2014 16:36:21 +0900
Subject: [PATCH 2/4] LSM: Pass comm name via commcpy()
When we pass task->comm to audit_log_untrustedstring(), we need to pass a
snapshot of it using commcpy() because task->comm can be changed from
"HelloLinuxWorld\0" (a string where
audit_string_contains_control("HelloLinuxWorld\0", 15) would return 0) to
"Good Morning\0\0\0\0" (a string where
audit_string_contains_control("Good Morning\0\0\0\0", 15) would return 1)
during a call to audit_log_untrustedstring(ab, task->comm). As a result,
the audit log will contain unexpected bytes (e.g. '"' and '\0') and might
confuse users who expect that the audit log does not contain such bytes.
Signed-off-by: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp>
---
security/lsm_audit.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 9a62045..a6c9152 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -212,6 +212,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
struct common_audit_data *a)
{
struct task_struct *tsk = current;
+ char name[TASK_COMM_LEN];
/*
* To keep stack sizes in check force programers to notice if they
@@ -221,7 +222,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
BUILD_BUG_ON(sizeof(a->u) > sizeof(void *)*2);
audit_log_format(ab, " pid=%d comm=", tsk->pid);
- audit_log_untrustedstring(ab, tsk->comm);
+ audit_log_untrustedstring(ab, commcpy(name, tsk));
switch (a->type) {
case LSM_AUDIT_DATA_NONE:
@@ -280,7 +281,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
tsk = a->u.tsk;
if (tsk && tsk->pid) {
audit_log_format(ab, " pid=%d comm=", tsk->pid);
- audit_log_untrustedstring(ab, tsk->comm);
+ audit_log_untrustedstring(ab, commcpy(name, tsk));
}
break;
case LSM_AUDIT_DATA_NET:
--
1.7.1
----------------------------------------
>From a3679132e7c22e6c74e5cfc36237656e5b252c52 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp>
Date: Sun, 12 Jan 2014 16:38:32 +0900
Subject: [PATCH 3/4] Integrity: Pass comm name via commcpy()
When we pass task->comm to audit_log_untrustedstring(), we need to pass a
snapshot of it using commcpy() because task->comm can be changed from
"HelloLinuxWorld\0" (a string where
audit_string_contains_control("HelloLinuxWorld\0", 15) would return 0) to
"Good Morning\0\0\0\0" (a string where
audit_string_contains_control("Good Morning\0\0\0\0", 15) would return 1)
during a call to audit_log_untrustedstring(ab, task->comm). As a result,
the audit log will contain unexpected bytes (e.g. '"' and '\0') and might
confuse users who expect that the audit log does not contain such bytes.
Signed-off-by: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp>
---
security/integrity/integrity_audit.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
index d7efb30..eb853d9 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -33,6 +33,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
const char *cause, int result, int audit_info)
{
struct audit_buffer *ab;
+ char name[TASK_COMM_LEN];
if (!integrity_audit_info && audit_info == 1) /* Skip info messages */
return;
@@ -49,7 +50,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
audit_log_format(ab, " cause=");
audit_log_string(ab, cause);
audit_log_format(ab, " comm=");
- audit_log_untrustedstring(ab, current->comm);
+ audit_log_untrustedstring(ab, commcpy(name, current));
if (fname) {
audit_log_format(ab, " name=");
audit_log_untrustedstring(ab, fname);
--
1.7.1
----------------------------------------
>From 8ac36b53256b1495ee3c12f3b52deabdd3e67d72 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp>
Date: Sun, 12 Jan 2014 16:42:50 +0900
Subject: [PATCH 4/4] Audit: Pass comm name via commcpy()
When we pass task->comm to audit_log_untrustedstring(), we need to pass a
snapshot of it using commcpy() because task->comm can be changed from
"HelloLinuxWorld\0" (a string where
audit_string_contains_control("HelloLinuxWorld\0", 15) would return 0) to
"Good Morning\0\0\0\0" (a string where
audit_string_contains_control("Good Morning\0\0\0\0", 15) would return 1)
during a call to audit_log_untrustedstring(ab, task->comm). As a result,
the audit log will contain unexpected bytes (e.g. '"' and '\0') and might
confuse users who expect that the audit log does not contain such bytes.
Signed-off-by: Tetsuo Handa <penguin-kernel(a)I-love.SAKURA.ne.jp>
---
kernel/auditsc.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 90594c9..3b1bf3c 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2352,6 +2352,7 @@ static void audit_log_task(struct audit_buffer *ab)
kuid_t auid, uid;
kgid_t gid;
unsigned int sessionid;
+ char name[TASK_COMM_LEN];
auid = audit_get_loginuid(current);
sessionid = audit_get_sessionid(current);
@@ -2364,7 +2365,7 @@ static void audit_log_task(struct audit_buffer *ab)
sessionid);
audit_log_task_context(ab);
audit_log_format(ab, " pid=%d comm=", current->pid);
- audit_log_untrustedstring(ab, current->comm);
+ audit_log_untrustedstring(ab, commcpy(name, current));
}
static void audit_log_abend(struct audit_buffer *ab, char *reason, long signr)
--
1.7.1
----------------------------------------
10 years, 8 months
[PATCH] loginuid change logging details
by Richard Guy Briggs
I missed posting this before the holidays. I discovered this while adding
other information to other message types.
It seemed to me that loginuid changes were significantly missing context
references. This patch adds that. Is this sufficient, or is there more
information missing too? If this is sufficient, stop reading this cover letter
and review the patch. If it is not sufficient, keep reading below...
The question has been raised that perhaps we should be switching this to use
audit_log_task_info() istead which adds a whole lot more information about this
task.
In the existing message
pid
uid
are already given, before
old-auid
new-auid
old-ses
new-ses
.
The function audit_log_task_info() gives:
ppid
pid
auid
uid
gid
euid
suid
fsuid
egid
sgid
fsgid
tty
ses
comm
exe
res
.
So,
pid
uid
are in the right order, along with
new-auid (auid)
new-ses (ses)
but if we give the
old-auid
old-ses
values first, then call audit_log_task_info(), the old values will preceed
pid
uid
.
Is this re-ordering acceptable to gain more information and reduce code duplicity?
Richard Guy Briggs (1):
audit: log task context when setting loginuid
kernel/auditsc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
10 years, 9 months
