[GIT PULL] Audit patches for v4.10
by Paul Moore
Hi Linus,
After the small number of patches for v4.9, we've got a much bigger pile for
v4.10.
The bulk of these patches involve a rework of the audit backlog queue to
enable us to move the netlink multicasting out of the task/thread that
generates the audit record and into the kernel thread that emits the record
(just like we do for the audit unicast to auditd). While we were playing
with the backlog queue(s) we fixed a number of other little problems with
the code, and from all the testing so far things look to be in much better
shape now. Doing this also allowed us to re-enable disabling IRQs for some
netns operations ("netns: avoid disabling irq for netns id"). The remaining
patches fix some small problems that are well documented in the commit
descriptions, as well as adding session ID filtering support.
You will likely hit two merge conflicts, one in net/core/net_namespace.c and
one in include/uapi/linux/audit.h, both are easily resolved so I won't
bother you with that here. If you have questions, you know how to find me.
Thanks,
-Paul
---
The following changes since commit c8d2bc9bc39ebea8437fd974fdbc21847bb897a3:
Linux 4.8 (2016-10-02 16:24:33 -0700)
are available in the git repository at:
git://git.infradead.org/users/pcmoore/audit stable-4.10
for you to fetch changes up to 533c7b69c764ad5febb3e716899f43a75564fcab:
audit: use proper refcount locking on audit_sock
(2016-12-14 13:06:04 -0500)
----------------------------------------------------------------
Alexey Dobriyan (1):
audit: less stack usage for /proc/*/loginuid
Paul Moore (9):
audit: fixup audit_init()
audit: queue netlink multicast sends just like we do for unicast sends
audit: rename the queues and kauditd related functions
audit: rework the audit queue handling
audit: rework audit_log_start()
audit: wake up kauditd_thread after auditd registers
audit: handle a clean auditd shutdown with grace
audit: don't ever sleep on a command record/message
netns: avoid disabling irq for netns id
Richard Guy Briggs (5):
audit: tame initialization warning len_abuf in audit_log_execve_info
audit: skip sessionid sentinel value when auto-incrementing
audit: add support for session ID user filter
audit: move kaudit thread start from auditd registration to
kaudit init (#2)
audit: use proper refcount locking on audit_sock
Steve Grubb (1):
audit: fix formatting of AUDIT_CONFIG_CHANGE events
fs/proc/base.c | 2 +-
include/uapi/linux/audit.h | 5 +-
kernel/audit.c | 532 ++++++++++++++++++++++++---------------
kernel/audit_fsnotify.c | 5 +-
kernel/audit_tree.c | 3 +-
kernel/audit_watch.c | 5 +-
kernel/auditfilter.c | 5 +-
kernel/auditsc.c | 12 +-
net/core/net_namespace.c | 35 ++-
9 files changed, 361 insertions(+), 243 deletions(-)
--
paul moore
security @ redhat
9 years, 2 months
Re: netlink: GPF in sock_sndtimeo
by Richard Guy Briggs
On 2016-11-29 23:52, Richard Guy Briggs wrote:
> On 2016-11-29 15:13, Cong Wang wrote:
> > On Tue, Nov 29, 2016 at 8:48 AM, Richard Guy Briggs <rgb(a)redhat.com> wrote:
> > > On 2016-11-26 17:11, Cong Wang wrote:
> > >> It is racy on audit_sock, especially on the netns exit path.
> > >
> > > I think that is the only place it is racy. The other places audit_sock
> > > is set is when the socket failure has just triggered a reset.
> > >
> > > Is there a notifier callback for failed or reaped sockets?
> >
> > Is NETLINK_URELEASE event what you are looking for?
>
> Possibly, yes. Thanks, I'll have a look.
I tried a quick compile attempt on the test case (I assume it is a
socket fuzzer) and get the following compile error:
cc -g -O0 -Wall -D_GNU_SOURCE -o socket_fuzz socket_fuzz.c
socket_fuzz.c:16:1: warning: "_GNU_SOURCE" redefined
<command-line>: warning: this is the location of the previous definition
socket_fuzz.c: In function ‘segv_handler’:
socket_fuzz.c:89: warning: implicit declaration of function ‘__atomic_load_n’
socket_fuzz.c:89: error: ‘__ATOMIC_RELAXED’ undeclared (first use in this function)
socket_fuzz.c:89: error: (Each undeclared identifier is reported only once
socket_fuzz.c:89: error: for each function it appears in.)
socket_fuzz.c: In function ‘loop’:
socket_fuzz.c:280: warning: unused variable ‘errno0’
socket_fuzz.c: In function ‘test’:
socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_add’
socket_fuzz.c:303: error: ‘__ATOMIC_SEQ_CST’ undeclared (first use in this function)
socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_sub’
I also tried to extend Cong Wang's idea to attempt to proactively respond to a
NETLINK_URELEASE on the audit_sock and reset it, but ran into a locking error
stack dump using mutex_lock(&audit_cmd_mutex) in the notifier callback.
Eliminating the lock since the sock is dead anways eliminates the error.
Is it safe? I'll resubmit if this looks remotely sane. Meanwhile I'll try to
get the test case to compile.
This is being tracked as https://github.com/linux-audit/audit-kernel/issues/30
Subject: [PATCH] audit: proactively reset audit_sock on matching NETLINK_URELEASE
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..91d222d 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -423,6 +423,7 @@ static void kauditd_send_skb(struct sk_buff *skb)
snprintf(s, sizeof(s), "audit_pid=%d reset", audit_pid);
audit_log_lost(s);
audit_pid = 0;
+ audit_nlk_portid = 0;
audit_sock = NULL;
} else {
pr_warn("re-scheduling(#%d) write to audit_pid=%d\n",
@@ -1143,6 +1144,28 @@ static int audit_bind(struct net *net, int group)
return 0;
}
+static int audit_sock_netlink_notify(struct notifier_block *nb,
+ unsigned long event,
+ void *_notify)
+{
+ struct netlink_notify *notify = _notify;
+ struct audit_net *aunet = net_generic(notify->net, audit_net_id);
+
+ if (event == NETLINK_URELEASE && notify->protocol == NETLINK_AUDIT) {
+ if (audit_nlk_portid == notify->portid &&
+ audit_sock == aunet->nlsk) {
+ audit_pid = 0;
+ audit_nlk_portid = 0;
+ audit_sock = NULL;
+ }
+ }
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block audit_netlink_notifier = {
+ .notifier_call = audit_sock_netlink_notify,
+};
+
static int __net_init audit_net_init(struct net *net)
{
struct netlink_kernel_cfg cfg = {
@@ -1167,10 +1190,14 @@ static void __net_exit audit_net_exit(struct net *net)
{
struct audit_net *aunet = net_generic(net, audit_net_id);
struct sock *sock = aunet->nlsk;
+
+ mutex_lock(&audit_cmd_mutex);
if (sock == audit_sock) {
audit_pid = 0;
+ audit_nlk_portid = 0;
audit_sock = NULL;
}
+ mutex_unlock(&audit_cmd_mutex);
RCU_INIT_POINTER(aunet->nlsk, NULL);
synchronize_net();
@@ -1202,6 +1229,7 @@ static int __init audit_init(void)
audit_enabled = audit_default;
audit_ever_enabled |= !!audit_default;
+ netlink_register_notifier(&audit_netlink_notifier);
audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized");
for (i = 0; i < AUDIT_INODE_BUCKETS; i++)
--
1.7.1
> - RGB
- RGB
--
Richard Guy Briggs <rgb(a)redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
9 years, 2 months
[RFC][PATCH] audit: add feature audit_lost reset
by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..5eb2dc2 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -330,10 +330,12 @@ enum {
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
#define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004
#define AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND 0x00000008
+#define AUDIT_FEATURE_BITMAP_LOST_RESET 0x00000010
#define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \
AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH | \
- AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND)
+ AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \
+ AUDIT_FEATURE_BITMAP_LOST_RESET)
/* deprecated: AUDIT_VERSION_* */
#define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALL
@@ -440,7 +442,8 @@ struct audit_features {
#define AUDIT_FEATURE_ONLY_UNSET_LOGINUID 0
#define AUDIT_FEATURE_LOGINUID_IMMUTABLE 1
-#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LOGINUID_IMMUTABLE
+#define AUDIT_FEATURE_LOST_RESET 2
+#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LOST_RESET
#define audit_feature_valid(x) ((x) >= 0 && (x) <= AUDIT_LAST_FEATURE)
#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & 31)) /* mask for __u32 */
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..6b52da6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
@@ -150,9 +150,10 @@
.features = 0,
.lock = 0,};
-static char *audit_feature_names[2] = {
+static char *audit_feature_names[3] = {
"only_unset_loginuid",
"loginuid_immutable",
+ "lost_reset",
};
@@ -854,7 +855,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
s.pid = audit_pid;
s.rate_limit = audit_rate_limit;
s.backlog_limit = audit_backlog_limit;
- s.lost = atomic_read(&audit_lost);
+ s.lost = is_audit_feature_set(AUDIT_FEATURE_LOST_RESET) ?
+ atomic_xchg(&audit_lost, 0) :
+ atomic_read(&audit_lost);
s.backlog = skb_queue_len(&audit_skb_queue);
s.feature_bitmap = AUDIT_FEATURE_BITMAP_ALL;
s.backlog_wait_time = audit_backlog_wait_time_master;
--
1.7.1
9 years, 2 months
Re: netlink: GPF in sock_sndtimeo
by Richard Guy Briggs
On 2016-12-09 12:53, Dmitry Vyukov wrote:
> On Fri, Dec 9, 2016 at 12:48 PM, Richard Guy Briggs <rgb(a)redhat.com> wrote:
> > On 2016-12-09 11:49, Dmitry Vyukov wrote:
> >> On Fri, Dec 9, 2016 at 7:02 AM, Richard Guy Briggs <rgb(a)redhat.com> wrote:
> >> > On 2016-11-29 23:52, Richard Guy Briggs wrote:
> >> > I tried a quick compile attempt on the test case (I assume it is a
> >> > socket fuzzer) and get the following compile error:
> >> > cc -g -O0 -Wall -D_GNU_SOURCE -o socket_fuzz socket_fuzz.c
> >> > socket_fuzz.c:16:1: warning: "_GNU_SOURCE" redefined
> >> > <command-line>: warning: this is the location of the previous definition
> >> > socket_fuzz.c: In function ‘segv_handler’:
> >> > socket_fuzz.c:89: warning: implicit declaration of function ‘__atomic_load_n’
> >> > socket_fuzz.c:89: error: ‘__ATOMIC_RELAXED’ undeclared (first use in this function)
> >> > socket_fuzz.c:89: error: (Each undeclared identifier is reported only once
> >> > socket_fuzz.c:89: error: for each function it appears in.)
> >> > socket_fuzz.c: In function ‘loop’:
> >> > socket_fuzz.c:280: warning: unused variable ‘errno0’
> >> > socket_fuzz.c: In function ‘test’:
> >> > socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_add’
> >> > socket_fuzz.c:303: error: ‘__ATOMIC_SEQ_CST’ undeclared (first use in this function)
> >> > socket_fuzz.c:303: warning: implicit declaration of function ‘__atomic_fetch_sub’
> >>
> >> -std=gnu99 should help
> >> ignore warnings
> >
> > I got a little further, left with "__ATOMIC_RELAXED undeclared", "__ATOMIC_SEQ_CST
> > undeclared" under gcc 4.4.7-16.
> >
> > gcc 4.8.2-15 leaves me with "undefined reference to `clock_gettime'"
>
> add -lrt
Ok, that helped. Thanks!
> > What compiler version do you recommend?
>
> 6.x sounds reasonable
> 4.4 branch is 7.5 years old, surprised that it does not disintegrate
> into dust yet :)
These are under RHEL6... so there are updates to them, but yeah, they are old.
> >> >> - RGB
> >> >
> >> > - RGB
> >
> > - RGB
> >
> > --
> > Richard Guy Briggs <rgb(a)redhat.com>
> > Kernel Security Engineering, Base Operating Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635
- RGB
--
Richard Guy Briggs <rgb(a)redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
9 years, 2 months
[PATCH 1/1] audit: Make AUDIT_ANOM_ABEND event normalized
by Steve Grubb
The audit event specification asks for certain fields to exist in
all events. Running 'ausearch -m anom_abend -sv yes' returns no
events. This patch adds the result field so that the
AUDIT_ANOM_ABEND event conforms to the rules.
Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
---
kernel/auditsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 5abf1dc..4317f5b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2403,7 +2403,7 @@ void audit_core_dumps(long signr)
if (unlikely(!ab))
return;
audit_log_task(ab);
- audit_log_format(ab, " sig=%ld", signr);
+ audit_log_format(ab, " sig=%ld res=1", signr);
audit_log_end(ab);
}
--
2.7.4
9 years, 2 months
[userspace PATCH] Prevent free() of stack buffer with NOLOG format
by George McCollister
When the NOLOG format is used replace_event_msg() doesn't change
e->reply.message so the message located on the stack is left and later is
free()'d in cleanup_event() resulting in the following:
*** Error in `auditd': free(): invalid pointer: 0x800bef7c ***
======= Backtrace: =========
/lib/libc.so.6(+0x676ba)[0xb752b6ba]
/lib/libc.so.6(+0x6e227)[0xb7532227]
/lib/libc.so.6(+0x6e9e6)[0xb75329e6]
auditd(+0x73df)[0x800a43df]
auditd(+0x4975)[0x800a1975]
auditd(+0x4a9c)[0x800a1a9c]
auditd(main+0x931)[0x800a0c21]
/lib/libc.so.6(__libc_start_main+0xf6)[0xb74dc1a6]
auditd(+0x44c4)[0x800a14c4]
======= Memory map: ========
...
This patch changes the log format to RAW when NOLOG format is detected
so that replace_event_msg() will replace e->reply.message with a message
that can be free()'d by cleanup_event().
Signed-off-by: George McCollister <george.mccollister(a)gmail.com>
---
src/auditd-config.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/auditd-config.c b/src/auditd-config.c
index 584f079..bc06b1c 100644
--- a/src/auditd-config.c
+++ b/src/auditd-config.c
@@ -839,6 +839,7 @@ static int log_format_parser(struct nv_pair *nv, int line,
if (strcasecmp(nv->value, log_formats[i].name) == 0) {
config->log_format = log_formats[i].option;
if (config->log_format == LF_NOLOG) {
+ config->log_format = LF_RAW;
audit_msg(LOG_WARNING,
"The NOLOG option to log_format is deprecated. Please use the write_logs option.");
if (config->write_logs != 0)
--
2.9.3
9 years, 2 months
EOE events in auparse output
by Nikolai Kondrashov
Hi Steve, everyone,
I was playing with auditd and aushape on Fedora 24 and found some strange
entries in my log. There was a separate *event* produced by auparse containing
a single EOE record. These events had the same serial number as the directly
preceding events, which were exclusively containing SYSCALL records.
Those EOE records didn't appear in the audit.log file.
Is this a bug? Is this normal?
Thank you.
Nick
9 years, 2 months
[PATCH] audit: skip sessionid sentinel value when auto-incrementing
by Richard Guy Briggs
The value (unsigned int)-1 is used as a sentinel to indicate the
sessionID is unset. Skip this value when the session_id value wraps.
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
kernel/auditsc.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 5abf1dc..e414dfa 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2025,8 +2025,11 @@ int audit_set_loginuid(kuid_t loginuid)
goto out;
/* are we setting or clearing? */
- if (uid_valid(loginuid))
+ if (uid_valid(loginuid)) {
sessionid = (unsigned int)atomic_inc_return(&session_id);
+ if (unlikely(sessionid == (unsigned int)-1))
+ sessionid = (unsigned int)atomic_inc_return(&session_id);
+ }
task->sessionid = sessionid;
task->loginuid = loginuid;
--
1.7.1
9 years, 2 months
[PATCH] audit: remove the audit freelist
by Florian Westphal
allows better debugging as freeing audit buffers now always honors slub
debug hooks (e.g. object poisoning) and leak checker can detect the
free operation.
Removal also results in a small speedup (using
single rule 'iptables -A INPUT -i lo -j AUDIT --type drop'):
super_netperf 4 -H 127.0.0.1 -l 360 -t UDP_RR -- -R 1 -m 64
Before:
294953
After:
298013
(alloc/free no longer serializes on spinlock, allocator can use percpu
pool).
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
kernel/audit.c | 53 ++++++++---------------------------------------------
1 file changed, 8 insertions(+), 45 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca11613379..396868dc523a 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -131,13 +131,6 @@ static int audit_net_id;
/* Hash for inode-based rules */
struct list_head audit_inode_hash[AUDIT_INODE_BUCKETS];
-/* The audit_freelist is a list of pre-allocated audit buffers (if more
- * than AUDIT_MAXFREE are in use, the audit buffer is freed instead of
- * being placed on the freelist). */
-static DEFINE_SPINLOCK(audit_freelist_lock);
-static int audit_freelist_count;
-static LIST_HEAD(audit_freelist);
-
static struct sk_buff_head audit_skb_queue;
/* queue of skbs to send to auditd when/if it comes back */
static struct sk_buff_head audit_skb_hold_queue;
@@ -164,17 +157,11 @@ DEFINE_MUTEX(audit_cmd_mutex);
* should be at least that large. */
#define AUDIT_BUFSIZ 1024
-/* AUDIT_MAXFREE is the number of empty audit_buffers we keep on the
- * audit_freelist. Doing so eliminates many kmalloc/kfree calls. */
-#define AUDIT_MAXFREE (2*NR_CPUS)
-
-/* The audit_buffer is used when formatting an audit record. The caller
- * locks briefly to get the record off the freelist or to allocate the
- * buffer, and locks briefly to send the buffer to the netlink layer or
+/* The audit_buffer is used when formatting an audit record.
+ * The caller locks briefly to send the buffer to the netlink layer or
* to place it on a transmit queue. Multiple audit_buffers can be in
* use simultaneously. */
struct audit_buffer {
- struct list_head list;
struct sk_buff *skb; /* formatted skb ready to send */
struct audit_context *ctx; /* NULL or associated context */
gfp_t gfp_mask;
@@ -1247,43 +1234,22 @@ __setup("audit_backlog_limit=", audit_backlog_limit_set);
static void audit_buffer_free(struct audit_buffer *ab)
{
- unsigned long flags;
-
if (!ab)
return;
kfree_skb(ab->skb);
- spin_lock_irqsave(&audit_freelist_lock, flags);
- if (audit_freelist_count > AUDIT_MAXFREE)
- kfree(ab);
- else {
- audit_freelist_count++;
- list_add(&ab->list, &audit_freelist);
- }
- spin_unlock_irqrestore(&audit_freelist_lock, flags);
+ kfree(ab);
}
static struct audit_buffer * audit_buffer_alloc(struct audit_context *ctx,
gfp_t gfp_mask, int type)
{
- unsigned long flags;
- struct audit_buffer *ab = NULL;
+ struct audit_buffer *ab;
struct nlmsghdr *nlh;
- spin_lock_irqsave(&audit_freelist_lock, flags);
- if (!list_empty(&audit_freelist)) {
- ab = list_entry(audit_freelist.next,
- struct audit_buffer, list);
- list_del(&ab->list);
- --audit_freelist_count;
- }
- spin_unlock_irqrestore(&audit_freelist_lock, flags);
-
- if (!ab) {
- ab = kmalloc(sizeof(*ab), gfp_mask);
- if (!ab)
- goto err;
- }
+ ab = kmalloc(sizeof(*ab), gfp_mask);
+ if (!ab)
+ return NULL;
ab->ctx = ctx;
ab->gfp_mask = gfp_mask;
@@ -1294,13 +1260,10 @@ static struct audit_buffer * audit_buffer_alloc(struct audit_context *ctx,
nlh = nlmsg_put(ab->skb, 0, 0, type, 0, 0);
if (!nlh)
- goto out_kfree_skb;
+ goto err;
return ab;
-out_kfree_skb:
- kfree_skb(ab->skb);
- ab->skb = NULL;
err:
audit_buffer_free(ab);
return NULL;
--
2.7.3
9 years, 2 months
