From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755528AbeEaP0h (ORCPT ); Thu, 31 May 2018 11:26:37 -0400 Received: from sonic301-27.consmr.mail.gq1.yahoo.com ([98.137.64.153]:38277 "EHLO sonic301-27.consmr.mail.gq1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755478AbeEaP0b (ORCPT ); Thu, 31 May 2018 11:26:31 -0400 X-YMail-OSG: f_xppgwVM1m3ZuLXkLIngZRjWGIXHpL4PJqa4mabZbHb4m_Way6h6koGQp2zo0Z cK5AVzoaEbusj0STy.cT_OQlUf5ZyzVoKjSNGMa1qnVB.8zcu0GQtezVqAt8DqMgmY466Cuj5FI1 6Y_jH1hN9G1cG1OTAddnATLLcO7ZEgvCIwiVqHPogVJvm5I5vUTY_ytepXShwLumrM73R3nIUivw iBplVCgWzPr6mWbPZCi7FGBjtPU1OgaGU5.YkqH47WzlCF6AdSeyOmHd4weeG7E1EYnuND0lDo5d LSzNhKpWVLaukJJUETa17J_qKozePYvKzmMGnsfT0OBeidNSAyalwZ7ZgRMruTvKiJsfFhCTMA9C 165DJSkkq159z7hZC7e1zhf8pby1LcdC4G2yS6Ya84pLxhIlMBvvSkiBjtrONv6GM8jEGm2_2LqQ cADgc.WmMEBxJJkVnJmaq1rt_JQ4dDZKn5r21Qb3ypb7wTxpodrI39l.E2dF_.Dr1O79fuhXyiZw rKzUVT5zmfluWCnh8p.DmNfYqZlRaQTY7CbAviXZc9OafkQje3GoC3kvbL9yJmLUSySlr_LVtsO9 0AG1ABu59n02NzK1AARw5OPqlso6kycxKlFM5IKy5jaR.vglsDv26fBCHoSjJm8DQ76EwwhXzYZb inCW6XYFsQ40PDQnlvTax4zoai.7kTbXGJuLUdedPWV2zR_TVGBYofJSephXnZBRwwTHt18lhEPf VSSfZe26NwmjNAmrZs94mXSXtNcutg1nr0jl_tVh8 Subject: Re: [PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set To: CHANDAN VN , gregkh@linuxfoundation.org, tj@kernel.org, bfields@fieldses.org, jlayton@kernel.org, linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org Cc: cpgs@samsung.com, sireesha.t@samsung.com, Casey Schaufler References: <1527758911-18610-1-git-send-email-chandan.vn@samsung.com> From: Casey Schaufler Message-ID: Date: Thu, 31 May 2018 08:26:23 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <1527758911-18610-1-git-send-email-chandan.vn@samsung.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/31/2018 2:28 AM, CHANDAN VN wrote: > From: "sireesha.t" > > Leak is caused because smack_inode_getsecurity() is allocating memory > using kstrdup(). Though the security_release_secctx() is called, it > would not free the allocated memory. Calling security_release_secctx is > not relevant for this scenario as inode_getsecurity() does not provide a > "secctx". > > Similar fix has been mainlined: > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?id=57e7ba04d422c3d41c8426380303ec9b7533ded9 > > The fix is to replace the security_release_secctx() with a kfree() > > Below is the KMEMLEAK dump: > unreferenced object 0xffffffc025e11c80 (size 64): > comm "systemd-tmpfile", pid 2452, jiffies 4294894464 (age 235587.492s) > hex dump (first 32 bytes): > 53 79 73 74 65 6d 3a 3a 53 68 61 72 65 64 00 00 System::Shared.. > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [] __save_stack_trace+0x28/0x34 > [] create_object+0x130/0x25c > [] kmemleak_alloc+0x30/0x5c > [] __kmalloc_track_caller+0x1cc/0x2a8 > [] kstrdup+0x3c/0x6c > [] smack_inode_getsecurity+0xcc/0xec > [] smack_inode_getsecctx+0x24/0x44 > [] security_inode_getsecctx+0x50/0x70 > [] kernfs_security_xattr_set+0x74/0xe0 > [] __vfs_setxattr+0x74/0x90 > [] __vfs_setxattr_noperm+0x80/0x1ac > [] vfs_setxattr+0x84/0xac > [] setxattr+0x114/0x178 > [] path_setxattr+0x74/0xb8 > [] SyS_lsetxattr+0x10/0x1c > [] __sys_trace_return+0x0/0x4 > > Signed-off-by: sireesha.t > Signed-off-by: CHANDAN VN Why not: static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) { - int len = 0; - len = smack_inode_getsecurity(inode, XATTR_SMACK_SUFFIX, ctx, true); + int len = smack_inode_getsecurity(inode, XATTR_SMACK_SUFFIX, ctx, false); if (len < 0) return len; > --- > fs/kernfs/inode.c | 3 ++- > fs/nfsd/nfs4xdr.c | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c > index a343039..53befb8 100644 > --- a/fs/kernfs/inode.c > +++ b/fs/kernfs/inode.c > @@ -369,7 +369,8 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler, > mutex_unlock(&kernfs_mutex); > > if (secdata) > - security_release_secctx(secdata, secdata_len); > + kfree(secdata); > + > return error; > } > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index aaa88c1..1e0dbe9 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -2911,7 +2911,7 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat) > out: > #ifdef CONFIG_NFSD_V4_SECURITY_LABEL > if (context) > - security_release_secctx(context, contextlen); > + kfree(context); > #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */ > kfree(acl); > if (tempfh) {