From patchwork Tue Sep 19 16:39:08 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Casey Schaufler X-Patchwork-Id: 9959597 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 76D8960568 for ; Tue, 19 Sep 2017 16:39:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6564E28E8B for ; Tue, 19 Sep 2017 16:39:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5A27E28E8D; Tue, 19 Sep 2017 16:39:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F083F28E8B for ; Tue, 19 Sep 2017 16:39:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751476AbdISQjN (ORCPT ); Tue, 19 Sep 2017 12:39:13 -0400 Received: from nm2.bullet.mail.ne1.yahoo.com ([98.138.90.65]:54643 "EHLO nm2.bullet.mail.ne1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750973AbdISQjN (ORCPT ); Tue, 19 Sep 2017 12:39:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1505839152; bh=gGnU1rRPzoPJ55hPUMnaRmW7B1Qz42pqRQLlDGIOPKw=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From:Subject; b=b48PTlVZoqf4QiyjHbjsKvPeyaMNjjbdgvHLNjMRJWtYoUpkPEoP4T2TCBCOU2bcMu3qPKzIbwlew7CWlHEmuUgYAti+TJAMhIaWsujB4MfOekhx/Soeegm1khzyNOkl2hvNvrXkVSkl1d2Xfg9qfiH4LKJvorVHta2TU3dPVK+QZM3iVuQlCc9QOAnVIcYpYPU9CUGkCFe3E6okPJVEYnlXEHTnfUdrqh3Hu3sjS0yzyNigTTCxrjoF220m5sm32Cv4JxnnwuJL4kUbb7MviEI8gTX+zkzRZ0hXiaswwRBM3mIjl1OGVwogr4x5euYhH8ioStqnhVLGMqqeJhGtww== Received: from [98.138.100.103] by nm2.bullet.mail.ne1.yahoo.com with NNFMP; 19 Sep 2017 16:39:12 -0000 Received: from [98.138.84.41] by tm102.bullet.mail.ne1.yahoo.com with NNFMP; 19 Sep 2017 16:39:12 -0000 Received: from [127.0.0.1] by smtp109.mail.ne1.yahoo.com with NNFMP; 19 Sep 2017 16:39:12 -0000 X-Yahoo-Newman-Id: 328468.98131.bm@smtp109.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: 0eZ9rKwVM1m53X_syVjapSswd12oFduCkPeUS8DKFtF_q1s fjo.ZC75DGurykeZ7apGdmXEFYHm3W1t1MO35U8v2B.TwizDUBBQ0hi03e4C DPm9Dw5qYcCcaSt2hH0x2n.0ZViip5_GeDLO7hfDo3uS0wDkXLIi0SJYjMJ0 zWVHlSYZhuOrlBFu7DO2fMMdwHrQvGpl6q1UQ9Di.m3biXnU8sQ12UzBLdut i94DF2hKRwe2ZEp8DZccyROfmda3bjrYS3vi4ymDudOeZXLhCQ2yBeR_Qij7 zVOjaJd4zBeKNEQj6_NTOeV7bGRL3LE42eQoLPw1dHDPCwxk3gDit8nzV8qM evp1kTAZnExgqlrvggbe_BIn5SXBtEi3v8OZUlOTxNA8QkuEcWE1z_BKI2VZ pw.uF1ySVENgQJC87ixDQ9_PES3R56d.gsBfnAf7a6pb0F4bcENS3hqD9DM6 01hK9ut2QDVzGGzvmen2RhN9ni3wXzUiBuA0KsE6YG7bLTLHhqa77Tq89A.h lcr4poDn2xRx4ivrOHirnhTdltbo- X-Yahoo-SMTP: OIJXglSswBDfgLtXluJ6wiAYv6_cnw-- Subject: [PATCH] fix security_release_secctx seems broken To: Konstantin Khlebnikov , linux-kernel Cc: Serge Hallyn , James Morris , LSM List , Casey Schaufler References: From: Casey Schaufler Message-ID: Date: Tue, 19 Sep 2017 09:39:08 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP Subject: [PATCH] fix security_release_secctx seems broken 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. Signed-off-by: Casey Schaufler Reviewed-by: Konstantin Khlebnikov --- fs/xattr.c | 2 +- security/smack/smack_lsm.c | 55 +++++++++++++++++++++------------------------- 2 files changed, 26 insertions(+), 31 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/xattr.c b/fs/xattr.c index 4424f7fecf14..61cd28ba25f3 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -250,7 +250,7 @@ xattr_getsecurity(struct inode *inode, const char *name, void *value, } memcpy(value, buffer, len); out: - security_release_secctx(buffer, len); + kfree(buffer); out_noalloc: return len; } diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 319add31b4a4..286171a16ed2 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1473,7 +1473,7 @@ static int smack_inode_removexattr(struct dentry *dentry, const char *name) * @inode: the object * @name: attribute name * @buffer: where to put the result - * @alloc: unused + * @alloc: duplicate memory * * Returns the size of the attribute or an error code */ @@ -1486,43 +1486,38 @@ static int smack_inode_getsecurity(struct inode *inode, struct super_block *sbp; struct inode *ip = (struct inode *)inode; 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); - ilen = strlen(isp->smk_known); - *buffer = isp->smk_known; - return ilen; - } + else { + /* + * The rest of the Smack xattrs are only on sockets. + */ + sbp = ip->i_sb; + if (sbp->s_magic != SOCKFS_MAGIC) + return -EOPNOTSUPP; - /* - * The rest of the Smack xattrs are only on sockets. - */ - sbp = ip->i_sb; - if (sbp->s_magic != SOCKFS_MAGIC) - return -EOPNOTSUPP; + sock = SOCKET_I(ip); + if (sock == NULL || sock->sk == NULL) + return -EOPNOTSUPP; - sock = SOCKET_I(ip); - if (sock == NULL || sock->sk == NULL) - return -EOPNOTSUPP; - - ssp = sock->sk->sk_security; + ssp = sock->sk->sk_security; - if (strcmp(name, XATTR_SMACK_IPIN) == 0) - isp = ssp->smk_in; - else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) - isp = ssp->smk_out; - else - return -EOPNOTSUPP; + if (strcmp(name, XATTR_SMACK_IPIN) == 0) + isp = ssp->smk_in; + else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) + isp = ssp->smk_out; + else + return -EOPNOTSUPP; + } - ilen = strlen(isp->smk_known); - if (rc == 0) { - *buffer = isp->smk_known; - rc = ilen; + if (alloc) { + *buffer = kstrdup(isp->smk_known, GFP_KERNEL); + if (*buffer == NULL) + return -ENOMEM; } - return rc; + return strlen(isp->smk_known); }