* [PATCH 0/4][v2] audit: Tidy up audit_context and stop bprm recursion
@ 2013-10-31 15:37 Richard Guy Briggs
2013-10-31 15:37 ` [PATCH 1/4][v2] audit: Kill the unused struct audit_aux_data_capset Richard Guy Briggs
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Richard Guy Briggs @ 2013-10-31 15:37 UTC (permalink / raw)
To: linux-audit, linux-kernel
Cc: Richard Guy Briggs, Eric W. Biederman, Oleg Nesterov, Eric Paris
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(-)
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/4][v2] audit: Kill the unused struct audit_aux_data_capset 2013-10-31 15:37 [PATCH 0/4][v2] audit: Tidy up audit_context and stop bprm recursion Richard Guy Briggs @ 2013-10-31 15:37 ` Richard Guy Briggs 2013-10-31 15:37 ` Richard Guy Briggs ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Richard Guy Briggs @ 2013-10-31 15:37 UTC (permalink / raw) To: linux-audit, linux-kernel; +Cc: Eric W. Biederman, Oleg Nesterov, Eric Paris From: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> (cherry picked from commit 6904431d6b41190e42d6b94430b67cb4e7e6a4b7) (cherry picked from commit 2b3a6c617396a9e6eedae9a56b2d9642da0216b6) --- kernel/auditsc.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 95293ab..24047f4 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -121,12 +121,6 @@ struct audit_aux_data_bprm_fcaps { struct audit_cap_data new_pcap; }; -struct audit_aux_data_capset { - struct audit_aux_data d; - pid_t pid; - struct audit_cap_data cap; -}; - struct audit_tree_refs { struct audit_tree_refs *next; struct audit_chunk *c[31]; -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4][v2] audit: remove unused envc member of audit_aux_data_execve 2013-10-31 15:37 [PATCH 0/4][v2] audit: Tidy up audit_context and stop bprm recursion Richard Guy Briggs @ 2013-10-31 15:37 ` Richard Guy Briggs 2013-10-31 15:37 ` Richard Guy Briggs ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Richard Guy Briggs @ 2013-10-31 15:37 UTC (permalink / raw) To: linux-audit, linux-kernel Cc: Richard Guy Briggs, Oleg Nesterov, Eric W. Biederman Get rid of write-only audit_aux_data_exeve structure member envc. Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/auditsc.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 24047f4..c9abaa0 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -98,7 +98,6 @@ struct audit_aux_data { struct audit_aux_data_execve { struct audit_aux_data d; int argc; - int envc; struct mm_struct *mm; }; @@ -2132,7 +2131,6 @@ int __audit_bprm(struct linux_binprm *bprm) return -ENOMEM; ax->argc = bprm->argc; - ax->envc = bprm->envc; ax->mm = bprm->mm; ax->d.type = AUDIT_EXECVE; ax->d.next = context->aux; -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4][v2] audit: remove unused envc member of audit_aux_data_execve @ 2013-10-31 15:37 ` Richard Guy Briggs 0 siblings, 0 replies; 8+ messages in thread From: Richard Guy Briggs @ 2013-10-31 15:37 UTC (permalink / raw) To: linux-audit, linux-kernel Cc: Richard Guy Briggs, Eric W. Biederman, Oleg Nesterov, Eric Paris Get rid of write-only audit_aux_data_exeve structure member envc. Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/auditsc.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 24047f4..c9abaa0 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -98,7 +98,6 @@ struct audit_aux_data { struct audit_aux_data_execve { struct audit_aux_data d; int argc; - int envc; struct mm_struct *mm; }; @@ -2132,7 +2131,6 @@ int __audit_bprm(struct linux_binprm *bprm) return -ENOMEM; ax->argc = bprm->argc; - ax->envc = bprm->envc; ax->mm = bprm->mm; ax->d.type = AUDIT_EXECVE; ax->d.next = context->aux; -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4][v2] audit: move audit_aux_data_execve contents into audit_context union 2013-10-31 15:37 [PATCH 0/4][v2] audit: Tidy up audit_context and stop bprm recursion Richard Guy Briggs @ 2013-10-31 15:37 ` Richard Guy Briggs 2013-10-31 15:37 ` Richard Guy Briggs ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Richard Guy Briggs @ 2013-10-31 15:37 UTC (permalink / raw) To: linux-audit, linux-kernel Cc: Richard Guy Briggs, Oleg Nesterov, Eric W. Biederman audit_bprm() was being called to add an AUDIT_EXECVE record to the audit context every time search_binary_handler() was recursively called. Only one reference is necessary, so just update it. Move the the contents of audit_aux_data_execve into the union in audit_context, removing dependence on a kmalloc along the way. Reported-by: Oleg Nesterov <onestero@redhat.com> Cc: Eric Paris <eparis@redhat.com> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- include/linux/audit.h | 4 ++-- kernel/audit.h | 4 ++++ kernel/auditsc.c | 41 ++++++++++++----------------------------- 3 files changed, 18 insertions(+), 31 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 729a4d1..fffefbd 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -207,7 +207,7 @@ static inline int audit_get_sessionid(struct task_struct *tsk) extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp); extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode); -extern int __audit_bprm(struct linux_binprm *bprm); +extern void __audit_bprm(struct linux_binprm *bprm); extern int __audit_socketcall(int nargs, unsigned long *args); extern int __audit_sockaddr(int len, void *addr); extern void __audit_fd_pair(int fd1, int fd2); @@ -239,7 +239,7 @@ static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid static inline int audit_bprm(struct linux_binprm *bprm) { if (unlikely(!audit_dummy_context())) - return __audit_bprm(bprm); + __audit_bprm(bprm); return 0; } static inline int audit_socketcall(int nargs, unsigned long *args) diff --git a/kernel/audit.h b/kernel/audit.h index 123c9b7..e7b94ab 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -197,6 +197,10 @@ struct audit_context { int fd; int flags; } mmap; + struct { + int argc; + struct mm_struct *mm; + } execve; }; int fds[2]; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index c9abaa0..eabe76a 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -95,12 +95,6 @@ struct audit_aux_data { /* Number of target pids per aux struct. */ #define AUDIT_AUX_PIDS 16 -struct audit_aux_data_execve { - struct audit_aux_data d; - int argc; - struct mm_struct *mm; -}; - struct audit_aux_data_pids { struct audit_aux_data d; pid_t target_pid[AUDIT_AUX_PIDS]; @@ -1144,20 +1138,19 @@ static int audit_log_single_execve_arg(struct audit_context *context, } static void audit_log_execve_info(struct audit_context *context, - struct audit_buffer **ab, - struct audit_aux_data_execve *axi) + struct audit_buffer **ab) { int i, len; size_t len_sent = 0; const char __user *p; char *buf; - if (axi->mm != current->mm) + if (context->execve.mm != current->mm) return; /* execve failed, no additional info */ - p = (const char __user *)axi->mm->arg_start; + p = (const char __user *)current->mm->arg_start; - audit_log_format(*ab, "argc=%d", axi->argc); + audit_log_format(*ab, "argc=%d", context->execve.argc); /* * we need some kernel buffer to hold the userspace args. Just @@ -1171,7 +1164,7 @@ static void audit_log_execve_info(struct audit_context *context, return; } - for (i = 0; i < axi->argc; i++) { + for (i = 0; i < context->execve.argc; i++) { len = audit_log_single_execve_arg(context, ab, i, &len_sent, p, buf); if (len <= 0) @@ -1274,6 +1267,9 @@ static void show_special(struct audit_context *context, int *call_panic) audit_log_format(ab, "fd=%d flags=0x%x", context->mmap.fd, context->mmap.flags); break; } + case AUDIT_EXECVE: { + audit_log_execve_info(context, &ab); + break; } } audit_log_end(ab); } @@ -1320,11 +1316,6 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts switch (aux->type) { - case AUDIT_EXECVE: { - struct audit_aux_data_execve *axi = (void *)aux; - audit_log_execve_info(context, &ab, axi); - break; } - case AUDIT_BPRM_FCAPS: { struct audit_aux_data_bprm_fcaps *axs = (void *)aux; audit_log_format(ab, "fver=%x", axs->fcap_ver); @@ -2121,21 +2112,13 @@ void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mo context->ipc.has_perm = 1; } -int __audit_bprm(struct linux_binprm *bprm) +void __audit_bprm(struct linux_binprm *bprm) { - struct audit_aux_data_execve *ax; struct audit_context *context = current->audit_context; - ax = kmalloc(sizeof(*ax), GFP_KERNEL); - if (!ax) - return -ENOMEM; - - ax->argc = bprm->argc; - ax->mm = bprm->mm; - ax->d.type = AUDIT_EXECVE; - ax->d.next = context->aux; - context->aux = (void *)ax; - return 0; + context->type = AUDIT_EXECVE; + context->execve.argc = bprm->argc; + context->execve.mm = bprm->mm; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4][v2] audit: move audit_aux_data_execve contents into audit_context union @ 2013-10-31 15:37 ` Richard Guy Briggs 0 siblings, 0 replies; 8+ messages in thread From: Richard Guy Briggs @ 2013-10-31 15:37 UTC (permalink / raw) To: linux-audit, linux-kernel Cc: Richard Guy Briggs, Eric W. Biederman, Oleg Nesterov, Eric Paris audit_bprm() was being called to add an AUDIT_EXECVE record to the audit context every time search_binary_handler() was recursively called. Only one reference is necessary, so just update it. Move the the contents of audit_aux_data_execve into the union in audit_context, removing dependence on a kmalloc along the way. Reported-by: Oleg Nesterov <onestero@redhat.com> Cc: Eric Paris <eparis@redhat.com> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- include/linux/audit.h | 4 ++-- kernel/audit.h | 4 ++++ kernel/auditsc.c | 41 ++++++++++++----------------------------- 3 files changed, 18 insertions(+), 31 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 729a4d1..fffefbd 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -207,7 +207,7 @@ static inline int audit_get_sessionid(struct task_struct *tsk) extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp); extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode); -extern int __audit_bprm(struct linux_binprm *bprm); +extern void __audit_bprm(struct linux_binprm *bprm); extern int __audit_socketcall(int nargs, unsigned long *args); extern int __audit_sockaddr(int len, void *addr); extern void __audit_fd_pair(int fd1, int fd2); @@ -239,7 +239,7 @@ static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid static inline int audit_bprm(struct linux_binprm *bprm) { if (unlikely(!audit_dummy_context())) - return __audit_bprm(bprm); + __audit_bprm(bprm); return 0; } static inline int audit_socketcall(int nargs, unsigned long *args) diff --git a/kernel/audit.h b/kernel/audit.h index 123c9b7..e7b94ab 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -197,6 +197,10 @@ struct audit_context { int fd; int flags; } mmap; + struct { + int argc; + struct mm_struct *mm; + } execve; }; int fds[2]; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index c9abaa0..eabe76a 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -95,12 +95,6 @@ struct audit_aux_data { /* Number of target pids per aux struct. */ #define AUDIT_AUX_PIDS 16 -struct audit_aux_data_execve { - struct audit_aux_data d; - int argc; - struct mm_struct *mm; -}; - struct audit_aux_data_pids { struct audit_aux_data d; pid_t target_pid[AUDIT_AUX_PIDS]; @@ -1144,20 +1138,19 @@ static int audit_log_single_execve_arg(struct audit_context *context, } static void audit_log_execve_info(struct audit_context *context, - struct audit_buffer **ab, - struct audit_aux_data_execve *axi) + struct audit_buffer **ab) { int i, len; size_t len_sent = 0; const char __user *p; char *buf; - if (axi->mm != current->mm) + if (context->execve.mm != current->mm) return; /* execve failed, no additional info */ - p = (const char __user *)axi->mm->arg_start; + p = (const char __user *)current->mm->arg_start; - audit_log_format(*ab, "argc=%d", axi->argc); + audit_log_format(*ab, "argc=%d", context->execve.argc); /* * we need some kernel buffer to hold the userspace args. Just @@ -1171,7 +1164,7 @@ static void audit_log_execve_info(struct audit_context *context, return; } - for (i = 0; i < axi->argc; i++) { + for (i = 0; i < context->execve.argc; i++) { len = audit_log_single_execve_arg(context, ab, i, &len_sent, p, buf); if (len <= 0) @@ -1274,6 +1267,9 @@ static void show_special(struct audit_context *context, int *call_panic) audit_log_format(ab, "fd=%d flags=0x%x", context->mmap.fd, context->mmap.flags); break; } + case AUDIT_EXECVE: { + audit_log_execve_info(context, &ab); + break; } } audit_log_end(ab); } @@ -1320,11 +1316,6 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts switch (aux->type) { - case AUDIT_EXECVE: { - struct audit_aux_data_execve *axi = (void *)aux; - audit_log_execve_info(context, &ab, axi); - break; } - case AUDIT_BPRM_FCAPS: { struct audit_aux_data_bprm_fcaps *axs = (void *)aux; audit_log_format(ab, "fver=%x", axs->fcap_ver); @@ -2121,21 +2112,13 @@ void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mo context->ipc.has_perm = 1; } -int __audit_bprm(struct linux_binprm *bprm) +void __audit_bprm(struct linux_binprm *bprm) { - struct audit_aux_data_execve *ax; struct audit_context *context = current->audit_context; - ax = kmalloc(sizeof(*ax), GFP_KERNEL); - if (!ax) - return -ENOMEM; - - ax->argc = bprm->argc; - ax->mm = bprm->mm; - ax->d.type = AUDIT_EXECVE; - ax->d.next = context->aux; - context->aux = (void *)ax; - return 0; + context->type = AUDIT_EXECVE; + context->execve.argc = bprm->argc; + context->execve.mm = bprm->mm; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/4][v2] audit: move audit_aux_data_execve contents into audit_context union 2013-10-31 15:37 ` Richard Guy Briggs (?) @ 2013-10-31 17:18 ` Oleg Nesterov -1 siblings, 0 replies; 8+ messages in thread From: Oleg Nesterov @ 2013-10-31 17:18 UTC (permalink / raw) To: Richard Guy Briggs Cc: linux-audit, linux-kernel, Eric W. Biederman, Eric Paris On 10/31, Richard Guy Briggs wrote: > > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -197,6 +197,10 @@ struct audit_context { > int fd; > int flags; > } mmap; > + struct { > + int argc; > + struct mm_struct *mm; > + } execve; Ah, nice, so we should not worry about kmalloc(GFP_KERNEL) failure. I am in no position to ack the changes in this area, but the whole series looks fine to me. Oleg. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/4][v2] audit: call audit_bprm() only once to add AUDIT_EXECVE information 2013-10-31 15:37 [PATCH 0/4][v2] audit: Tidy up audit_context and stop bprm recursion Richard Guy Briggs ` (2 preceding siblings ...) 2013-10-31 15:37 ` Richard Guy Briggs @ 2013-10-31 15:37 ` Richard Guy Briggs 3 siblings, 0 replies; 8+ messages in thread From: Richard Guy Briggs @ 2013-10-31 15:37 UTC (permalink / raw) To: linux-audit, linux-kernel Cc: Richard Guy Briggs, Eric W. Biederman, Oleg Nesterov, Eric Paris Move the audit_bprm() call from search_binary_handler() to exec_binprm(). This allows us to get rid of the mm member of struct audit_aux_data_execve since bprm->mm will equal current->mm. This also mitigates the issue that ->argc could be modified by the load_binary() call in search_binary_handler(). audit_bprm() was being called to add an AUDIT_EXECVE record to the audit context every time search_binary_handler() was recursively called. Only one reference is necessary. Reported-by: Oleg Nesterov <onestero@redhat.com> Cc: Eric Paris <eparis@redhat.com> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- fs/exec.c | 5 +---- include/linux/audit.h | 9 +++------ kernel/audit.h | 1 - kernel/auditsc.c | 4 ---- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 8875dd1..47d7edb 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1385,10 +1385,6 @@ int search_binary_handler(struct linux_binprm *bprm) if (retval) return retval; - retval = audit_bprm(bprm); - if (retval) - return retval; - retval = -ENOENT; retry: read_lock(&binfmt_lock); @@ -1436,6 +1432,7 @@ static int exec_binprm(struct linux_binprm *bprm) ret = search_binary_handler(bprm); if (ret >= 0) { + audit_bprm(bprm); trace_sched_process_exec(current, old_pid, bprm); ptrace_event(PTRACE_EVENT_EXEC, old_vpid); current->did_exec = 1; diff --git a/include/linux/audit.h b/include/linux/audit.h index fffefbd..a757e6c 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -236,11 +236,10 @@ static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid if (unlikely(!audit_dummy_context())) __audit_ipc_set_perm(qbytes, uid, gid, mode); } -static inline int audit_bprm(struct linux_binprm *bprm) +static inline void audit_bprm(struct linux_binprm *bprm) { if (unlikely(!audit_dummy_context())) __audit_bprm(bprm); - return 0; } static inline int audit_socketcall(int nargs, unsigned long *args) { @@ -367,10 +366,8 @@ static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp) static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode) { } -static inline int audit_bprm(struct linux_binprm *bprm) -{ - return 0; -} +static inline void audit_bprm(struct linux_binprm *bprm) +{ } static inline int audit_socketcall(int nargs, unsigned long *args) { return 0; diff --git a/kernel/audit.h b/kernel/audit.h index e7b94ab..b779642 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -199,7 +199,6 @@ struct audit_context { } mmap; struct { int argc; - struct mm_struct *mm; } execve; }; int fds[2]; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index eabe76a..dc1adee 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -1145,9 +1145,6 @@ static void audit_log_execve_info(struct audit_context *context, const char __user *p; char *buf; - if (context->execve.mm != current->mm) - return; /* execve failed, no additional info */ - p = (const char __user *)current->mm->arg_start; audit_log_format(*ab, "argc=%d", context->execve.argc); @@ -2118,7 +2115,6 @@ void __audit_bprm(struct linux_binprm *bprm) context->type = AUDIT_EXECVE; context->execve.argc = bprm->argc; - context->execve.mm = bprm->mm; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-10-31 17:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-31 15:37 [PATCH 0/4][v2] audit: Tidy up audit_context and stop bprm recursion Richard Guy Briggs 2013-10-31 15:37 ` [PATCH 1/4][v2] audit: Kill the unused struct audit_aux_data_capset Richard Guy Briggs 2013-10-31 15:37 ` [PATCH 2/4][v2] audit: remove unused envc member of audit_aux_data_execve Richard Guy Briggs 2013-10-31 15:37 ` Richard Guy Briggs 2013-10-31 15:37 ` [PATCH 3/4][v2] audit: move audit_aux_data_execve contents into audit_context union Richard Guy Briggs 2013-10-31 15:37 ` Richard Guy Briggs 2013-10-31 17:18 ` Oleg Nesterov 2013-10-31 15:37 ` [PATCH 4/4][v2] audit: call audit_bprm() only once to add AUDIT_EXECVE information Richard Guy Briggs
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.