All of lore.kernel.org
 help / color / mirror / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: chandan.vn@samsung.com,
	"linux-security-module@vger.kernel.org"
	<linux-security-module@vger.kernel.org>
Cc: Tejun Heo <tj@kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"bfields@fieldses.org" <bfields@fieldses.org>,
	"jlayton@kernel.org" <jlayton@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	CPGS <cpgs@samsung.com>,
	Sireesha Talluri <sireesha.t@samsung.com>,
	Chris Wright <chrisw@sous-sol.org>,
	Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set
Date: Fri, 1 Jun 2018 09:41:39 -0700	[thread overview]
Message-ID: <5b0b157a-0e8c-d8f5-901e-836d545a8e4c@schaufler-ca.com> (raw)
In-Reply-To: <20180601162913epcms5p7737f5b4376d8865af1eae119aa866550@epcms5p7>

On 6/1/2018 9:29 AM, CHANDAN VN wrote:
>>>  I agree that the fix can be done simply by using "false" for 
>>>  smack_inode_getsecurity(), but what happens with kernfs_node_setsecdata()
>>>  and smack_inode_notifysecctx(). kernfs_node_setsecdata() is probably ignorable
>>>  but smack_inode_notifysecctx() is sending the "ctx" to smack_inode_setsecurity()
>>>  and since "ctx" would be NULL because we used "false", smack_inode_setsecurity()
>>>  becomes dummy.
>  
>> Thank you for pointing this out. You're right, there's more
>> at issue here than changing the alloc flag will fix. I think
>> that calling smack_inode_getsecurity() from smack_inode_getsecctx()
>> is making the code more complicated than it needs to be. I will
>> have a patch shortly.
> If you think the patch would take time or is complicated, I suggest that the kfree() fix should go
> to fix the leaks for now.

Heavens no! The patch is very simple. I'm building a kernel with
it now, and should have it tested and posted within a few hours.
The implementation of smack_inode_getsecctx() that's there is
understandable, but wrong. There's a much better way to do the
job.


WARNING: multiple messages have this Message-ID (diff)
From: casey@schaufler-ca.com (Casey Schaufler)
To: linux-security-module@vger.kernel.org
Subject: [PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set
Date: Fri, 1 Jun 2018 09:41:39 -0700	[thread overview]
Message-ID: <5b0b157a-0e8c-d8f5-901e-836d545a8e4c@schaufler-ca.com> (raw)
In-Reply-To: <20180601162913epcms5p7737f5b4376d8865af1eae119aa866550@epcms5p7>

On 6/1/2018 9:29 AM, CHANDAN VN wrote:
>>> ?I?agree?that?the?fix?can?be?done?simply?by?using?"false"?for?
>>> ?smack_inode_getsecurity(),?but?what?happens?with?kernfs_node_setsecdata()
>>> ?and?smack_inode_notifysecctx().?kernfs_node_setsecdata()?is?probably?ignorable
>>> ?but?smack_inode_notifysecctx()?is?sending?the?"ctx"?to?smack_inode_setsecurity()
>>> ?and?since?"ctx"?would?be?NULL?because?we?used?"false",?smack_inode_setsecurity()
>>> ?becomes?dummy.
> ?
>> Thank?you?for?pointing?this?out.?You're?right,?there's?more
>> at?issue?here?than?changing?the?alloc?flag?will?fix.?I?think
>> that?calling?smack_inode_getsecurity()?from?smack_inode_getsecctx()
>> is?making?the?code?more?complicated?than?it?needs?to?be.?I?will
>> have?a?patch?shortly.
> If you think the patch would take time or is complicated, I suggest that the kfree() fix should go
> to fix the leaks for now.

Heavens no! The patch is very simple. I'm building a kernel with
it now, and should have it tested and posted within a few hours.
The implementation of smack_inode_getsecctx() that's there is
understandable, but wrong. There's a much better way to do the
job.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2018-06-01 16:41 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20180531092848epcas1p24b638ccd6da00f1e039bdb64de7e1a5b@epcas1p2.samsung.com>
2018-05-31  9:28 ` [PATCH 1/1] Fix memory leak in kernfs_security_xattr_set and kernfs_security_xattr_set CHANDAN VN
2018-05-31 15:26   ` Casey Schaufler
2018-05-31 20:57     ` Eric W. Biederman
2018-05-31 21:08       ` Casey Schaufler
2018-05-31 15:39   ` Tejun Heo
2018-05-31 15:39     ` Tejun Heo
2018-05-31 16:04     ` Casey Schaufler
2018-05-31 16:04       ` Casey Schaufler
2018-05-31 16:11       ` Tejun Heo
2018-05-31 16:11         ` Tejun Heo
2018-05-31 16:22         ` Casey Schaufler
2018-05-31 16:22           ` Casey Schaufler
     [not found]         ` <CGME20180531092848epcas1p24b638ccd6da00f1e039bdb64de7e1a5b@epcms5p5>
2018-06-01  8:56           ` CHANDAN VN
2018-06-01  8:56             ` CHANDAN VN
2018-06-01  8:56             ` CHANDAN VN
2018-06-01 16:22             ` Casey Schaufler
2018-06-01 16:22               ` Casey Schaufler
     [not found]             ` <CGME20180531092848epcas1p24b638ccd6da00f1e039bdb64de7e1a5b@epcms5p7>
2018-06-01 16:29               ` CHANDAN VN
2018-06-01 16:29                 ` CHANDAN VN
2018-06-01 16:29                 ` CHANDAN VN
2018-06-01 16:41                 ` Casey Schaufler [this message]
2018-06-01 16:41                   ` Casey Schaufler
2018-06-01 17:45                   ` [PATCH] Smack: Fix memory leak in smack_inode_getsecctx Casey Schaufler
2018-06-01 17:45                     ` Casey Schaufler
2018-06-04 21:01                     ` Casey Schaufler
2018-06-04 21:01                       ` Casey Schaufler
2018-06-04 21:27                       ` Tejun Heo
2018-06-04 21:27                         ` Tejun Heo
     [not found]                       ` <CGME20180531092848epcas1p24b638ccd6da00f1e039bdb64de7e1a5b@epcms5p3>
2018-06-05  7:04                         ` CHANDAN VN
2018-06-05  7:04                           ` CHANDAN VN
2018-06-05  7:04                           ` CHANDAN VN
2018-06-05 14:29                           ` Casey Schaufler
     [not found]                           ` <CGME20180531092848epcas1p24b638ccd6da00f1e039bdb64de7e1a5b@epcms5p4>
2018-06-05 14:46                             ` CHANDAN VN

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5b0b157a-0e8c-d8f5-901e-836d545a8e4c@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=bfields@fieldses.org \
    --cc=chandan.vn@samsung.com \
    --cc=chrisw@sous-sol.org \
    --cc=cpgs@samsung.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=sireesha.t@samsung.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.