Re: [PATCH ghak96] audit: set cwd in audit context for file-related LSM audit records

From: "Casey Schaufler" <casey(a)schaufler-ca.com&gt; Subject: Re: [PATCH ghak96] audit: set cwd in audit context for file-related LSM audit records On 4/2/2020 7:13 AM, Vladis Dronov wrote: > Set a current working directory in an audit context for the following > record > types in dump_common_audit_data(): LSM_AUDIT_DATA_PATH, > LSM_AUDIT_DATA_FILE, > LSM_AUDIT_DATA_IOCTL_OP, LSM_AUDIT_DATA_DENTRY, LSM_AUDIT_DATA_INODE so a > separate CWD record is emitted later. > > Link: https://github.com/linux-audit/audit-kernel/issues/96 I don't have a problem with the patch, but it sure would be nice if you explained why these events "could use a CWD record".

> Signed-off-by: Vladis Dronov <vdronov(a)redhat.com&gt; > --- > out-of-commit-message-note: > > Hello, > Honestly, I'm not sure about "if (!context->in_syscall)" check in > __audit_getcwd(). It was copied from __audit_getname() and I do > not quite understand why it is there and if __audit_getcwd() needs > it. If you have an idea on this, could you please, tell? > > include/linux/audit.h | 9 ++++++++- > kernel/auditsc.c | 17 +++++++++++++++++ > security/lsm_audit.c | 5 +++++ > 3 files changed, 30 insertions(+), 1 deletion(-) > > diff --git a/include/linux/audit.h b/include/linux/audit.h > index f9ceae57ca8d..b4306abc5891 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -268,7 +268,7 @@ extern void __audit_syscall_entry(int major, unsigned > long a0, unsigned long a1, > extern void __audit_syscall_exit(int ret_success, long ret_value); > extern struct filename *__audit_reusename(const __user char *uptr); > extern void __audit_getname(struct filename *name); > - > +extern void __audit_getcwd(void); > extern void __audit_inode(struct filename *name, const struct dentry > *dentry, > unsigned int flags); > extern void __audit_file(const struct file *); > @@ -327,6 +327,11 @@ static inline void audit_getname(struct filename > *name) > if (unlikely(!audit_dummy_context())) > __audit_getname(name); > } > +static inline void audit_getcwd(void) > +{ > + if (unlikely(!audit_dummy_context())) > + __audit_getcwd(); > +} > static inline void audit_inode(struct filename *name, > const struct dentry *dentry, > unsigned int aflags) { > @@ -545,6 +550,8 @@ static inline struct filename *audit_reusename(const > __user char *name) > } > static inline void audit_getname(struct filename *name) > { } > +static inline void audit_getcwd(void) > +{ } > static inline void __audit_inode(struct filename *name, > const struct dentry *dentry, > unsigned int flags) > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 814406a35db1..16316032ef9f 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -1890,6 +1890,23 @@ void __audit_getname(struct filename *name) > get_fs_pwd(current->fs, &context->pwd); > } > > +/** > + * __audit_getcwd - set a current working directory > + * > + * Set a current working directory of an audited process for this context. > + * Called from security/lsm_audit.c:dump_common_audit_data(). > + */ > +void __audit_getcwd(void) > +{ > + struct audit_context *context = audit_context(); > + > + if (!context->in_syscall) > + return; > + > + if (!context->pwd.dentry) > + get_fs_pwd(current->fs, &context->pwd); > +} > + > static inline int audit_copy_fcaps(struct audit_names *name, > const struct dentry *dentry) > { > diff --git a/security/lsm_audit.c b/security/lsm_audit.c > index 2d2bf49016f4..7c555621c2bd 100644 > --- a/security/lsm_audit.c > +++ b/security/lsm_audit.c > @@ -241,6 +241,7 @@ static void dump_common_audit_data(struct audit_buffer > *ab, > audit_log_untrustedstring(ab, inode->i_sb->s_id); > audit_log_format(ab, " ino=%lu", inode->i_ino); > } > + audit_getcwd(); > break; > } > case LSM_AUDIT_DATA_FILE: { > @@ -254,6 +255,7 @@ static void dump_common_audit_data(struct audit_buffer > *ab, > audit_log_untrustedstring(ab, inode->i_sb->s_id); > audit_log_format(ab, " ino=%lu", inode->i_ino); > } > + audit_getcwd(); > break; > } > case LSM_AUDIT_DATA_IOCTL_OP: { > @@ -269,6 +271,7 @@ static void dump_common_audit_data(struct audit_buffer > *ab, > } > > audit_log_format(ab, " ioctlcmd=0x%hx", a->u.op->cmd); > + audit_getcwd(); > break; > } > case LSM_AUDIT_DATA_DENTRY: { > @@ -283,6 +286,7 @@ static void dump_common_audit_data(struct audit_buffer > *ab, > audit_log_untrustedstring(ab, inode->i_sb->s_id); > audit_log_format(ab, " ino=%lu", inode->i_ino); > } > + audit_getcwd(); > break; > } > case LSM_AUDIT_DATA_INODE: { > @@ -300,6 +304,7 @@ static void dump_common_audit_data(struct audit_buffer > *ab, > audit_log_format(ab, " dev="); > audit_log_untrustedstring(ab, inode->i_sb->s_id); > audit_log_format(ab, " ino=%lu", inode->i_ino); > + audit_getcwd(); > break; > } > case LSM_AUDIT_DATA_TASK: {