Re: [BUG][PATCH] audit: audit_log_start running on auditd should not stop
by Richard Guy Briggs
On Tue, Oct 15, 2013 at 02:30:34PM +0800, Gao feng wrote:
> Hi Toshiyuki-san,
Toshiuki and Gao,
> On 10/15/2013 12:43 PM, Toshiyuki Okajima wrote:
> > The backlog cannot be consumed when audit_log_start is running on auditd
> > even if audit_log_start calls wait_for_auditd to consume it.
> > The situation is a deadlock because only auditd can consume the backlog.
> > If the other process needs to send the backlog, it can be also stopped
> > by the deadlock.
> >
> > So, audit_log_start running on auditd should not stop.
> >
> > You can see the deadlock with the following reproducer:
> > # auditctl -a exit,always -S all
> > # reboot
> Hmm, I see, There may be other code paths that auditd can call audit_log_start except
> audit_log_config_change. so it's better to handle this problem in audit_log_start.
>
> but current task is only meaningful when gfp_mask & __GFP_WAIT is true.
> so maybe the below patch is what you want.
I have been following this thread with interest. I like the general
evolution of this patch. The first patch was a bit too abrupt, dropping
too much, but this one makes much more sense. I would be tempted to
make the reserve even bigger.
I see that you should be using a kernel that has included commit
8ac1c8d5 (which made it into v3.12-rc3)
audit: fix endless wait in audit_log_start()
That was an obvious bug, but I was still concerned about the cause of
the initial wait. There are other fixes and ideas in the works that
should alleviate some of the pressure to make the service more usable.
https://lkml.org/lkml/2013/9/18/453
I have tested with and without this v3 patch and I don't see any
significant difference with the reproducer provided above. I'm also
testing with a reproducer of the endless wait bug (readahead-collector).
What are your expected results? What are your actual results in each
case? How are they different?
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 7b0e23a..10b4545 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1095,7 +1095,9 @@ struct audit_buffer *audit_log_start(struct audit_context
> struct audit_buffer *ab = NULL;
> struct timespec t;
> unsigned int uninitialized_var(serial);
> - int reserve;
> + int reserve = 5; /* Allow atomic callers to go up to five
> + entries over the normal backlog limit */
> +
> unsigned long timeout_start = jiffies;
>
> if (audit_initialized != AUDIT_INITIALIZED)
> @@ -1104,11 +1106,12 @@ struct audit_buffer *audit_log_start(struct audit_contex
> if (unlikely(audit_filter_type(type)))
> return NULL;
>
> - if (gfp_mask & __GFP_WAIT)
> - reserve = 0;
> - else
> - reserve = 5; /* Allow atomic callers to go up to five
> - entries over the normal backlog limit */
> + if (gfp_mask & __GFP_WAIT) {
> + if (audit_pid && audit_pid == current->pid)
> + gfp_mask &= ~__GFP_WAIT;
> + else
> + reserve = 0;
> + }
>
> while (audit_backlog_limit
> && skb_queue_len(&audit_skb_queue) > audit_backlog_limit + reserv
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545
10 years, 10 months
Format specifier issue when building kernel
by William Roberts
I've been working off of Richard Guy Brigs git repo on branch
audit-for-next prepping my patch and I noticed a build warning:
kernel/audit.c:832:8: warning: format ‘%A’ expects argument of type
‘double’, but argument 3 has type ‘char *’ [-Wformat]
Looking at the code, it looks wrong:
--
Respectfully,
William C Roberts
11 years
[PATCH] audit: don't generate loginuid log when audit disabled
by Gao feng
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
---
kernel/auditsc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 065c7a1..92d0e92 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1990,6 +1990,9 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
struct audit_buffer *ab;
uid_t uid, ologinuid, nloginuid;
+ if (audit_enabled == AUDIT_OFF)
+ return;
+
uid = from_kuid(&init_user_ns, task_uid(current));
ologinuid = from_kuid(&init_user_ns, koldloginuid);
nloginuid = from_kuid(&init_user_ns, kloginuid),
--
1.8.3.1
11 years
[PATCH 1/3] audit: fix incorrect order of log new and old feature
by Gao feng
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
---
kernel/audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 7c7c028..f16f835 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -664,7 +664,7 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature
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_log_format(ab, "feature=%s old=%d new=%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);
--
1.8.3.1
11 years
[PATCH 0/4][v2] audit: Tidy up audit_context and stop bprm recursion
by Richard Guy Briggs
This patchset is a clean up of the audit_aux_data and audit_context structures
and the audit_bprm() call that was needlessly recursing, allocating more
resources than necessary.
Eric W. Biederman (1):
audit: Kill the unused struct audit_aux_data_capset
Richard Guy Briggs (3):
audit: remove unused envc member of audit_aux_data_execve
audit: move audit_aux_data_execve contents into audit_context union
audit: call audit_bprm() only once to add AUDIT_EXECVE information
fs/exec.c | 5 +----
include/linux/audit.h | 13 +++++--------
kernel/audit.h | 3 +++
kernel/auditsc.c | 49 ++++++++++---------------------------------------
4 files changed, 19 insertions(+), 51 deletions(-)
11 years
[PATCH] audit: Add cmdline to taskinfo output
by William Roberts
On some devices, the cmdline and task info vary. For instance, on
Android, the cmdline is set to the package name, and the task info
is the name of the VM, which is not very helpful.
The additional cmdline output only runs if the audit feature
AUDIT_FEATURE_CMDLINE_OUTPUT is set high at runtime.
Signed-off-by: William Roberts <wroberts(a)tresys.com>
---
Sorry for the noise, the commit message got truncated last time.
This will apply to Richard's tree on branch audit-for-next.
This requires eparis's generic get/set patches.
fs/proc/base.c | 2 +-
include/linux/proc_fs.h | 1 +
include/uapi/linux/audit.h | 4 +++-
kernel/audit.c | 38 +++++++++++++++++++++++++++++++++++++-
4 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 03c8d74..cb1ba2f 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -200,7 +200,7 @@ static int proc_root_link(struct dentry *dentry, struct path *path)
return result;
}
-static int proc_pid_cmdline(struct task_struct *task, char * buffer)
+int proc_pid_cmdline(struct task_struct *task, char *buffer)
{
int res = 0;
unsigned int len;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 608e60a..2f386b3 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -41,6 +41,7 @@ extern void *proc_get_parent_data(const struct inode *);
extern void proc_remove(struct proc_dir_entry *);
extern void remove_proc_entry(const char *, struct proc_dir_entry *);
extern int remove_proc_subtree(const char *, struct proc_dir_entry *);
+extern int proc_pid_cmdline(struct task_struct *, char *);
#else /* CONFIG_PROC_FS */
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index e2f0d99..5d77124 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -388,7 +388,9 @@ 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_CMDLINE_OUTPUT 2
+#define AUDIT_LAST_FEATURE AUDIT_FEATURE_CMDLINE_OUTPUT
+
#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 8378c5e..bf4b1af 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -63,6 +63,7 @@
#include <linux/freezer.h>
#include <linux/tty.h>
#include <linux/pid_namespace.h>
+#include <linux/proc_fs.h>
#include "audit.h"
@@ -144,9 +145,10 @@ static struct audit_features af = {.vers = AUDIT_FEATURE_VERSION,
.features = 0,
.lock = 0,};
-static char *audit_feature_names[2] = {
+static char *audit_feature_names[3] = {
"only_unset_loginuid",
"loginuid_immutable",
+ "audit_output_cmdline",
};
@@ -1691,6 +1693,39 @@ error_path:
}
EXPORT_SYMBOL(audit_log_task_context);
+static void audit_log_add_cmdline(struct audit_buffer *ab,
+ struct task_struct *tsk)
+{
+ int len;
+ unsigned long page;
+
+ /* Ensure that the feature is set */
+ if (!is_audit_feature_set(AUDIT_FEATURE_CMDLINE_OUTPUT))
+ return;
+
+ /* Get the process cmdline */
+ page = __get_free_page(GFP_TEMPORARY);
+ if (!page)
+ return;
+
+ len = proc_pid_cmdline(tsk, (char *)page);
+ if (len <= 0) {
+ free_page(page);
+ return;
+ }
+
+ /*
+ * Ensure NULL terminated! Application could
+ * could be using setproctitle(3).
+ */
+ ((char *)page)[len-1] = '\0';
+
+ audit_log_format(ab, " cmdline=");
+ audit_log_untrustedstring(ab, (char *)page);
+
+ free_page(page);
+}
+
void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
{
const struct cred *cred;
@@ -1739,6 +1774,7 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
up_read(&mm->mmap_sem);
}
audit_log_task_context(ab);
+ audit_log_add_cmdline(ab, tsk);
}
EXPORT_SYMBOL(audit_log_task_info);
--
1.7.9.5
11 years
[PATCH] audit: remove useless code in audit_enable
by Gao feng
Since kernel parameter is operated before
initcall, so the audit_initialized must be
AUDIT_UNINITIALIZED or DISABLED in audit_enable.
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
---
kernel/audit.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 50fdcba..b7269a4 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -927,17 +927,8 @@ static int __init audit_enable(char *str)
if (!audit_default)
audit_initialized = AUDIT_DISABLED;
- printk(KERN_INFO "audit: %s", audit_default ? "enabled" : "disabled");
-
- if (audit_initialized == AUDIT_INITIALIZED) {
- audit_enabled = audit_default;
- audit_ever_enabled |= !!audit_default;
- } else if (audit_initialized == AUDIT_UNINITIALIZED) {
- printk(" (after initialization)");
- } else {
- printk(" (until reboot)");
- }
- printk("\n");
+ printk(KERN_INFO "audit: %s\n", audit_default ?
+ "enabled (after initialization)" : "disabled (until reboot)");
return 1;
}
--
1.8.3.1
11 years
[PATCH 0/3] audit: Tidy up audit_context and stop bprm recursion
by Richard Guy Briggs
This patchset is a clean up of the audit_aux_data and audit_context structures
and the audit_bprm() call that was needlessly recursing, allocating more
resources than necessary.
Eric W. Biederman (1):
audit: Kill the unused struct audit_aux_data_capset
Richard Guy Briggs (2):
audit: remove unused envc member of audit_aux_data_execve
audit: call audit_bprm() only once to add AUDIT_EXECVE information
fs/exec.c | 5 +----
include/linux/audit.h | 13 +++++--------
kernel/audit.h | 3 +++
kernel/auditsc.c | 49 ++++++++++---------------------------------------
4 files changed, 19 insertions(+), 51 deletions(-)
11 years
UNSUBSCRIBE
by Wilson, Mark - IS
UNSUBSCRIBE
________________________________
This e-mail and any files transmitted with it may be proprietary and are intended solely for the use of the individual or entity to whom they are addressed. If you have received this e-mail in error please notify the sender. Please note that any views or opinions presented in this e-mail are solely those of the author and do not necessarily represent those of Exelis Inc. The recipient should check this e-mail and any attachments for the presence of viruses. Exelis Inc. accepts no liability for any damage caused by any virus transmitted by this e-mail.
11 years
ABIs, syscall tables, and the AUDIT_ARCH_* defines
by Paul Moore
Hello all,
I've been dealing with the AUDIT_ARCH_* defines, different ABIs and syscall
tables a fair amount lately as part of libseccomp[1] and a little birdie
thought it might be a good idea to post something to the Linux audit list.
So here we go. I'll try to be brief.
First off, if you already understand that in some cases a given AUDIT_ARCH_*
value can represent multiple ABIs and you are fine with that, feel free to hit
delete now and move on. I'm not trying to argue that what audit is currently
doing is right or wrong, just trying to make things perhaps a bit more clear.
The core issue is that AUDIT_ARCH alone can not be used to specify a given
ABI, all the AUDIT_ARCH value can tell you is the syscall table; which in it's
defense, is all the original source comments claim. However, if you want to
be able to identify an ABI, I'm finding that you need both the AUDIT_ARCH
value and the syscall number (from my experience this hold true for x86,
x86_64, x32, ARM OABI, and ARM EABI, I can't speak for others at this point).
Take x86_64 and x32 as an example (think of x32 as a 32-bit version of
x86_64). Both x32 and x86_64 use the AUDIT_ARCH_X86_64 value and general
calling convention, but they have a different syscall table. The x32 syscall
table is the same as the x86_64 syscall table but with a 0x40000000 offset,
e.g. on x86_64 the write() syscall is 0x01 but on x32 write() is 0x40000001.
The 32-bit ARM ABIs are similar in that the EABI and OABI ABIs share the same
AUDIT_ARCH_ARM value and have similar syscall tables, separated by a 0x900000
offset. With ARM there is some additional oddities between OABI and EABI with
respect to syscall arguments, but I'm still figuring that out myself and it
wouldn't be right for me to talk about that here.
There ya go, hopefully this helps somewhat. If you have any questions I'll do
my best to try and answer them.
-Paul
[1] http://sourceforge.net/projects/libseccomp
--
paul moore
security and virtualization @ redhat
11 years
