On Fri, May 4, 2018 at 4:54 PM, Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> Recognizing that the audit context is an internal audit value, use an
> access function to retrieve the audit context pointer for the task
> rather than reaching directly into the task struct to get it.
>
> Signed-off-by: Richard Guy Briggs <rgb@xxxxxxxxxx>
> ---
> include/linux/audit.h | 16 ++++++++---
> include/net/xfrm.h | 2 +-
> kernel/audit.c | 4 +--
> kernel/audit_watch.c | 2 +-
> kernel/auditsc.c | 52 ++++++++++++++++++------------------
> net/bridge/netfilter/ebtables.c | 2 +-
> net/core/dev.c | 2 +-
> net/netfilter/x_tables.c | 2 +-
> net/netlabel/netlabel_user.c | 2 +-
> security/integrity/ima/ima_api.c | 2 +-
> security/integrity/integrity_audit.c | 2 +-
> security/lsm_audit.c | 2 +-
> security/selinux/hooks.c | 4 +--
> security/selinux/selinuxfs.c | 6 ++---
> security/selinux/ss/services.c | 12 ++++-----
> 15 files changed, 60 insertions(+), 52 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 5f86f7c..93e4c61 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -235,26 +235,30 @@ extern void __audit_inode_child(struct inode *parent,
> extern void __audit_seccomp(unsigned long syscall, long signr, int code);
> extern void __audit_ptrace(struct task_struct *t);
>
> +static inline struct audit_context *audit_context(struct task_struct *task)
> +{
> + return task->audit_context;
> +}
Another case where I think I agree with everything here on principle,
especially when one considers it in the larger context of the audit
container ID work. However, I think we might be able to somply this a
bit by eliminating the parameter to the new audit_context() helper and
making it always reference the current task_struct. Based on this
patch it would appear that this change would work for all callers
except for audit_take_context() and __audit_syscall_entry(), both of
which are contained within the core audit code and are enough of a
special case that I think it is acceptable for them to access the
context directly. I'm trying to think of reasons why a non-audit
kernel subsystem would ever need to access the audit context of a
process other than current and I can't think of any ... removing the
task_struct pointer might help prevent mistakes/abuse in the future.
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 6e3ceb9..a4bbdcc 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -836,7 +836,7 @@ static inline struct audit_context *audit_take_context(struct task_struct *tsk,
> int return_valid,
> long return_code)
> {
> - struct audit_context *context = tsk->audit_context;
> + struct audit_context *context = audit_context(tsk);
>
> if (!context)
> return NULL;
> @@ -1510,7 +1510,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
> unsigned long a3, unsigned long a4)
> {
> struct task_struct *tsk = current;
> - struct audit_context *context = tsk->audit_context;
> + struct audit_context *context = audit_context(tsk);
> enum audit_state state;
>
> if (!audit_enabled || !context)
--
paul moore
www.paul-moore.com
