Commit 57e7ba04 authored by Casey Schaufler's avatar Casey Schaufler Committed by James Morris

lsm: fix smack_inode_removexattr and xattr_getsecurity memleak

security_inode_getsecurity() provides the text string value
of a security attribute. It does not provide a "secctx".
The code in xattr_getsecurity() that calls security_inode_getsecurity()
and then calls security_release_secctx() happened to work because
SElinux and Smack treat the attribute and the secctx the same way.
It fails for cap_inode_getsecurity(), because that module has no
secctx that ever needs releasing. It turns out that Smack is the
one that's doing things wrong by not allocating memory when instructed
to do so by the "alloc" parameter.

The fix is simple enough. Change the security_release_secctx() to
kfree() because it isn't a secctx being returned by
security_inode_getsecurity(). Change Smack to allocate the string when
told to do so.

Note: this also fixes memory leaks for LSMs which implement
inode_getsecurity but not release_secctx, such as capabilities.
Signed-off-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
Reported-by: default avatarKonstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
parent d81fa669
...@@ -250,7 +250,7 @@ xattr_getsecurity(struct inode *inode, const char *name, void *value, ...@@ -250,7 +250,7 @@ xattr_getsecurity(struct inode *inode, const char *name, void *value,
} }
memcpy(value, buffer, len); memcpy(value, buffer, len);
out: out:
security_release_secctx(buffer, len); kfree(buffer);
out_noalloc: out_noalloc:
return len; return len;
} }
......
...@@ -1473,7 +1473,7 @@ static int smack_inode_removexattr(struct dentry *dentry, const char *name) ...@@ -1473,7 +1473,7 @@ static int smack_inode_removexattr(struct dentry *dentry, const char *name)
* @inode: the object * @inode: the object
* @name: attribute name * @name: attribute name
* @buffer: where to put the result * @buffer: where to put the result
* @alloc: unused * @alloc: duplicate memory
* *
* Returns the size of the attribute or an error code * Returns the size of the attribute or an error code
*/ */
...@@ -1486,16 +1486,10 @@ static int smack_inode_getsecurity(struct inode *inode, ...@@ -1486,16 +1486,10 @@ static int smack_inode_getsecurity(struct inode *inode,
struct super_block *sbp; struct super_block *sbp;
struct inode *ip = (struct inode *)inode; struct inode *ip = (struct inode *)inode;
struct smack_known *isp; struct smack_known *isp;
int ilen;
int rc = 0;
if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) { if (strcmp(name, XATTR_SMACK_SUFFIX) == 0)
isp = smk_of_inode(inode); isp = smk_of_inode(inode);
ilen = strlen(isp->smk_known); else {
*buffer = isp->smk_known;
return ilen;
}
/* /*
* The rest of the Smack xattrs are only on sockets. * The rest of the Smack xattrs are only on sockets.
*/ */
...@@ -1515,14 +1509,15 @@ static int smack_inode_getsecurity(struct inode *inode, ...@@ -1515,14 +1509,15 @@ static int smack_inode_getsecurity(struct inode *inode,
isp = ssp->smk_out; isp = ssp->smk_out;
else else
return -EOPNOTSUPP; return -EOPNOTSUPP;
}
ilen = strlen(isp->smk_known); if (alloc) {
if (rc == 0) { *buffer = kstrdup(isp->smk_known, GFP_KERNEL);
*buffer = isp->smk_known; if (*buffer == NULL)
rc = ilen; return -ENOMEM;
} }
return rc; return strlen(isp->smk_known);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment