From: Chris Mason security_inode_copy_up() leaks a struct cred when one LSM allocates new creds for an overlayfs copy up and a later LSM then denies the copy up. One such pair is SELinux and a BPF LSM program that denies inode_copy_up. Enabling both LSMs and loading the program requires root. After that, each denied copy up leaks one struct cred, no matter which user triggers it. The hook calls each enabled LSM in turn. SELinux or Smack allocates the creds and returns them through a pointer argument of the hook. A later LSM can still return an error, and the hook then returns that error with the creds still allocated. Its kernel-doc does not say that the caller owns them on error, and the only caller, overlayfs, does not put them. Reproduced with SELinux and such a BPF LSM program: 36000 failed opens and chmods by an unprivileged user on an overlayfs mount left 36000 struct cred objects allocated, which kmemleak reported. Fix this by putting the creds in security_inode_copy_up() when the hook fails, so the caller gets either creds that it owns or none. Fixes: d8ad8b496184 ("security, overlayfs: provide copy up security hook for unioned files") Cc: stable@vger.kernel.org Signed-off-by: Chris Mason [amir: put the creds in security_inode_copy_up() instead of in overlayfs] Signed-off-by: Amir Goldstein --- Hi Paul, I think this LSM hook was introduced with confusing contact for chained LSMs and this change makes the hook contract more sane in this case. Thanks, Amir. security/security.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/security/security.c b/security/security.c index f6583d5073959..ad382afbff271 100644 --- a/security/security.c +++ b/security/security.c @@ -2314,18 +2314,26 @@ void security_inode_getlsmprop(struct inode *inode, struct lsm_prop *prop) /** * security_inode_copy_up() - Create new creds for an overlayfs copy-up op * @src: union dentry of copy-up file - * @new: newly created creds + * @new: newly created creds, must point to NULL on entry * * A file is about to be copied up from lower layer to upper layer of overlay * filesystem. Security module can prepare a set of new creds and modify as * need be and return new creds. Caller will switch to new creds temporarily to * create new file and release newly allocated creds. * - * Return: Returns 0 on success or a negative error code on error. + * Return: Returns 0 on success or a negative error code on error, in which + * case @new is left pointing to NULL. */ int security_inode_copy_up(struct dentry *src, struct cred **new) { - return call_int_hook(inode_copy_up, src, new); + int rc = call_int_hook(inode_copy_up, src, new); + + /* An earlier LSM may have prepared creds before a later one failed */ + if (unlikely(rc < 0) && *new) { + put_cred(*new); + *new = NULL; + } + return rc; } EXPORT_SYMBOL(security_inode_copy_up); -- 2.55.0