From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751227AbdISEWm (ORCPT ); Tue, 19 Sep 2017 00:22:42 -0400 Received: from nm10-vm5.bullet.mail.ne1.yahoo.com ([98.138.91.232]:50725 "EHLO nm10-vm5.bullet.mail.ne1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750758AbdISEWk (ORCPT ); Tue, 19 Sep 2017 00:22:40 -0400 X-Yahoo-Newman-Id: 69693.70573.bm@smtp203.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: RZ_nPf0VM1msSaXaJXVkOpSct1S9Fp7yLPFN3zWzB0CwDnV dQzSrATHLjzecbX6bZyDlZ4Slxyud61NWk1V_x42P1oTdkgqV6UeaV8ZrD0F VLic1ViIfEY7xjh33JW_tliubllnrQhC7N7vm8BgrSjbrHFFGQcMltWaL5km tRK3x_.VkA8JV_5.M5_x5rdEFnplB1i6261RA6dIfITWVhD2dQSb56r7zHM_ bFrVWofXLLw8Dn8OG.tg6fS3G.I39AFtnyW3.GBtGgAH53Yk9fo9_3SjAEIH w_ZrFyqdll4uFBr0930cuoMCUHT0Xu628J5cSk8iKyzKQLKvfRrkMNq9Tl9k t_Sgmle8mgmsQrrD49H.KQ81oLyu7plISKCuUAKxRPP7sBx.ckDGUI.z6o0h 4x.ZpVe_iXNAF_Pl5THuSa.OIqA8f80jmc0p79jSp_cZ62pJVI24Wm3bzJ7C dZ8VIyjlZofzUyvXTUWgS3B1mCgrrKV5R8fA98aFlUqfeIT20eo3VREV4yhQ csd3fzPc17Q-- X-Yahoo-SMTP: OIJXglSswBDfgLtXluJ6wiAYv6_cnw-- Subject: Re: [BUG] 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: <3e8b84d3-6d29-2e91-7449-69c0685ef7be@schaufler-ca.com> Date: Mon, 18 Sep 2017 19:21:40 -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-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 9/18/2017 1:25 PM, Casey Schaufler wrote: > On 9/16/2017 11:18 AM, Konstantin Khlebnikov wrote: >> I've got this kmemleak splat Do you have a convenient test case? I'd like to verify my patch with your case if it is reasonable to do so. >> >> unreferenced object 0xffff880f687ff6a8 (size 32): >>   comm "cp", pid 4279, jiffies 4295784487 (age 2866.296s) >>   hex dump (first 32 bytes): >>     01 00 00 02 02 00 08 00 00 00 00 00 00 00 00 00  ................ >>     00 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5  .....kkkkkkkkkk. >>   backtrace: >>     [] kmemleak_alloc+0x4a/0xa0 >>     [] __kmalloc_track_caller+0x171/0x280 >>     [] krealloc+0xa7/0xc0 >>     [] vfs_getxattr_alloc+0xbd/0x110 >>     [] cap_inode_getsecurity+0x79/0x200 >>     [] security_inode_getsecurity+0x52/0x80 >>     [] xattr_getsecurity+0x3a/0xa0 >>     [] vfs_getxattr+0x74/0xa0 >>     [] getxattr+0x9e/0x170 >>     [] SyS_fgetxattr+0x5a/0x90 >>     [] entry_SYSCALL_64_fastpath+0x1e/0xae >>     [] 0xffffffffffffffff >> >> allocated in  xattr_getsecurity() security_inode_getsecurity() -> cap_inode_getsecurity() >> should be freed by security_release_secctx() but there is no release_secctx hook. > There is a bug here, but the fix suggested is not correct. > security_inode_getsecurity() provides the security attribute > with a specified name. In the past there would be exactly one > use for this call, which was to return the value of the > attribute that happened to match the security "context" of > the inode. Freeing the value with security_release_secctx() > works coincidentally. Because security_inode_getsecurity() is > called with the "true" value for the alloc parameter the correct > fix is: > > - fix smack_inode_getsecurity() to honor the alloc flag > - change the security_release_secctx() to kfree here. > > I can provide a patch in a day or so. > >> I don't know details about modern security models stack but it >> seems security_release_secctx() have to know which layer owns >> secdata to free it property: selinux just calls kfree, >> capability_hooks should do the same, but other modules returns >> non-freeable pointers. Currently security_release_secctx() calls >> release_secctx for all models, this is obviously wrong. >> > > . >