Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH ghak96] audit: set cwd in audit context for file-related
 LSM audit records
To:     Vladis Dronov <vdronov@redhat.com>,
        Paul Moore <paul@paul-moore.com>,
        Eric Paris <eparis@redhat.com>, linux-audit@redhat.com,
        James Morris <jmorris@namei.org>,
        "Serge E . Hallyn" <serge@hallyn.com>,
        linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20200402141319.28714-1-vdronov@redhat.com>
From:   Casey Schaufler <casey@schaufler-ca.com>
Autocrypt: addr=casey@schaufler-ca.com; keydata=
 mQINBFzV9HABEAC/mmv3jeJyF7lR7QhILYg1+PeBLIMZv7KCzBSc/4ZZipoWdmr77Lel/RxQ
 1PrNx0UaM5r6Hj9lJmJ9eg4s/TUBSP67mTx+tsZ1RhG78/WFf9aBe8MSXxY5cu7IUwo0J/CG
 vdSqACKyYPV5eoTJmnMxalu8/oVUHyPnKF3eMGgE0mKOFBUMsb2pLS/enE4QyxhcZ26jeeS6
 3BaqDl1aTXGowM5BHyn7s9LEU38x/y2ffdqBjd3au2YOlvZ+XUkzoclSVfSR29bomZVVyhMB
 h1jTmX4Ac9QjpwsxihT8KNGvOM5CeCjQyWcW/g8LfWTzOVF9lzbx6IfEZDDoDem4+ZiPsAXC
 SWKBKil3npdbgb8MARPes2DpuhVm8yfkJEQQmuLYv8GPiJbwHQVLZGQAPBZSAc7IidD2zbf9
 XAw1/SJGe1poxOMfuSBsfKxv9ba2i8hUR+PH7gWwkMQaQ97B1yXYxVEkpG8Y4MfE5Vd3bjJU
 kvQ/tOBUCw5zwyIRC9+7zr1zYi/3hk+OG8OryZ5kpILBNCo+aePeAJ44znrySarUqS69tuXd
 a3lMPHUJJpUpIwSKQ5UuYYkWlWwENEWSefpakFAIwY4YIBkzoJ/t+XJHE1HTaJnRk6SWpeDf
 CreF3+LouP4njyeLEjVIMzaEpwROsw++BX5i5vTXJB+4UApTAQARAQABtChDYXNleSBTY2hh
 dWZsZXIgPGNhc2V5QHNjaGF1Zmxlci1jYS5jb20+iQJUBBMBCAA+FiEEC+9tH1YyUwIQzUIe
 OKUVfIxDyBEFAlzV9HACGwMFCRLMAwAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQOKUV
 fIxDyBG6ag/6AiRl8yof47YOEVHlrmewbpnlBTaYNfJ5cZflNRKRX6t4bp1B2YV1whlDTpiL
 vNOwFkh+ZE0eI5M4x8Gw2Oiok+4Q5liA9PHTozQYF+Ia+qdL5EehfbLGoEBqklpGvG3h8JsO
 7SvONJuFDgvab/U/UriDYycJwzwKZuhVtK9EMpnTtUDyP3DY+Q8h7MWsniNBLVXnh4yBIEJg
 SSgDn3COpZoFTPGKE+rIzioo/GJe8CTa2g+ZggJiY/myWTS3quG0FMvwvNYvZ4I2g6uxSl7n
 bZVqAZgqwoTAv1HSXIAn9muwZUJL03qo25PFi2gQmX15BgJKQcV5RL0GHFHRThDS3IyadOgK
 P2j78P8SddTN73EmsG5OoyzwZAxXfck9A512BfVESqapHurRu2qvMoUkQaW/2yCeRQwGTsFj
 /rr0lnOBkyC6wCmPSKXe3dT2mnD5KnCkjn7KxLqexKt4itGjJz4/ynD/qh+gL7IPbifrQtVH
 JI7cr0fI6Tl8V6efurk5RjtELsAlSR6fKV7hClfeDEgLpigHXGyVOsynXLr59uE+g/+InVic
 jKueTq7LzFd0BiduXGO5HbGyRKw4MG5DNQvC//85EWmFUnDlD3WHz7Hicg95D+2IjD2ZVXJy
 x3LTfKWdC8bU8am1fi+d6tVEFAe/KbUfe+stXkgmfB7pxqW5Ag0EXNX0cAEQAPIEYtPebJzT
 wHpKLu1/j4jQcke06Kmu5RNuj1pEje7kX5IKzQSs+CPH0NbSNGvrA4dNGcuDUTNHgb5Be9hF
 zVqRCEvF2j7BFbrGe9jqMBWHuWheQM8RRoa2UMwQ704mRvKr4sNPh01nKT52ASbWpBPYG3/t
 WbYaqfgtRmCxBnqdOx5mBJIBh9Q38i63DjQgdNcsTx2qS7HFuFyNef5LCf3jogcbmZGxG/b7
 yF4OwmGsVc8ufvlKo5A9Wm+tnRjLr/9Mn9vl5Xa/tQDoPxz26+aWz7j1in7UFzAarcvqzsdM
 Em6S7uT+qy5jcqyuipuenDKYF/yNOVSNnsiFyQTFqCPCpFihOnuaWqfmdeUOQHCSo8fD4aRF
 emsuxqcsq0Jp2ODq73DOTsdFxX2ESXYoFt3Oy7QmIxeEgiHBzdKU2bruIB5OVaZ4zWF+jusM
 Uh+jh+44w9DZkDNjxRAA5CxPlmBIn1OOYt1tsphrHg1cH1fDLK/pDjsJZkiH8EIjhckOtGSb
 aoUUMMJ85nVhN1EbU/A3DkWCVFEA//Vu1+BckbSbJKE7Hl6WdW19BXOZ7v3jo1q6lWwcFYth
 esJfk3ZPPJXuBokrFH8kqnEQ9W2QgrjDX3et2WwZFLOoOCItWxT0/1QO4ikcef/E7HXQf/ij
 Dxf9HG2o5hOlMIAkJq/uLNMvABEBAAGJAjwEGAEIACYWIQQL720fVjJTAhDNQh44pRV8jEPI
 EQUCXNX0cAIbDAUJEswDAAAKCRA4pRV8jEPIEWkzEACKFUnpp+wIVHpckMfBqN8BE5dUbWJc
 GyQ7wXWajLtlPdw1nNw0Wrv+ob2RCT7qQlUo6GRLcvj9Fn5tR4hBvR6D3m8aR0AGHbcC62cq
 I7LjaSDP5j/em4oVL2SMgNTrXgE2w33JMGjAx9oBzkxmKUqprhJomPwmfDHMJ0t7y39Da724
 oLPTkQDpJL1kuraM9TC5NyLe1+MyIxqM/8NujoJbWeQUgGjn9uxQAil7o/xSCjrWCP3kZDID
 vd5ZaHpdl8e1mTExQoKr4EWgaMjmD/a3hZ/j3KfTVNpM2cLfD/QwTMaC2fkK8ExMsz+rUl1H
 icmcmpptCwOSgwSpPY1Zfio6HvEJp7gmDwMgozMfwQuT9oxyFTxn1X3rn1IoYQF3P8gsziY5
 qtTxy2RrgqQFm/hr8gM78RhP54UPltIE96VywviFzDZehMvuwzW//fxysIoK97Y/KBZZOQs+
 /T+Bw80Pwk/dqQ8UmIt2ffHEgwCTbkSm711BejapWCfklxkMZDp16mkxSt2qZovboVjXnfuq
 wQ1QL4o4t1hviM7LyoflsCLnQFJh6RSBhBpKQinMJl/z0A6NYDkQi6vEGMDBWX/M2vk9Jvwa
 v0cEBfY3Z5oFgkh7BUORsu1V+Hn0fR/Lqq/Pyq+nTR26WzGDkolLsDr3IH0TiAVH5ZuPxyz6
 abzjfg==
Message-ID: <2d7174b1-115f-b86f-8054-a5caef4b69ff@schaufler-ca.com>
Date:   Thu, 2 Apr 2020 08:38:51 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200402141319.28714-1-vdronov@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Content-Length: 4751
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@redhat.com>
> ---
> 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: {