All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sargun Dhillon <sargun@sargun.me>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Paul Moore <paul@paul-moore.com>
Cc: LSM <linux-security-module@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Casey Schaufler <casey@schaufler-ca.com>,
	James Morris <jmorris@namei.org>,
	Peter Dolding <oiaohm@gmail.com>,
	Igor Stoppa <igor.stoppa@huawei.com>
Subject: Re: [PATCH v5 1/1] security: Add mechanism to safely (un)load LSMs after boot time
Date: Tue, 10 Apr 2018 14:24:02 -0700	[thread overview]
Message-ID: <CAMp4zn9uc-Z4NOn_0YQ-8qgdDdJxdNA56GCWGhjEbYQBzp_pOg@mail.gmail.com> (raw)
In-Reply-To: <201804090525.w395P1qS044316@www262.sakura.ne.jp>

On Sun, Apr 8, 2018 at 10:25 PM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> Sargun Dhillon wrote:
>> >   Remove SECURITY_HOOK_COUNT and "struct security_hook_list"->owner and
>> >   the exception in randomize_layout_plugin.c because preventing module
>> >   unloading won't work as expected.
>> >
>>
>> Rather than completely removing the unloading code, might it make
>> sense to add a BUG_ON or WARN_ON, in security_delete_hooks if
>> allow_unload_module is false, and owner is not NULL?
>
> Do we need to check ->owner != NULL? Although it will be true that
> SELinux's ->owner == NULL and LKM-based LSM module's ->owner != NULL,
> I think we unregister SELinux before setting allow_unload_module to false.
> Thus, rejecting delete_security_hooks() if allow_unload_module == false will
> be sufficient. SELinux might want to call panic() if delete_security_hooks()
> did not unregister due to allow_unload_module == false. Also,
> allow_unload_module would be renamed to allow_unregister_module.
>
> By the way, please don't use BUG_ON() or WARN_ON() because syzbot would hit
> and call panic() because syzbot runs tests with panic_on_warn == true.

I think my primary question is for the SELinux folks -- what do you
think the behaviour should be? If allow_unload_modules /
allow_unregister_module is set, do you want to be able to call
security_delete_hooks? What do you think the right
action should be if it fails?

WARNING: multiple messages have this Message-ID (diff)
From: sargun@sargun.me (Sargun Dhillon)
To: linux-security-module@vger.kernel.org
Subject: [PATCH v5 1/1] security: Add mechanism to safely (un)load LSMs after boot time
Date: Tue, 10 Apr 2018 14:24:02 -0700	[thread overview]
Message-ID: <CAMp4zn9uc-Z4NOn_0YQ-8qgdDdJxdNA56GCWGhjEbYQBzp_pOg@mail.gmail.com> (raw)
In-Reply-To: <201804090525.w395P1qS044316@www262.sakura.ne.jp>

On Sun, Apr 8, 2018 at 10:25 PM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> Sargun Dhillon wrote:
>> >   Remove SECURITY_HOOK_COUNT and "struct security_hook_list"->owner and
>> >   the exception in randomize_layout_plugin.c because preventing module
>> >   unloading won't work as expected.
>> >
>>
>> Rather than completely removing the unloading code, might it make
>> sense to add a BUG_ON or WARN_ON, in security_delete_hooks if
>> allow_unload_module is false, and owner is not NULL?
>
> Do we need to check ->owner != NULL? Although it will be true that
> SELinux's ->owner == NULL and LKM-based LSM module's ->owner != NULL,
> I think we unregister SELinux before setting allow_unload_module to false.
> Thus, rejecting delete_security_hooks() if allow_unload_module == false will
> be sufficient. SELinux might want to call panic() if delete_security_hooks()
> did not unregister due to allow_unload_module == false. Also,
> allow_unload_module would be renamed to allow_unregister_module.
>
> By the way, please don't use BUG_ON() or WARN_ON() because syzbot would hit
> and call panic() because syzbot runs tests with panic_on_warn == true.

I think my primary question is for the SELinux folks -- what do you
think the behaviour should be? If allow_unload_modules /
allow_unregister_module is set, do you want to be able to call
security_delete_hooks? What do you think the right
action should be if it fails?
--
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-04-10 21:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-08  6:59 [PATCH v5 1/1] security: Add mechanism to safely (un)load LSMs after boot time Sargun Dhillon
2018-04-08  6:59 ` Sargun Dhillon
2018-04-09  3:38 ` Tetsuo Handa
2018-04-09  3:38   ` Tetsuo Handa
2018-04-09  4:21   ` Sargun Dhillon
2018-04-09  4:21     ` Sargun Dhillon
2018-04-09  5:25     ` Tetsuo Handa
2018-04-09  5:25       ` Tetsuo Handa
2018-04-10 21:24       ` Sargun Dhillon [this message]
2018-04-10 21:24         ` Sargun Dhillon
2018-04-11 14:17         ` Stephen Smalley
2018-04-11 14:17           ` Stephen Smalley
2018-04-11 21:36           ` Paul Moore
2018-04-11 21:36             ` Paul Moore

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=CAMp4zn9uc-Z4NOn_0YQ-8qgdDdJxdNA56GCWGhjEbYQBzp_pOg@mail.gmail.com \
    --to=sargun@sargun.me \
    --cc=casey@schaufler-ca.com \
    --cc=igor.stoppa@huawei.com \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=oiaohm@gmail.com \
    --cc=paul@paul-moore.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    /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.