linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Casey Schaufler <casey@schaufler-ca.com>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	Kees Cook <keescook@chromium.org>
Cc: LSM <linux-security-module@vger.kernel.org>,
	"James Morris" <jmorris@namei.org>,
	"SE Linux" <selinux@tycho.nsa.gov>,
	LKLM <linux-kernel@vger.kernel.org>,
	"John Johansen" <john.johansen@canonical.com>,
	"Paul Moore" <paul@paul-moore.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"Alexey Dobriyan" <adobriyan@gmail.com>,
	"Mickaël Salaün" <mic@digikod.net>,
	"Salvatore Mesoraca" <s.mesoraca16@gmail.com>
Subject: Re: [PATCH v4 00/19] LSM: Module stacking for SARA and Landlock
Date: Tue, 25 Sep 2018 02:22:28 +0900	[thread overview]
Message-ID: <f0059ebc-3586-dd08-efe5-f9ea69747218@i-love.sakura.ne.jp> (raw)
In-Reply-To: <2a1ceb8c-0288-47ff-a763-d620e904b5b2@schaufler-ca.com>

On 2018/09/25 1:15, Casey Schaufler wrote:
>>>>    Since all free hooks are called when one of init hooks failed, each
>>>>    free hook needs to check whether init hook was called. An example is
>>>>    inode_free_security() in security/selinux/hooks.c (but not addressed in
>>>>    this patch).
>>>
>>> I *think* that selinux_inode_free_security() is safe in this
>>> case because the blob will be zeroed, hence isec->list will
>>> be NULL.
>>
>> That's not safe - look more closely at what list_empty_careful() tests, and then think about what happens when list_del_init() gets called on that isec->list.  selinux_inode_free_security() presumes that selinux_inode_alloc_security() has been called already.  If you are breaking that assumption, you have to fix it.
> 
> Yup. I misread the macro my first time around. Easy fix.

Oh, I didn't notice that it is doing !list_empty_careful() than list_empty_careful().
Unsafe indeed. But easy to fix.

> 
>> Is there a reason you can't make inode_alloc_security() return void since you moved the allocation to the framework? 
> 
> No reason with any of the existing modules, But I could see someone
> doing unnatural things during allocation that might result in a
> failure.

Currently upstreamed LSM modules and AKARI would be OK. But I can't guarantee it
for future / not-yet-upstreamed LSM modules.

  reply	other threads:[~2018-09-24 23:25 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-21 23:59 [PATCH v4 00/19] LSM: Module stacking for SARA and Landlock Casey Schaufler
2018-09-22  0:16 ` [PATCH v4 01/19] procfs: add smack subdir to attrs Casey Schaufler
2018-09-22  0:17 ` [PATCH v4 02/19] Smack: Abstract use of cred security blob Casey Schaufler
2018-09-22  2:44   ` Kees Cook
2018-09-22  0:17 ` [PATCH v4 03/19] SELinux: " Casey Schaufler
2018-09-22  0:17 ` [PATCH v4 04/19] SELinux: Remove cred security blob poisoning Casey Schaufler
2018-09-22  2:43   ` Kees Cook
2018-09-27 22:13   ` James Morris
2018-09-27 22:32     ` Casey Schaufler
2018-09-22  0:17 ` [PATCH v4 05/19] SELinux: Remove unused selinux_is_enabled Casey Schaufler
2018-09-22  2:43   ` Kees Cook
2018-09-22  0:17 ` [PATCH v4 06/19] AppArmor: Abstract use of cred security blob Casey Schaufler
2018-09-22  2:46   ` Kees Cook
2018-09-22  0:18 ` [PATCH v4 07/19] TOMOYO: " Casey Schaufler
2018-09-22  2:47   ` Kees Cook
2018-09-22  0:18 ` [PATCH v4 08/19] Infrastructure management of the " Casey Schaufler
2018-09-22  2:50   ` Kees Cook
2018-09-22  0:18 ` [PATCH v4 09/19] SELinux: Abstract use of file " Casey Schaufler
2018-09-22  0:18 ` [PATCH v4 10/19] Smack: " Casey Schaufler
2018-09-22  2:51   ` Kees Cook
2018-09-22  0:19 ` [PATCH v4 11/19] LSM: Infrastructure management of the file security Casey Schaufler
2018-09-22  2:53   ` Kees Cook
2018-09-22  0:19 ` [PATCH v4 12/19] SELinux: Abstract use of inode security blob Casey Schaufler
2018-09-22  0:19 ` [PATCH v4 13/19] Smack: " Casey Schaufler
2018-09-22  0:19 ` [PATCH v4 14/19] LSM: Infrastructure management of the inode security Casey Schaufler
2018-09-22  2:55   ` Kees Cook
2018-10-03 18:13     ` James Morris
2018-10-04  4:49       ` Casey Schaufler
2018-09-22  0:19 ` [PATCH v4 15/19] LSM: Infrastructure management of the task security Casey Schaufler
2018-09-22  2:56   ` Kees Cook
2018-09-22  0:19 ` [PATCH v4 16/19] SELinux: Abstract use of ipc security blobs Casey Schaufler
2018-09-22  2:56   ` Kees Cook
2018-09-22  0:19 ` [PATCH v4 17/19] Smack: " Casey Schaufler
2018-09-22  2:57   ` Kees Cook
2018-09-22  0:20 ` [PATCH v4 18/19] LSM: Infrastructure management of the ipc security blob Casey Schaufler
2018-09-22  2:58   ` Kees Cook
2018-09-22  0:20 ` [PATCH v4 19/19] LSM: Blob sharing support for S.A.R.A and LandLock Casey Schaufler
2018-09-22  0:22 ` [PATCH v4 09/19] SELinux: Abstract use of file security blob Casey Schaufler
2018-09-22  3:02 ` [PATCH v4 00/19] LSM: Module stacking for SARA and Landlock Kees Cook
2018-09-22 16:38   ` Casey Schaufler
2018-09-23  2:43     ` Kees Cook
2018-09-23 15:59       ` Tetsuo Handa
2018-09-23 17:09         ` Casey Schaufler
2018-09-24  1:53           ` Tetsuo Handa
2018-09-24 17:16             ` Casey Schaufler
2018-09-24 17:53               ` Tetsuo Handa
2018-09-24 20:33                 ` Casey Schaufler
2018-09-24 15:01           ` Stephen Smalley
2018-09-24 16:15             ` Casey Schaufler
2018-09-24 17:22               ` Tetsuo Handa [this message]
2018-10-01 17:58           ` James Morris
2018-09-26 21:57 ` [PATCH v4 20/19] LSM: Correct file blob free empty blob check Casey Schaufler
2018-10-01 20:29   ` Kees Cook
2018-09-26 21:57 ` [PATCH 21/19] LSM: Cleanup and fixes from Tetsuo Handa Casey Schaufler
2018-10-01 21:48   ` Kees Cook
2018-10-12 20:07     ` Kees Cook

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=f0059ebc-3586-dd08-efe5-f9ea69747218@i-love.sakura.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=adobriyan@gmail.com \
    --cc=casey@schaufler-ca.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mic@digikod.net \
    --cc=paul@paul-moore.com \
    --cc=s.mesoraca16@gmail.com \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@tycho.nsa.gov \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).