[no subject]

[Rev. Luke]

Confidential! Open the attached for details


letter.rtf
Description: RTF file

Re: [PATCH v5 04/21] dt-bindings: Add doc for the Ingenic TCU drivers

[Daniel Lezcano]

On 31/07/2018 00:01, Paul Cercueil wrote:

[ ... ]

>>>  +- ingenic,timer-channel: Specifies the TCU channel that should be
>>> used as
>>>  +  system timer. If not provided, the TCU channel 0 is used for the
>>> system timer.
>>>  +
>>>  +- ingenic,clocksource-channel: Specifies the TCU channel that
>>> should be used
>>>  +  as clocksource and sched_clock. It must be a different channel
>>> than the one
>>>  +  used as system timer. If not provided, neither a clocksource nor a
>>>  +  sched_clock is instantiated.
>>
>> clocksource and sched_clock are Linux specific and don't belong in DT.
>> You should define properties of the hardware or use existing properties
>> like interrupts or clocks to figure out which channel to use. For
>> example, if some channels don't have an interrupt, then use them for
>> clocksource and not a clockevent. Or you could have timers that run in
>> low-power modes or not. If all the channels are identical, then it
>> shouldn't matter which ones the OS picks.

It can't work in this case because the pmw and the timer driver are not
communicating and the first one can stole a channel to the last one.


> We already talked about that. All the TCU channels can be used for PWM.
> The problem is I cannot know from the driver's scope which channels will
> be free and which channels will be requested for PWM. You suggested that I
> parse the devicetree for clients, and I did that in the V3/V4 patchset. But
> it only works for clients requesting through devicetree, not from platform
> code or even sysfs.
> 
> One thing I can try is to dynamically change the channels the system timer
> and clocksource are using when the current ones are requested for PWM. But
> that sounds hardcore...

Yes, it is :/

Sorry for letting you wasting time and effort to write an overkill code
not suitable for upstream.

A very gross thought, wouldn't be possible to "register" a channel from
the timer driver code in a shared data area (but well self-encapsulated)
and the pwm code will check such channel isn't in use ?

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

[PATCH] nvmem: fix nvmem_cell_get_from_lookup()

[Bartosz Golaszewski]

From: Bartosz Golaszewski 

We check if the pointer returned by __nvmem_device_get() is not NULL
while we should actually check if it is not IS_ERR(nvmem). Fix it.

While we're at it: fix the next error path where we should assign an
error value to cell before returning.

Signed-off-by: Bartosz Golaszewski 
---
 drivers/nvmem/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ad1227df1984..8249621d11a6 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -952,9 +952,9 @@ nvmem_cell_get_from_lookup(struct device *dev, const char 
*con_id)
(strcmp(lookup->con_id, con_id) == 0)) {
/* This is the right entry. */
nvmem = __nvmem_device_get(NULL, lookup->nvmem_name);
-   if (!nvmem) {
+   if (IS_ERR(nvmem)) {
/* Provider may not be registered yet. */
-   cell = ERR_PTR(-EPROBE_DEFER);
+   cell = (struct nvmem_cell *)nvmem;
goto out;
}
 
@@ -962,6 +962,7 @@ nvmem_cell_get_from_lookup(struct device *dev, const char 
*con_id)
   lookup->cell_name);
if (!cell) {
__nvmem_device_put(nvmem);
+   cell = ERR_PTR(-ENOENT);
goto out;
}
}
-- 
2.18.0

Re: [PATCH v2 0/3] gpio: Fix VLA removal fallout

[Linus Walleij]

On Thu, Sep 27, 2018 at 1:38 PM Geert Uytterhoeven
 wrote:

> This patch series fixes various (mostly harmless) issues introduced by
> commit 3027743f83f867d8 ("gpio: Remove VLA from gpiolib").
>
> As per the "one patch should fix one issue"-policy, this series contains 3
> patches, although they all have the same Fixes: tag.
>
> Changes compared to v1:
>   - Rebase on top of gpio array rework.

Patches applied, thanks Geert!

> W.r.t. propagating errors: while gpiod_set_array_value_complex() and its
> callers can now return an error code, this is currently limited to -ENOMEM.
> Actual failures setting a GPIO output value cannot be propagated, as
> gpio_chip.set() still returns void.  Do you want to change that?
> E.g. gen_74x164_set_value() can fail.

I think we discussed it at one point and said that it's something
we should fix some day, just nobody has got around to do it.

Yours,
Linus Walleij

[PATCH] Documentation: lockstat: Fix trivial typo

[Andrew Murray]

Fix incorrect line number in example output

Signed-off-by: Andrew Murray 
---
 Documentation/locking/lockstat.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/locking/lockstat.txt 
b/Documentation/locking/lockstat.txt
index 5786ad2..fdbeb0c 100644
--- a/Documentation/locking/lockstat.txt
+++ b/Documentation/locking/lockstat.txt
@@ -91,7 +91,7 @@ Look at the current lock statistics:
 07 &mm->mmap_sem-R:37100   
1.31  299502.61  325629.523256.30 212344   
34316685   0.107744.9195016910.20   2.77
 08 ---
 09   &mm->mmap_sem  1  
[] khugepaged_scan_mm_slot+0x57/0x280
-19   &mm->mmap_sem 96  
[] __do_page_fault+0x1d4/0x510
+10   &mm->mmap_sem 96  
[] __do_page_fault+0x1d4/0x510
 11   &mm->mmap_sem 34  
[] vm_mmap_pgoff+0x87/0xd0
 12   &mm->mmap_sem 17  
[] vm_munmap+0x41/0x80
 13 ---
-- 
2.7.4

Re: [PATCH] nvmem: fix nvmem_cell_get_from_lookup()

[David Lechner]

On 10/01/2018 05:00 AM, Bartosz Golaszewski wrote:

From: Bartosz Golaszewski 

We check if the pointer returned by __nvmem_device_get() is not NULL
while we should actually check if it is not IS_ERR(nvmem). Fix it.

While we're at it: fix the next error path where we should assign an
error value to cell before returning.

Signed-off-by: Bartosz Golaszewski 
---
  drivers/nvmem/core.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ad1227df1984..8249621d11a6 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -952,9 +952,9 @@ nvmem_cell_get_from_lookup(struct device *dev, const char 
*con_id)
(strcmp(lookup->con_id, con_id) == 0)) {
/* This is the right entry. */
nvmem = __nvmem_device_get(NULL, lookup->nvmem_name);
-   if (!nvmem) {
+   if (IS_ERR(nvmem)) {
/* Provider may not be registered yet. */
-   cell = ERR_PTR(-EPROBE_DEFER);
+   cell = (struct nvmem_cell *)nvmem;


perhaps ERR_CAST() would better indicate the intent here?


goto out;
}
  
@@ -962,6 +962,7 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id)

   lookup->cell_name);
if (!cell) {
__nvmem_device_put(nvmem);
+   cell = ERR_PTR(-ENOENT);
goto out;
}
}

Re: [PATCH] nvmem: fix nvmem_cell_get_from_lookup()

[Bartosz Golaszewski]

pon., 1 paź 2018 o 18:03 David Lechner  napisał(a):
>
> On 10/01/2018 05:00 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski 
> >
> > We check if the pointer returned by __nvmem_device_get() is not NULL
> > while we should actually check if it is not IS_ERR(nvmem). Fix it.
> >
> > While we're at it: fix the next error path where we should assign an
> > error value to cell before returning.
> >
> > Signed-off-by: Bartosz Golaszewski 
> > ---
> >   drivers/nvmem/core.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index ad1227df1984..8249621d11a6 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -952,9 +952,9 @@ nvmem_cell_get_from_lookup(struct device *dev, const 
> > char *con_id)
> >   (strcmp(lookup->con_id, con_id) == 0)) {
> >   /* This is the right entry. */
> >   nvmem = __nvmem_device_get(NULL, lookup->nvmem_name);
> > - if (!nvmem) {
> > + if (IS_ERR(nvmem)) {
> >   /* Provider may not be registered yet. */
> > - cell = ERR_PTR(-EPROBE_DEFER);
> > + cell = (struct nvmem_cell *)nvmem;
>
> perhaps ERR_CAST() would better indicate the intent here?
>

Wow so that exists too...

Thanks, I'll resend tomorrow.

Bart

> >   goto out;
> >   }
> >
> > @@ -962,6 +962,7 @@ nvmem_cell_get_from_lookup(struct device *dev, const 
> > char *con_id)
> >  lookup->cell_name);
> >   if (!cell) {
> >   __nvmem_device_put(nvmem);
> > + cell = ERR_PTR(-ENOENT);
> >   goto out;
> >   }
> >   }
> >
>

Re: [PATCH security-next v3 01/29] LSM: Correctly announce start of LSM initialization

[James Morris]

On Mon, 24 Sep 2018, Kees Cook wrote:

> For a while now, the LSM core has said it was "initializED", rather than
> "initializING". This adjust the report to be more accurate (i.e. before
> this was reported before any LSMs had been initialized.)
> 
> Signed-off-by: Kees Cook 
> Reviewed-by: Casey Schaufler 


Reviewed-by: James Morris 


-- 
James Morris

Re: [PATCH security-next v3 02/29] vmlinux.lds.h: Avoid copy/paste of security_init section

[James Morris]

On Mon, 24 Sep 2018, Kees Cook wrote:

> Avoid copy/paste by defining SECURITY_INIT in terms of SECURITY_INITCALL.
> 
> Cc: Arnd Bergmann 
> Cc: linux-a...@vger.kernel.org
> Signed-off-by: Kees Cook 
> ---
>  include/asm-generic/vmlinux.lds.h | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)


Reviewed-by: James Morris 


-- 
James Morris

Re: [PATCH security-next v3 03/29] LSM: Rename .security_initcall section to .lsm_info

[James Morris]

On Mon, 24 Sep 2018, Kees Cook wrote:

> In preparation for switching from initcall to just a regular set of
> pointers in a section, rename the internal section name.
> 
> Cc: Arnd Bergmann 
> Cc: James Morris 
> Cc: "Serge E. Hallyn" 
> Cc: Ard Biesheuvel 
> Cc: linux-a...@vger.kernel.org
> Cc: linux-security-mod...@vger.kernel.org
> Signed-off-by: Kees Cook 


Reviewed-by: James Morris 


-- 
James Morris

Re: [PATCH security-next v3 05/29] LSM: Convert from initcall to struct lsm_info

[James Morris]

On Mon, 24 Sep 2018, Kees Cook wrote:

> In preparation for doing more interesting LSM init probing, this converts
> the existing initcall system into an explicit call into a function pointer
> from a section-collected struct lsm_info array.
> 
> Cc: James Morris 
> Cc: "Serge E. Hallyn" 
> Cc: Ard Biesheuvel 
> Cc: Paul Moore 
> Cc: linux-security-mod...@vger.kernel.org
> Signed-off-by: Kees Cook 


Reviewed-by: James Morris 


-- 
James Morris

Re: [PATCH security-next v3 01/29] LSM: Correctly announce start of LSM initialization

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> For a while now, the LSM core has said it was "initializED", rather than
> "initializING". This adjust the report to be more accurate (i.e. before
> this was reported before any LSMs had been initialized.)
> 
> Signed-off-by: Kees Cook 
> Reviewed-by: Casey Schaufler 

Reviewed-by: John Johansen 

> ---
>  security/security.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/security/security.c b/security/security.c
> index 736e78da1ab9..4cbcf244a965 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -72,10 +72,11 @@ int __init security_init(void)
>   int i;
>   struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
>  
> + pr_info("Security Framework initializing\n");
> +
>   for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
>i++)
>   INIT_HLIST_HEAD(&list[i]);
> - pr_info("Security Framework initialized\n");
>  
>   /*
>* Load minor LSMs, with the capability module always first.
> 

Re: [PATCH security-next v3 02/29] vmlinux.lds.h: Avoid copy/paste of security_init section

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> Avoid copy/paste by defining SECURITY_INIT in terms of SECURITY_INITCALL.
> 
> Cc: Arnd Bergmann 
> Cc: linux-a...@vger.kernel.org
> Signed-off-by: Kees Cook 


Reviewed-by: John Johansen 

> ---
>  include/asm-generic/vmlinux.lds.h | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h 
> b/include/asm-generic/vmlinux.lds.h
> index 7b75ff6e2fce..934a45395547 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -473,13 +473,6 @@
>  #define RODATA  RO_DATA_SECTION(4096)
>  #define RO_DATA(align)  RO_DATA_SECTION(align)
>  
> -#define SECURITY_INIT
> \
> - .security_initcall.init : AT(ADDR(.security_initcall.init) - 
> LOAD_OFFSET) { \
> - __security_initcall_start = .;  \
> - KEEP(*(.security_initcall.init))\
> - __security_initcall_end = .;\
> - }
> -
>  /*
>   * .text section. Map to function alignment to avoid address changes
>   * during second ld run in second ld pass when generating System.map
> @@ -798,6 +791,12 @@
>   KEEP(*(.security_initcall.init))\
>   __security_initcall_end = .;
>  
> +/* Older linker script style for security init. */
> +#define SECURITY_INIT
> \
> + .security_initcall.init : AT(ADDR(.security_initcall.init) - 
> LOAD_OFFSET) { \
> + SECURITY_INITCALL   \
> + }
> +
>  #ifdef CONFIG_BLK_DEV_INITRD
>  #define INIT_RAM_FS  \
>   . = ALIGN(4);   \
> 

Re: [PATCH security-next v3 03/29] LSM: Rename .security_initcall section to .lsm_info

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> In preparation for switching from initcall to just a regular set of
> pointers in a section, rename the internal section name.
> 
> Cc: Arnd Bergmann 
> Cc: James Morris 
> Cc: "Serge E. Hallyn" 
> Cc: Ard Biesheuvel 
> Cc: linux-a...@vger.kernel.org
> Cc: linux-security-mod...@vger.kernel.org
> Signed-off-by: Kees Cook 


Reviewed-by: John Johansen 

> ---
>  include/asm-generic/vmlinux.lds.h | 10 +-
>  include/linux/init.h  |  4 ++--
>  security/security.c   |  4 ++--
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h 
> b/include/asm-generic/vmlinux.lds.h
> index 934a45395547..5079a969e612 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -787,14 +787,14 @@
>   __con_initcall_end = .;
>  
>  #define SECURITY_INITCALL\
> - __security_initcall_start = .;  \
> - KEEP(*(.security_initcall.init))\
> - __security_initcall_end = .;
> + __start_lsm_info = .;   \
> + KEEP(*(.lsm_info.init)) \
> + __end_lsm_info = .;
>  
>  /* Older linker script style for security init. */
>  #define SECURITY_INIT
> \
> - .security_initcall.init : AT(ADDR(.security_initcall.init) - 
> LOAD_OFFSET) { \
> - SECURITY_INITCALL   \
> + .lsm_info.init : AT(ADDR(.lsm_info.init) - LOAD_OFFSET) {   \
> + LSM_INFO\
>   }
>  
>  #ifdef CONFIG_BLK_DEV_INITRD
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 2538d176dd1f..77636539e77c 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -133,7 +133,7 @@ static inline initcall_t 
> initcall_from_entry(initcall_entry_t *entry)
>  #endif
>  
>  extern initcall_entry_t __con_initcall_start[], __con_initcall_end[];
> -extern initcall_entry_t __security_initcall_start[], 
> __security_initcall_end[];
> +extern initcall_entry_t __start_lsm_info[], __end_lsm_info[];
>  
>  /* Used for contructor calls. */
>  typedef void (*ctor_fn_t)(void);
> @@ -236,7 +236,7 @@ extern bool initcall_debug;
>   static exitcall_t __exitcall_##fn __exit_call = fn
>  
>  #define console_initcall(fn) ___define_initcall(fn,, .con_initcall)
> -#define security_initcall(fn)___define_initcall(fn,, 
> .security_initcall)
> +#define security_initcall(fn)___define_initcall(fn,, .lsm_info)
>  
>  struct obs_kernel_param {
>   const char *str;
> diff --git a/security/security.c b/security/security.c
> index 4cbcf244a965..892fe6b691cf 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -51,9 +51,9 @@ static void __init do_security_initcalls(void)
>   initcall_t call;
>   initcall_entry_t *ce;
>  
> - ce = __security_initcall_start;
> + ce = __start_lsm_info;
>   trace_initcall_level("security");
> - while (ce < __security_initcall_end) {
> + while (ce < __end_lsm_info) {
>   call = initcall_from_entry(ce);
>   trace_initcall_start(call);
>   ret = call();
> 

Re: [PATCH security-next v3 04/29] LSM: Remove initcall tracing

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> This partially reverts commit 58eacfffc417 ("init, tracing: instrument
> security and console initcall trace events") since security init calls
> are about to no longer resemble regular init calls.
> 
> Cc: James Morris 
> Cc: "Serge E. Hallyn" 
> Cc: Abderrahmane Benbachir 
> Cc: Steven Rostedt (VMware) 
> Cc: linux-security-mod...@vger.kernel.org
> Signed-off-by: Kees Cook 


Reviewed-by: John Johansen 

though I do think it would be a good idea to add a new set
of trace points, but that can come as a separate patch

> ---
>  security/security.c | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/security/security.c b/security/security.c
> index 892fe6b691cf..41a5da2c7faf 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -30,8 +30,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #define MAX_LSM_EVM_XATTR2
>  
>  /* Maximum number of letters for an LSM name string */
> @@ -47,17 +45,13 @@ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
>  
>  static void __init do_security_initcalls(void)
>  {
> - int ret;
>   initcall_t call;
>   initcall_entry_t *ce;
>  
>   ce = __start_lsm_info;
> - trace_initcall_level("security");
>   while (ce < __end_lsm_info) {
>   call = initcall_from_entry(ce);
> - trace_initcall_start(call);
> - ret = call();
> - trace_initcall_finish(call, ret);
> + call();
>   ce++;
>   }
>  }
> 

Re: [PATCH security-next v3 05/29] LSM: Convert from initcall to struct lsm_info

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> In preparation for doing more interesting LSM init probing, this converts
> the existing initcall system into an explicit call into a function pointer
> from a section-collected struct lsm_info array.
> 
> Cc: James Morris 
> Cc: "Serge E. Hallyn" 
> Cc: Ard Biesheuvel 
> Cc: Paul Moore 
> Cc: linux-security-mod...@vger.kernel.org
> Signed-off-by: Kees Cook 


Reviewed-by: John Johansen 

> ---
>  include/linux/init.h  |  2 --
>  include/linux/lsm_hooks.h | 12 
>  include/linux/module.h|  1 -
>  security/integrity/iint.c |  1 +
>  security/security.c   | 14 +-
>  5 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 77636539e77c..9c2aba1dbabf 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -133,7 +133,6 @@ static inline initcall_t 
> initcall_from_entry(initcall_entry_t *entry)
>  #endif
>  
>  extern initcall_entry_t __con_initcall_start[], __con_initcall_end[];
> -extern initcall_entry_t __start_lsm_info[], __end_lsm_info[];
>  
>  /* Used for contructor calls. */
>  typedef void (*ctor_fn_t)(void);
> @@ -236,7 +235,6 @@ extern bool initcall_debug;
>   static exitcall_t __exitcall_##fn __exit_call = fn
>  
>  #define console_initcall(fn) ___define_initcall(fn,, .con_initcall)
> -#define security_initcall(fn)___define_initcall(fn,, .lsm_info)
>  
>  struct obs_kernel_param {
>   const char *str;
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 97a020c616ad..ad04761e5587 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2039,6 +2039,18 @@ extern char *lsm_names;
>  extern void security_add_hooks(struct security_hook_list *hooks, int count,
>   char *lsm);
>  
> +struct lsm_info {
> + int (*init)(void);
> +};
> +
> +extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
> +
> +#define security_initcall(lsm)   
> \
> + static struct lsm_info __lsm_##lsm  \
> + __used __section(.lsm_info.init)\
> + __aligned(sizeof(unsigned long))\
> + = { .init = lsm, }
> +
>  #ifdef CONFIG_SECURITY_SELINUX_DISABLE
>  /*
>   * Assuring the safety of deleting a security module is up to
> diff --git a/include/linux/module.h b/include/linux/module.h
> index f807f15bebbe..264979283756 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -123,7 +123,6 @@ extern void cleanup_module(void);
>  #define late_initcall_sync(fn)   module_init(fn)
>  
>  #define console_initcall(fn) module_init(fn)
> -#define security_initcall(fn)module_init(fn)
>  
>  /* Each module must use one module_init(). */
>  #define module_init(initfn)  \
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 5a6810041e5c..70d21b566955 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "integrity.h"
>  
>  static struct rb_root integrity_iint_tree = RB_ROOT;
> diff --git a/security/security.c b/security/security.c
> index 41a5da2c7faf..e74f46fba591 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -43,16 +43,12 @@ char *lsm_names;
>  static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
>   CONFIG_DEFAULT_SECURITY;
>  
> -static void __init do_security_initcalls(void)
> +static void __init major_lsm_init(void)
>  {
> - initcall_t call;
> - initcall_entry_t *ce;
> + struct lsm_info *lsm;
>  
> - ce = __start_lsm_info;
> - while (ce < __end_lsm_info) {
> - call = initcall_from_entry(ce);
> - call();
> - ce++;
> + for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> + lsm->init();
>   }
>  }
>  
> @@ -82,7 +78,7 @@ int __init security_init(void)
>   /*
>* Load all the remaining security modules.
>*/
> - do_security_initcalls();
> + major_lsm_init();
>  
>   return 0;
>  }
> 

Re: [PATCH security-next v3 06/29] vmlinux.lds.h: Move LSM_TABLE into INIT_DATA

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> Since the struct lsm_info table is not an initcall, we can just move it
> into INIT_DATA like all the other tables.
> 
> Cc: linux-a...@vger.kernel.org
> Signed-off-by: Kees Cook 


Reviewed-by: John Johansen 


> ---
>  arch/arc/kernel/vmlinux.lds.S|  1 -
>  arch/arm/kernel/vmlinux-xip.lds.S|  1 -
>  arch/arm64/kernel/vmlinux.lds.S  |  1 -
>  arch/h8300/kernel/vmlinux.lds.S  |  1 -
>  arch/microblaze/kernel/vmlinux.lds.S |  2 --
>  arch/powerpc/kernel/vmlinux.lds.S|  2 --
>  arch/um/include/asm/common.lds.S |  2 --
>  arch/xtensa/kernel/vmlinux.lds.S |  1 -
>  include/asm-generic/vmlinux.lds.h| 24 +++-
>  9 files changed, 11 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/arc/kernel/vmlinux.lds.S b/arch/arc/kernel/vmlinux.lds.S
> index f35ed578e007..8fb16bdabdcf 100644
> --- a/arch/arc/kernel/vmlinux.lds.S
> +++ b/arch/arc/kernel/vmlinux.lds.S
> @@ -71,7 +71,6 @@ SECTIONS
>   INIT_SETUP(L1_CACHE_BYTES)
>   INIT_CALLS
>   CON_INITCALL
> - SECURITY_INITCALL
>   }
>  
>   .init.arch.info : {
> diff --git a/arch/arm/kernel/vmlinux-xip.lds.S 
> b/arch/arm/kernel/vmlinux-xip.lds.S
> index 3593d5c1acd2..8c74037ade22 100644
> --- a/arch/arm/kernel/vmlinux-xip.lds.S
> +++ b/arch/arm/kernel/vmlinux-xip.lds.S
> @@ -96,7 +96,6 @@ SECTIONS
>   INIT_SETUP(16)
>   INIT_CALLS
>   CON_INITCALL
> - SECURITY_INITCALL
>   INIT_RAM_FS
>   }
>  
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 605d1b60469c..7d23d591b03c 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -166,7 +166,6 @@ SECTIONS
>   INIT_SETUP(16)
>   INIT_CALLS
>   CON_INITCALL
> - SECURITY_INITCALL
>   INIT_RAM_FS
>   *(.init.rodata.* .init.bss) /* from the EFI stub */
>   }
> diff --git a/arch/h8300/kernel/vmlinux.lds.S b/arch/h8300/kernel/vmlinux.lds.S
> index 35716a3048de..49f716c0a1df 100644
> --- a/arch/h8300/kernel/vmlinux.lds.S
> +++ b/arch/h8300/kernel/vmlinux.lds.S
> @@ -56,7 +56,6 @@ SECTIONS
>   __init_begin = .;
>   INIT_TEXT_SECTION(4)
>   INIT_DATA_SECTION(4)
> - SECURITY_INIT
>   __init_end = .;
>   _edata = . ;
>   _begin_data = LOADADDR(.data);
> diff --git a/arch/microblaze/kernel/vmlinux.lds.S 
> b/arch/microblaze/kernel/vmlinux.lds.S
> index 289d0e7f3e3a..e1f3e8741292 100644
> --- a/arch/microblaze/kernel/vmlinux.lds.S
> +++ b/arch/microblaze/kernel/vmlinux.lds.S
> @@ -117,8 +117,6 @@ SECTIONS {
>   CON_INITCALL
>   }
>  
> - SECURITY_INIT
> -
>   __init_end_before_initramfs = .;
>  
>   .init.ramfs : AT(ADDR(.init.ramfs) - LOAD_OFFSET) {
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S 
> b/arch/powerpc/kernel/vmlinux.lds.S
> index 07ae018e550e..105a976323aa 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -212,8 +212,6 @@ SECTIONS
>   CON_INITCALL
>   }
>  
> - SECURITY_INIT
> -
>   . = ALIGN(8);
>   __ftr_fixup : AT(ADDR(__ftr_fixup) - LOAD_OFFSET) {
>   __start___ftr_fixup = .;
> diff --git a/arch/um/include/asm/common.lds.S 
> b/arch/um/include/asm/common.lds.S
> index 7adb4e6b658a..4049f2c46387 100644
> --- a/arch/um/include/asm/common.lds.S
> +++ b/arch/um/include/asm/common.lds.S
> @@ -53,8 +53,6 @@
>   CON_INITCALL
>}
>  
> -  SECURITY_INIT
> -
>.exitcall : {
>   __exitcall_begin = .;
>   *(.exitcall.exit)
> diff --git a/arch/xtensa/kernel/vmlinux.lds.S 
> b/arch/xtensa/kernel/vmlinux.lds.S
> index a1c3edb8ad56..b727b18a68ac 100644
> --- a/arch/xtensa/kernel/vmlinux.lds.S
> +++ b/arch/xtensa/kernel/vmlinux.lds.S
> @@ -197,7 +197,6 @@ SECTIONS
>  INIT_SETUP(XCHAL_ICACHE_LINESIZE)
>  INIT_CALLS
>  CON_INITCALL
> -SECURITY_INITCALL
>  INIT_RAM_FS
>}
>  
> diff --git a/include/asm-generic/vmlinux.lds.h 
> b/include/asm-generic/vmlinux.lds.h
> index 5079a969e612..b31ea8bdfef9 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -203,6 +203,15 @@
>  #define EARLYCON_TABLE()
>  #endif
>  
> +#ifdef CONFIG_SECURITY
> +#define LSM_TABLE()  . = ALIGN(8);   \
> + __start_lsm_info = .;   \
> + KEEP(*(.lsm_info.init)) \
> + __end_lsm_info = .;
> +#else
> +#define LSM_TABLE()
> +#endif
> +
>  #define ___OF_TABLE(cfg, name)   _OF_TABLE_##cfg(name)
>  #define __OF_TABLE(cfg, name)___OF_TABLE(cfg, name)
>  #define OF_TABLE(cfg, name)  __OF_TABLE(IS_ENABLED(cfg), name)
> @@ -597,7 +606,8 @@
>   IRQCHIP_OF_MATCH_TABLE()\
>   ACPI_PROBE_TABLE(irqchi

Re: [PATCH security-next v3 07/29] LSM: Convert security_initcall() into DEFINE_LSM()

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> Instead of using argument-based initializers, switch to defining the
> contents of struct lsm_info on a per-LSM basis. This also drops
> the final use of the now inaccurate "initcall" naming.
> 
> Cc: John Johansen 
> Cc: James Morris 
> Cc: "Serge E. Hallyn" 
> Cc: Paul Moore 
> Cc: Stephen Smalley 
> Cc: Casey Schaufler 
> Cc: Tetsuo Handa 
> Cc: Mimi Zohar 
> Cc: linux-security-mod...@vger.kernel.org
> Cc: seli...@tycho.nsa.gov
> Signed-off-by: Kees Cook 
> ---
>  include/linux/lsm_hooks.h  | 6 --
>  security/apparmor/lsm.c| 4 +++-
>  security/integrity/iint.c  | 4 +++-
>  security/selinux/hooks.c   | 4 +++-
>  security/smack/smack_lsm.c | 4 +++-
>  security/tomoyo/tomoyo.c   | 4 +++-
>  6 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ad04761e5587..02ec717189f9 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2045,11 +2045,13 @@ struct lsm_info {
>  
>  extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
>  
> -#define security_initcall(lsm)   
> \
> +#define DEFINE_LSM(lsm)  
> \
>   static struct lsm_info __lsm_##lsm  \
>   __used __section(.lsm_info.init)\
>   __aligned(sizeof(unsigned long))\
> - = { .init = lsm, }
> + = { \
> +
> +#define END_LSM}
>  

I am with Tetsuo on this one, I really don't like the END_LSM thing.

Re: [PATCH security-next v3 08/29] LSM: Record LSM name in struct lsm_info

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> In preparation for making LSM selections outside of the LSMs, include
> the name of LSMs in struct lsm_info.
> 
> Cc: James Morris 
> Signed-off-by: Kees Cook 

I'll leave this one until after the changes you have already discussed with 
Tetsuo around, END_LSM and .name

> ---
>  include/linux/lsm_hooks.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 02ec717189f9..543636f18152 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2040,16 +2040,20 @@ extern void security_add_hooks(struct 
> security_hook_list *hooks, int count,
>   char *lsm);
>  
>  struct lsm_info {
> + const char *name;   /* Populated automatically. */
>   int (*init)(void);
>  };
>  
>  extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
>  
>  #define DEFINE_LSM(lsm)  
> \
> + static const char __lsm_name_##lsm[] __initconst\
> + __aligned(1) = #lsm;\
>   static struct lsm_info __lsm_##lsm  \
>   __used __section(.lsm_info.init)\
>   __aligned(sizeof(unsigned long))\
>   = { \
> + .name = __lsm_name_##lsm,   \
>  
>  #define END_LSM}
>  
> 

Re: [PATCH security-next v3 09/29] LSM: Provide init debugging infrastructure

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> Booting with "lsm.debug" will report future details on how LSM ordering
> decisions are being made.
> 
> Signed-off-by: Kees Cook 

Reviewed-by: John Johansen 


> ---
>  .../admin-guide/kernel-parameters.txt  |  2 ++
>  security/security.c| 18 ++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index 9871e649ffef..32d323ee9218 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2274,6 +2274,8 @@
>   ltpc=   [NET]
>   Format: ,,
>  
> + lsm.debug   [SECURITY] Enable LSM initialization debugging output.
> +
>   machvec=[IA-64] Force the use of a particular machine-vector
>   (machvec) in a generic kernel.
>   Example: machvec=hpzx1_swiotlb
> diff --git a/security/security.c b/security/security.c
> index e74f46fba591..ee49b921d750 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -12,6 +12,8 @@
>   *   (at your option) any later version.
>   */
>  
> +#define pr_fmt(fmt) "LSM: " fmt
> +
>  #include 
>  #include 
>  #include 
> @@ -43,11 +45,19 @@ char *lsm_names;
>  static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
>   CONFIG_DEFAULT_SECURITY;
>  
> +static bool debug __initdata;
> +#define init_debug(...)  \
> + do {\
> + if (debug)  \
> + pr_info(__VA_ARGS__);   \
> + } while (0)
> +
>  static void __init major_lsm_init(void)
>  {
>   struct lsm_info *lsm;
>  
>   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> + init_debug("initializing %s\n", lsm->name);
>   lsm->init();
>   }
>  }
> @@ -91,6 +101,14 @@ static int __init choose_lsm(char *str)
>  }
>  __setup("security=", choose_lsm);
>  
> +/* Enable LSM order debugging. */
> +static int __init enable_debug(char *str)
> +{
> + debug = true;
> + return 1;
> +}
> +__setup("lsm.debug", enable_debug);
> +
>  static bool match_last_lsm(const char *list, const char *lsm)
>  {
>   const char *last;
> 

Re: [PATCH security-next v3 10/29] LSM: Don't ignore initialization failures

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> LSM initialization failures have traditionally been ignored. We should
> at least WARN when something goes wrong.
> 
> Signed-off-by: Kees Cook 

about time

Reviewed-by: John Johansen 

> ---
>  security/security.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/security/security.c b/security/security.c
> index ee49b921d750..1f055936a746 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -55,10 +55,12 @@ static bool debug __initdata;
>  static void __init major_lsm_init(void)
>  {
>   struct lsm_info *lsm;
> + int ret;
>  
>   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
>   init_debug("initializing %s\n", lsm->name);
> - lsm->init();
> + ret = lsm->init();
> + WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
>   }
>  }
>  
> 

Re: [PATCH security-next v3 11/29] LSM: Introduce LSM_FLAG_LEGACY_MAJOR

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> This adds a flag for the current "major" LSMs to distinguish them when
> we have a universal method for ordering all LSMs. It's called "legacy"
> since the distinction of "major" will go away in the blob-sharing world.
> 
> Signed-off-by: Kees Cook 

Reviewed-by: John Johansen 

> ---
>  include/linux/lsm_hooks.h  | 3 +++
>  security/apparmor/lsm.c| 1 +
>  security/selinux/hooks.c   | 1 +
>  security/smack/smack_lsm.c | 1 +
>  security/tomoyo/tomoyo.c   | 1 +
>  5 files changed, 7 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 543636f18152..5056f7374b3d 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2039,8 +2039,11 @@ extern char *lsm_names;
>  extern void security_add_hooks(struct security_hook_list *hooks, int count,
>   char *lsm);
>  
> +#define LSM_FLAG_LEGACY_MAJORBIT(0)
> +
>  struct lsm_info {
>   const char *name;   /* Populated automatically. */
> + unsigned long flags;/* Optional: flags describing LSM */
>   int (*init)(void);
>  };
>  
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 7fa7b4464cf4..4c5f63e9aeba 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1607,5 +1607,6 @@ static int __init apparmor_init(void)
>  }
>  
>  DEFINE_LSM(apparmor)
> + .flags = LSM_FLAG_LEGACY_MAJOR,
>   .init = apparmor_init,
>  END_LSM;
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 469a90806bc6..615cf6498c0f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7203,6 +7203,7 @@ void selinux_complete_init(void)
>  /* SELinux requires early initialization in order to label
> all processes and objects when they are created. */
>  DEFINE_LSM(selinux)
> + .flags = LSM_FLAG_LEGACY_MAJOR,
>   .init = selinux_init,
>  END_LSM;
>  
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 1e1ace718e75..4aef844fc0e2 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4883,5 +4883,6 @@ static __init int smack_init(void)
>   * all processes and objects when they are created.
>   */
>  DEFINE_LSM(smack)
> + .flags = LSM_FLAG_LEGACY_MAJOR,
>   .init = smack_init,
>  END_LSM;
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index a280d4eab456..528b6244a648 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -551,5 +551,6 @@ static int __init tomoyo_init(void)
>  }
>  
>  DEFINE_LSM(tomoyo)
> + .flags = LSM_FLAG_LEGACY_MAJOR,
>   .init = tomoyo_init,
>  END_LSM;
> 

Re: [PATCH security-next v3 12/29] LSM: Provide separate ordered initialization

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> This provides a place for ordered LSMs to be initialized, separate from
> the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
> ordered_lsm_init(), but it will change drastically in later patches.
> 
> What is not obvious in the patch is that this change moves the integrity
> LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
> with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
> list, there is no reordering yet created.
> 
> Signed-off-by: Kees Cook 

I know its already being done, but I don't like splitting the init
order

Reviewed-by: John Johansen 

> ---
>  security/security.c | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/security/security.c b/security/security.c
> index 1f055936a746..a886a978214a 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -52,12 +52,30 @@ static bool debug __initdata;
>   pr_info(__VA_ARGS__);   \
>   } while (0)
>  
> +static void __init ordered_lsm_init(void)
> +{
> + struct lsm_info *lsm;
> + int ret;
> +
> + for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> + if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) != 0)
> + continue;
> +
> + init_debug("initializing %s\n", lsm->name);
> + ret = lsm->init();
> + WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> + }
> +}
> +
>  static void __init major_lsm_init(void)
>  {
>   struct lsm_info *lsm;
>   int ret;
>  
>   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> + if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
> + continue;
> +
>   init_debug("initializing %s\n", lsm->name);
>   ret = lsm->init();
>   WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> @@ -87,6 +105,9 @@ int __init security_init(void)
>   yama_add_hooks();
>   loadpin_add_hooks();
>  
> + /* Load LSMs in specified order. */
> + ordered_lsm_init();
> +
>   /*
>* Load all the remaining security modules.
>*/
> 

Re: [PATCH security-next v3 13/29] LoadPin: Rename "enable" to "enforce"

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> LoadPin's "enable" setting is really about enforcement, not whether
> or not the LSM is using LSM hooks. Instead, split this out so that LSM
> enabling can be logically distinct from whether enforcement is happening
> (for example, the pinning happens when the LSM is enabled, but the pin
> is only checked when "enforce" is set). This allows LoadPin to continue
> to operate sanely in test environments once LSM enable/disable is
> centrally handled (i.e. we want LoadPin to be enabled separately from
> its enforcement).
> 
> Signed-off-by: Kees Cook 

Reviewed-by: John Johansen 

> ---
>  security/loadpin/Kconfig   |  4 ++--
>  security/loadpin/loadpin.c | 21 +++--
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
> index dd01aa91e521..8653608a3693 100644
> --- a/security/loadpin/Kconfig
> +++ b/security/loadpin/Kconfig
> @@ -10,10 +10,10 @@ config SECURITY_LOADPIN
> have a root filesystem backed by a read-only device such as
> dm-verity or a CDROM.
>  
> -config SECURITY_LOADPIN_ENABLED
> +config SECURITY_LOADPIN_ENFORCING
>   bool "Enforce LoadPin at boot"
>   depends on SECURITY_LOADPIN
>   help
> If selected, LoadPin will enforce pinning at boot. If not
> selected, it can be enabled at boot with the kernel parameter
> -   "loadpin.enabled=1".
> +   "loadpin.enforcing=1".
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 0716af28808a..d8a68a6f6fef 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -44,7 +44,7 @@ static void report_load(const char *origin, struct file 
> *file, char *operation)
>   kfree(pathname);
>  }
>  
> -static int enabled = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENABLED);
> +static int enforcing = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENFORCING);
>  static struct super_block *pinned_root;
>  static DEFINE_SPINLOCK(pinned_root_spinlock);
>  
> @@ -60,8 +60,8 @@ static struct ctl_path loadpin_sysctl_path[] = {
>  
>  static struct ctl_table loadpin_sysctl_table[] = {
>   {
> - .procname   = "enabled",
> - .data   = &enabled,
> + .procname   = "enforcing",
> + .data   = &enforcing,
>   .maxlen = sizeof(int),
>   .mode   = 0644,
>   .proc_handler   = proc_dointvec_minmax,
> @@ -97,7 +97,7 @@ static void check_pinning_enforcement(struct super_block 
> *mnt_sb)
>  loadpin_sysctl_table))
>   pr_notice("sysctl registration failed!\n");
>   else
> - pr_info("load pinning can be disabled.\n");
> + pr_info("enforcement can be disabled.\n");
>   } else
>   pr_info("load pinning engaged.\n");
>  }
> @@ -128,7 +128,7 @@ static int loadpin_read_file(struct file *file, enum 
> kernel_read_file_id id)
>  
>   /* This handles the older init_module API that has a NULL file. */
>   if (!file) {
> - if (!enabled) {
> + if (!enforcing) {
>   report_load(origin, NULL, "old-api-pinning-ignored");
>   return 0;
>   }
> @@ -151,7 +151,7 @@ static int loadpin_read_file(struct file *file, enum 
> kernel_read_file_id id)
>* Unlock now since it's only pinned_root we care about.
>* In the worst case, we will (correctly) report pinning
>* failures before we have announced that pinning is
> -  * enabled. This would be purely cosmetic.
> +  * enforcing. This would be purely cosmetic.
>*/
>   spin_unlock(&pinned_root_spinlock);
>   check_pinning_enforcement(pinned_root);
> @@ -161,7 +161,7 @@ static int loadpin_read_file(struct file *file, enum 
> kernel_read_file_id id)
>   }
>  
>   if (IS_ERR_OR_NULL(pinned_root) || load_root != pinned_root) {
> - if (unlikely(!enabled)) {
> + if (unlikely(!enforcing)) {
>   report_load(origin, file, "pinning-ignored");
>   return 0;
>   }
> @@ -186,10 +186,11 @@ static struct security_hook_list loadpin_hooks[] 
> __lsm_ro_after_init = {
>  
>  void __init loadpin_add_hooks(void)
>  {
> - pr_info("ready to pin (currently %sabled)", enabled ? "en" : "dis");
> + pr_info("ready to pin (currently %senforcing)\n",
> + enforcing ? "" : "not ");
>   security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
>  }
>  
>  /* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
> -module_param(enabled, int, 0);
> -MODULE_PARM_DESC(enabled, "Pin module/firmware loading (default: true)");
> +module_param(enforcing, int, 0);
> +MODULE_PARM_DESC(enforcing, "Enforce module/firm

Re: [PATCH security-next v3 14/29] LSM: Plumb visibility into optional "enabled" state

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> In preparation for lifting the "is this LSM enabled?" logic out of the
> individual LSMs, pass in any special enabled state tracking (as needed
> for SELinux, AppArmor, and LoadPin). This should be an "int" to include
> handling any future cases where "enabled" is exposed via sysctl which
> has no "bool" type.
> 
> Signed-off-by: Kees Cook 

Reviewed-by: John Johansen 


> ---
>  include/linux/lsm_hooks.h | 1 +
>  security/apparmor/lsm.c   | 5 +++--
>  security/selinux/hooks.c  | 1 +
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 5056f7374b3d..2a41e8e6f6e5 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct 
> security_hook_list *hooks, int count,
>  struct lsm_info {
>   const char *name;   /* Populated automatically. */
>   unsigned long flags;/* Optional: flags describing LSM */
> + int *enabled;   /* Optional: NULL means enabled. */
>   int (*init)(void);
>  };
>  
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 4c5f63e9aeba..d03133a267f2 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1303,8 +1303,8 @@ bool aa_g_paranoid_load = true;
>  module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
>  
>  /* Boot time disable flag */
> -static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
> -module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
> +static int apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
> +module_param_named(enabled, apparmor_enabled, int, 0444);
>  
>  static int __init apparmor_enabled_setup(char *str)
>  {
> @@ -1608,5 +1608,6 @@ static int __init apparmor_init(void)
>  
>  DEFINE_LSM(apparmor)
>   .flags = LSM_FLAG_LEGACY_MAJOR,
> + .enabled = &apparmor_enabled,
>   .init = apparmor_init,
>  END_LSM;
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 615cf6498c0f..3f999ed98cfd 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7204,6 +7204,7 @@ void selinux_complete_init(void)
> all processes and objects when they are created. */
>  DEFINE_LSM(selinux)
>   .flags = LSM_FLAG_LEGACY_MAJOR,
> + .enabled = &selinux_enabled,
>   .init = selinux_init,
>  END_LSM;
>  
> 

Re: [PATCH security-next v3 15/29] LSM: Lift LSM selection out of individual LSMs

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> As a prerequisite to adjusting LSM selection logic in the future, this
> moves the selection logic up out of the individual major LSMs, making
> their init functions only run when actually enabled. This considers all
> LSMs enabled by default unless they specified an external "enable"
> variable.
> 
> Signed-off-by: Kees Cook 

Reviewed-by: John Johansen 


> ---
>  include/linux/lsm_hooks.h  |  1 -
>  security/apparmor/lsm.c|  6 ---
>  security/security.c| 84 --
>  security/selinux/hooks.c   | 10 -
>  security/smack/smack_lsm.c |  3 --
>  security/tomoyo/tomoyo.c   |  2 -
>  6 files changed, 53 insertions(+), 53 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 2a41e8e6f6e5..95798f212dbf 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2091,7 +2091,6 @@ static inline void security_delete_hooks(struct 
> security_hook_list *hooks,
>  #define __lsm_ro_after_init  __ro_after_init
>  #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
>  
> -extern int __init security_module_enable(const char *module);
>  extern void __init capability_add_hooks(void);
>  #ifdef CONFIG_SECURITY_YAMA
>  extern void __init yama_add_hooks(void);
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index d03133a267f2..5399c2f03536 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1542,12 +1542,6 @@ static int __init apparmor_init(void)
>  {
>   int error;
>  
> - if (!apparmor_enabled || !security_module_enable("apparmor")) {
> - aa_info_message("AppArmor disabled by boot time parameter");
> - apparmor_enabled = false;
> - return 0;
> - }
> -
>   aa_secids_init();
>  
>   error = aa_setup_dfa_engine();
> diff --git a/security/security.c b/security/security.c
> index a886a978214a..056b36cf6245 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -52,33 +52,78 @@ static bool debug __initdata;
>   pr_info(__VA_ARGS__);   \
>   } while (0)
>  
> +static bool __init is_enabled(struct lsm_info *lsm)
> +{
> + if (!lsm->enabled || *lsm->enabled)
> + return true;
> +
> + return false;
> +}
> +
> +/* Mark an LSM's enabled flag, if it exists. */
> +static void __init set_enabled(struct lsm_info *lsm, bool enabled)
> +{
> + if (lsm->enabled)
> + *lsm->enabled = enabled;
> +}
> +
> +/* Is an LSM allowed to be initialized? */
> +static bool __init lsm_allowed(struct lsm_info *lsm)
> +{
> + /* Skip if the LSM is disabled. */
> + if (!is_enabled(lsm))
> + return false;
> +
> + /* Skip major-specific checks if not a major LSM. */
> + if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
> + return true;
> +
> + /* Disabled if this LSM isn't the chosen one. */
> + if (strcmp(lsm->name, chosen_lsm) != 0)
> + return false;
> +
> + return true;
> +}
> +
> +/* Check if LSM should be enabled. Mark any that are disabled. */
> +static void __init maybe_initialize_lsm(struct lsm_info *lsm)
> +{
> + int enabled = lsm_allowed(lsm);
> +
> + /* Record enablement. */
> + set_enabled(lsm, enabled);
> +
> + /* If selected, initialize the LSM. */
> + if (enabled) {
> + int ret;
> +
> + init_debug("initializing %s\n", lsm->name);
> + ret = lsm->init();
> + WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> + }
> +}
> +
>  static void __init ordered_lsm_init(void)
>  {
>   struct lsm_info *lsm;
> - int ret;
>  
>   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
>   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) != 0)
>   continue;
>  
> - init_debug("initializing %s\n", lsm->name);
> - ret = lsm->init();
> - WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> + maybe_initialize_lsm(lsm);
>   }
>  }
>  
>  static void __init major_lsm_init(void)
>  {
>   struct lsm_info *lsm;
> - int ret;
>  
>   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
>   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
>   continue;
>  
> - init_debug("initializing %s\n", lsm->name);
> - ret = lsm->init();
> - WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> + maybe_initialize_lsm(lsm);
>   }
>  }
>  
> @@ -168,29 +213,6 @@ static int lsm_append(char *new, char **result)
>   return 0;
>  }
>  
> -/**
> - * security_module_enable - Load given security module on boot ?
> - * @module: the name of the module
> - *
> - * Each LSM must pass this method before registering its own operations
> - * to avoid security registration races. This method may also be used
> - * to check if your 

Re: [PATCH security-next v3 16/29] LSM: Prepare for arbitrary LSM enabling

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> Before now, all the LSMs that did not specify an "enable" variable in their
> struct lsm_info were considered enabled by default. This prepares to make
> LSM enabling more explicit. For all LSMs without an explicit "enable"
> variable, a hard-coded storage location is chosen, and all LSMs without
> an external "enable" state have their state explicitly set to "enabled".
> 
> This code appears more complex than it needs to be (comma-separated
> list parsing and "set" function parameter) because its use will be
> expanded on in the following patches to provide more explicit enabling.
> 
> Signed-off-by: Kees Cook 


Reviewed-by: John Johansen 


> ---
>  security/security.c | 69 ++---
>  1 file changed, 65 insertions(+), 4 deletions(-)
> 
> diff --git a/security/security.c b/security/security.c
> index 056b36cf6245..a8107d54b3d3 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -54,17 +54,46 @@ static bool debug __initdata;
>  
>  static bool __init is_enabled(struct lsm_info *lsm)
>  {
> - if (!lsm->enabled || *lsm->enabled)
> - return true;
> + if (WARN_ON(!lsm->enabled))
> + return false;
>  
> - return false;
> + return *lsm->enabled;
>  }
>  
>  /* Mark an LSM's enabled flag, if it exists. */
> -static void __init set_enabled(struct lsm_info *lsm, bool enabled)
> +static int lsm_enabled_true __initdata = 1;
> +static int lsm_enabled_false __initdata = 0;
> +
> +static void __init default_enabled(struct lsm_info *lsm, bool enabled)
>  {
> + /* If storage location already set, skip this one. */
>   if (lsm->enabled)
> + return;
> +
> + /*
> +  * When an LSM hasn't configured an enable variable, we can use
> +  * a hard-coded location for storing the default enabled state.
> +  */
> + if (enabled)
> + lsm->enabled = &lsm_enabled_true;
> + else
> + lsm->enabled = &lsm_enabled_false;
> +}
> +
> +static void __init set_enabled(struct lsm_info *lsm, bool enabled)
> +{
> + if (WARN_ON(!lsm->enabled))
> + return;
> +
> + if (lsm->enabled == &lsm_enabled_true) {
> + if (!enabled)
> + lsm->enabled = &lsm_enabled_false;
> + } else if (lsm->enabled == &lsm_enabled_false) {
> + if (enabled)
> + lsm->enabled = &lsm_enabled_true;
> + } else {
>   *lsm->enabled = enabled;
> + }
>  }
>  
>  /* Is an LSM allowed to be initialized? */
> @@ -127,6 +156,35 @@ static void __init major_lsm_init(void)
>   }
>  }
>  
> +static void __init parse_lsm_enable(const char *str,
> + void (*set)(struct lsm_info *, bool),
> + bool enabled)
> +{
> + char *sep, *name, *next;
> +
> + if (!str)
> + return;
> +
> + sep = kstrdup(str, GFP_KERNEL);
> + next = sep;
> + while ((name = strsep(&next, ",")) != NULL) {
> + struct lsm_info *lsm;
> +
> + for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> + if (strcmp(name, "all") == 0 ||
> + strcmp(name, lsm->name) == 0)
> + set(lsm, enabled);
> + }
> + }
> + kfree(sep);
> +}
> +
> +static void __init prepare_lsm_enable(void)
> +{
> + /* Prepare defaults. */
> + parse_lsm_enable("all", default_enabled, true);
> +}
> +
>  /**
>   * security_init - initializes the security framework
>   *
> @@ -143,6 +201,9 @@ int __init security_init(void)
>i++)
>   INIT_HLIST_HEAD(&list[i]);
>  
> + /* Figure out which LSMs are enabled and disabled. */
> + prepare_lsm_enable();
> +
>   /*
>* Load minor LSMs, with the capability module always first.
>*/
> 

Re: [PATCH security-next v3 04/29] LSM: Remove initcall tracing

[Steven Rostedt]

On Mon, 1 Oct 2018 14:07:55 -0700
John Johansen  wrote:

> On 09/24/2018 05:18 PM, Kees Cook wrote:
> > This partially reverts commit 58eacfffc417 ("init, tracing: instrument
> > security and console initcall trace events") since security init calls
> > are about to no longer resemble regular init calls.
> > 
> > Cc: James Morris 
> > Cc: "Serge E. Hallyn" 
> > Cc: Abderrahmane Benbachir 
> > Cc: Steven Rostedt (VMware) 
> > Cc: linux-security-mod...@vger.kernel.org
> > Signed-off-by: Kees Cook   
> 
> 
> Reviewed-by: John Johansen 
> 
> though I do think it would be a good idea to add a new set
> of trace points, but that can come as a separate patch
> 
>

Agreed.

-- Steve

Re: [PATCH security-next v3 17/29] LSM: Introduce CONFIG_LSM_ENABLE

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> To provide a set of default-enabled LSMs at boot, this introduces the
> new CONFIG_LSM_ENABLE. A value of "all" means all builtin LSMs are
> enabled by default. Any unlisted LSMs will be implicitly disabled
> (excepting those with LSM-specific CONFIGs for enabling/disabling).
> 
> The behavior of the LSM-specific CONFIGs for SELinux are AppArmor
> unchanged: the default-enabled state for those LSMs remains controlled
> through their LSM-specific "enable" CONFIGs.
> 
> Signed-off-by: Kees Cook 

The patch is fine but I am not sure I like the behavior. I much prefer
that its an explicit list and nothing is left to implicit. That is
if there is a conflict between the list and the LSM-specific config
the LSM is disabled.



> ---
>  include/linux/lsm_hooks.h | 2 +-
>  security/Kconfig  | 8 
>  security/security.c   | 4 +++-
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 95798f212dbf..ab23f1bc6d77 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2044,7 +2044,7 @@ extern void security_add_hooks(struct 
> security_hook_list *hooks, int count,
>  struct lsm_info {
>   const char *name;   /* Populated automatically. */
>   unsigned long flags;/* Optional: flags describing LSM */
> - int *enabled;   /* Optional: NULL means enabled. */
> + int *enabled;   /* Optional: NULL checks CONFIG_LSM_ENABLE */
>   int (*init)(void);
>  };
>  
> diff --git a/security/Kconfig b/security/Kconfig
> index 27d8b2688f75..71306b046270 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -276,5 +276,13 @@ config DEFAULT_SECURITY
>   default "apparmor" if DEFAULT_SECURITY_APPARMOR
>   default "" if DEFAULT_SECURITY_DAC
>  
> +config LSM_ENABLE
> + string "LSMs to enable at boot time"
> + default "all"
> + help
> +   A comma-separate list of LSMs to enable by default at boot. The
> +   default is "all", to enable all LSM modules at boot. Any LSMs
> +   not listed here will be disabled by default.
> +
>  endmenu
>  
> diff --git a/security/security.c b/security/security.c
> index a8107d54b3d3..7ecb9879a863 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -45,6 +45,8 @@ char *lsm_names;
>  static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
>   CONFIG_DEFAULT_SECURITY;
>  
> +static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
> +
>  static bool debug __initdata;
>  #define init_debug(...)  \
>   do {\
> @@ -182,7 +184,7 @@ static void __init parse_lsm_enable(const char *str,
>  static void __init prepare_lsm_enable(void)
>  {
>   /* Prepare defaults. */
> - parse_lsm_enable("all", default_enabled, true);
> + parse_lsm_enable(builtin_lsm_enable, default_enabled, true);
>  }
>  
>  /**
> 

Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable=

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
> which each can contain a comma-separated list of LSMs to enable or
> disable, respectively. The string "all" matches all LSMs.
> 
> This has very similar functionality to the existing per-LSM enable
> handling ("apparmor.enabled=...", etc), but provides a centralized
> place to perform the changes. These parameters take precedent over any
> LSM-specific boot parameters.
> 
> Disabling an LSM means it will not be considered when performing
> initializations. Enabling an LSM means either undoing a previous
> LSM-specific boot parameter disabling or a undoing a default-disabled
> CONFIG setting.
> 
> For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
> AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
> result in SELinux being enabled.
> 
> Signed-off-by: Kees Cook 

I don't like this. It brings about conflicting kernel params that are
bound to confuse users. Its pretty easy for a user to understand that
when they specify a parameter manually at boot, that  it overrides the
build time default. But conflicting kernel parameters are a lot harder
to deal with.

I prefer a plain enabled= list being an override of the default build
time value. Where conflicts with LSM-specific configs always result in
the LSM being disabled with a complaint about the conflict.

Though I have yet to be convinced its worth the cost, I do recognize
it is sometimes convenient to disable a single LSM, instead of typing
in a whole list of what to enable. If we have to have conflicting
kernel parameters I would prefer that the conflict throw up a warning
and leaving the LSM with the conflicting config disabled.



> ---
>  .../admin-guide/kernel-parameters.txt | 12 ++
>  security/Kconfig  |  4 +++-
>  security/security.c   | 22 +++
>  3 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index 32d323ee9218..67c90985d2b8 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2276,6 +2276,18 @@
>  
>   lsm.debug   [SECURITY] Enable LSM initialization debugging output.
>  
> + lsm.disable=lsm1,...,lsmN
> + [SECURITY] Comma-separated list of LSMs to disable
> + at boot time. This overrides "lsm.enable=",
> + CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot
> + parameters.
> +
> + lsm.enable=lsm1,...,lsmN
> + [SECURITY] Comma-separated list of LSMs to enable
> + at boot time. This overrides any omissions from
> + CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and
> + boot parameters.
> +
>   machvec=[IA-64] Force the use of a particular machine-vector
>   (machvec) in a generic kernel.
>   Example: machvec=hpzx1_swiotlb
> diff --git a/security/Kconfig b/security/Kconfig
> index 71306b046270..1a82a006cc62 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -282,7 +282,9 @@ config LSM_ENABLE
>   help
> A comma-separate list of LSMs to enable by default at boot. The
> default is "all", to enable all LSM modules at boot. Any LSMs
> -   not listed here will be disabled by default.
> +   not listed here will be disabled by default. This can be
> +   changed with the "lsm.enable=" and "lsm.disable=" boot
> +   parameters.
>  
>  endmenu
>  
> diff --git a/security/security.c b/security/security.c
> index 7ecb9879a863..456a3f73bc36 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -44,6 +44,8 @@ char *lsm_names;
>  /* Boot-time LSM user choice */
>  static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
>   CONFIG_DEFAULT_SECURITY;
> +static __initdata const char *chosen_lsm_enable;
> +static __initdata const char *chosen_lsm_disable;
>  
>  static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
>  
> @@ -185,6 +187,10 @@ static void __init prepare_lsm_enable(void)
>  {
>   /* Prepare defaults. */
>   parse_lsm_enable(builtin_lsm_enable, default_enabled, true);
> +
> + /* Process "lsm.enable=" and "lsm.disable=", if given. */
> + parse_lsm_enable(chosen_lsm_enable, set_enabled, true);
> + parse_lsm_enable(chosen_lsm_disable, set_enabled, false);
>  }
>  
>  /**
> @@ -240,6 +246,22 @@ static int __init enable_debug(char *str)
>  }
>  __setup("lsm.debug", enable_debug);
>  
> +/* Explicitly enable a list of LSMs. */
> +static int __init enable_lsm(char *str)
> +{
> + chosen_lsm_enable = str;
> + return 1;
> +}
> +__setup("lsm.enable=", enable_lsm);
> +
> +/* Explicitly disable a list of LS

Re: [PATCH security-next v3 19/29] LSM: Prepare for reorganizing "security=" logic

[John Johansen]

On 09/24/2018 05:18 PM, Kees Cook wrote:
> This moves the string handling for "security=" boot parameter into
> a stored pointer instead of a string duplicate. This will allow
> easier handling of the string when switching logic to use the coming
> enable/disable infrastructure.
> 
> Signed-off-by: Kees Cook 

Reviewed-by: John Johansen 


> ---
>  security/security.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/security/security.c b/security/security.c
> index 456a3f73bc36..e325fcc41f00 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -34,18 +34,14 @@
>  
>  #define MAX_LSM_EVM_XATTR2
>  
> -/* Maximum number of letters for an LSM name string */
> -#define SECURITY_NAME_MAX10
> -
>  struct security_hook_heads security_hook_heads __lsm_ro_after_init;
>  static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
>  
>  char *lsm_names;
>  /* Boot-time LSM user choice */
> -static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
> - CONFIG_DEFAULT_SECURITY;
>  static __initdata const char *chosen_lsm_enable;
>  static __initdata const char *chosen_lsm_disable;
> +static __initdata const char *chosen_major_lsm;
>  
>  static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
>  
> @@ -112,7 +108,7 @@ static bool __init lsm_allowed(struct lsm_info *lsm)
>   return true;
>  
>   /* Disabled if this LSM isn't the chosen one. */
> - if (strcmp(lsm->name, chosen_lsm) != 0)
> + if (strcmp(lsm->name, chosen_major_lsm) != 0)
>   return false;
>  
>   return true;
> @@ -191,6 +187,9 @@ static void __init prepare_lsm_enable(void)
>   /* Process "lsm.enable=" and "lsm.disable=", if given. */
>   parse_lsm_enable(chosen_lsm_enable, set_enabled, true);
>   parse_lsm_enable(chosen_lsm_disable, set_enabled, false);
> +
> + if (!chosen_major_lsm)
> + chosen_major_lsm = CONFIG_DEFAULT_SECURITY;
>  }
>  
>  /**
> @@ -231,12 +230,12 @@ int __init security_init(void)
>  }
>  
>  /* Save user chosen LSM */
> -static int __init choose_lsm(char *str)
> +static int __init choose_major_lsm(char *str)
>  {
> - strncpy(chosen_lsm, str, SECURITY_NAME_MAX);
> + chosen_major_lsm = str;
>   return 1;
>  }
> -__setup("security=", choose_lsm);
> +__setup("security=", choose_major_lsm);
>  
>  /* Enable LSM order debugging. */
>  static int __init enable_debug(char *str)
> 

Re: [PATCH security-next v3 14/29] LSM: Plumb visibility into optional "enabled" state

[James Morris]

On Mon, 24 Sep 2018, Kees Cook wrote:

> In preparation for lifting the "is this LSM enabled?" logic out of the
> individual LSMs, pass in any special enabled state tracking (as needed
> for SELinux, AppArmor, and LoadPin). This should be an "int" to include
> handling any future cases where "enabled" is exposed via sysctl which
> has no "bool" type.
> 
> Signed-off-by: Kees Cook 
> ---
>  include/linux/lsm_hooks.h | 1 +
>  security/apparmor/lsm.c   | 5 +++--
>  security/selinux/hooks.c  | 1 +
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 5056f7374b3d..2a41e8e6f6e5 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct 
> security_hook_list *hooks, int count,
>  struct lsm_info {
>   const char *name;   /* Populated automatically. */
>   unsigned long flags;/* Optional: flags describing LSM */
> + int *enabled;   /* Optional: NULL means enabled. */

This seems potentially confusing.

Perhaps initialize 'enabled' to a default int pointer, like:

static int lsm_default_enabled = 1;

Then,

DEFINE_LSM(foobar)
flags = LSM_FLAG_LEGACY_MAJOR,
.enabled = &lsm_default_enabled,
.init = foobar_init,
END_LSM;



>   int (*init)(void);
>  };
>  
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 4c5f63e9aeba..d03133a267f2 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1303,8 +1303,8 @@ bool aa_g_paranoid_load = true;
>  module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
>  
>  /* Boot time disable flag */
> -static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
> -module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
> +static int apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
> +module_param_named(enabled, apparmor_enabled, int, 0444);
>  
>  static int __init apparmor_enabled_setup(char *str)
>  {
> @@ -1608,5 +1608,6 @@ static int __init apparmor_init(void)
>  
>  DEFINE_LSM(apparmor)
>   .flags = LSM_FLAG_LEGACY_MAJOR,
> + .enabled = &apparmor_enabled,
>   .init = apparmor_init,
>  END_LSM;
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 615cf6498c0f..3f999ed98cfd 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7204,6 +7204,7 @@ void selinux_complete_init(void)
> all processes and objects when they are created. */
>  DEFINE_LSM(selinux)
>   .flags = LSM_FLAG_LEGACY_MAJOR,
> + .enabled = &selinux_enabled,
>   .init = selinux_init,
>  END_LSM;
>  
> 

-- 
James Morris

Re: [PATCH security-next v3 14/29] LSM: Plumb visibility into optional "enabled" state

[Kees Cook]

On Mon, Oct 1, 2018 at 2:47 PM, James Morris  wrote:
> On Mon, 24 Sep 2018, Kees Cook wrote:
>
>> In preparation for lifting the "is this LSM enabled?" logic out of the
>> individual LSMs, pass in any special enabled state tracking (as needed
>> for SELinux, AppArmor, and LoadPin). This should be an "int" to include
>> handling any future cases where "enabled" is exposed via sysctl which
>> has no "bool" type.
>>
>> Signed-off-by: Kees Cook 
>> ---
>>  include/linux/lsm_hooks.h | 1 +
>>  security/apparmor/lsm.c   | 5 +++--
>>  security/selinux/hooks.c  | 1 +
>>  3 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 5056f7374b3d..2a41e8e6f6e5 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct 
>> security_hook_list *hooks, int count,
>>  struct lsm_info {
>>   const char *name;   /* Populated automatically. */
>>   unsigned long flags;/* Optional: flags describing LSM */
>> + int *enabled;   /* Optional: NULL means enabled. */
>
> This seems potentially confusing.
>
> Perhaps initialize 'enabled' to a default int pointer, like:
>
> static int lsm_default_enabled = 1;
>
> Then,
>
> DEFINE_LSM(foobar)
> flags = LSM_FLAG_LEGACY_MAJOR,
> .enabled = &lsm_default_enabled,
> .init = foobar_init,
> END_LSM;

The reason I didn't do this is because there are only two LSMs that
expose this "enabled" variable, so I didn't like making the other LSMs
have to declare this. Internally, though, this is exactly what the
infrastructure does: if it finds a NULL, it aims it at
&lsm_default_enabled (in a later patch).

However, it seems more discussion is needed on the "enable" bit of
this, so I'll reply to John in a moment...

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH security-next v3 12/29] LSM: Provide separate ordered initialization

[Kees Cook]

On Mon, Oct 1, 2018 at 2:17 PM, John Johansen
 wrote:
> On 09/24/2018 05:18 PM, Kees Cook wrote:
>> This provides a place for ordered LSMs to be initialized, separate from
>> the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
>> ordered_lsm_init(), but it will change drastically in later patches.
>>
>> What is not obvious in the patch is that this change moves the integrity
>> LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
>> with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
>> list, there is no reordering yet created.
>>
>> Signed-off-by: Kees Cook 
>
> I know its already being done, but I don't like splitting the init
> order

Can you describe what you mean here? Do you mean having two init
functions? This is only done temporarily while the other pieces are
reorganized. The later patches reintegrate this. (Before this series,
we effectively had three implicit init paths: minor, major, and
integrity, so even this patch "alone" is an improvement IMO.)

Thanks for the reviews!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH security-next v3 14/29] LSM: Plumb visibility into optional "enabled" state

[John Johansen]

On 10/01/2018 02:56 PM, Kees Cook wrote:
> On Mon, Oct 1, 2018 at 2:47 PM, James Morris  wrote:
>> On Mon, 24 Sep 2018, Kees Cook wrote:
>>
>>> In preparation for lifting the "is this LSM enabled?" logic out of the
>>> individual LSMs, pass in any special enabled state tracking (as needed
>>> for SELinux, AppArmor, and LoadPin). This should be an "int" to include
>>> handling any future cases where "enabled" is exposed via sysctl which
>>> has no "bool" type.
>>>
>>> Signed-off-by: Kees Cook 
>>> ---
>>>  include/linux/lsm_hooks.h | 1 +
>>>  security/apparmor/lsm.c   | 5 +++--
>>>  security/selinux/hooks.c  | 1 +
>>>  3 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>>> index 5056f7374b3d..2a41e8e6f6e5 100644
>>> --- a/include/linux/lsm_hooks.h
>>> +++ b/include/linux/lsm_hooks.h
>>> @@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct 
>>> security_hook_list *hooks, int count,
>>>  struct lsm_info {
>>>   const char *name;   /* Populated automatically. */
>>>   unsigned long flags;/* Optional: flags describing LSM */
>>> + int *enabled;   /* Optional: NULL means enabled. */
>>
>> This seems potentially confusing.
>>
>> Perhaps initialize 'enabled' to a default int pointer, like:
>>
>> static int lsm_default_enabled = 1;
>>
>> Then,
>>
>> DEFINE_LSM(foobar)
>> flags = LSM_FLAG_LEGACY_MAJOR,
>> .enabled = &lsm_default_enabled,
>> .init = foobar_init,
>> END_LSM;
> 
> The reason I didn't do this is because there are only two LSMs that
> expose this "enabled" variable, so I didn't like making the other LSMs
> have to declare this. Internally, though, this is exactly what the
> infrastructure does: if it finds a NULL, it aims it at
> &lsm_default_enabled (in a later patch).
> 
> However, it seems more discussion is needed on the "enable" bit of
> this, so I'll reply to John in a moment...
> 
fwiw the apparmor.enabled config is really only a meant to be used to
disable apparmor. I'd drop it entirely except its part of the userspace
api now and needs to show up in

  /sys/module/apparmor/parameters/enabled

Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable=

[Kees Cook]

On Mon, Oct 1, 2018 at 2:46 PM, John Johansen
 wrote:
> On 09/24/2018 05:18 PM, Kees Cook wrote:
>> This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
>> which each can contain a comma-separated list of LSMs to enable or
>> disable, respectively. The string "all" matches all LSMs.
>>
>> This has very similar functionality to the existing per-LSM enable
>> handling ("apparmor.enabled=...", etc), but provides a centralized
>> place to perform the changes. These parameters take precedent over any
>> LSM-specific boot parameters.
>>
>> Disabling an LSM means it will not be considered when performing
>> initializations. Enabling an LSM means either undoing a previous
>> LSM-specific boot parameter disabling or a undoing a default-disabled
>> CONFIG setting.
>>
>> For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
>> AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
>> result in SELinux being enabled.
>>
>> Signed-off-by: Kees Cook 
>
> I don't like this. It brings about conflicting kernel params that are
> bound to confuse users. Its pretty easy for a user to understand that
> when they specify a parameter manually at boot, that  it overrides the
> build time default. But conflicting kernel parameters are a lot harder
> to deal with.
>
> I prefer a plain enabled= list being an override of the default build
> time value. Where conflicts with LSM-specific configs always result in
> the LSM being disabled with a complaint about the conflict.
>
> Though I have yet to be convinced its worth the cost, I do recognize
> it is sometimes convenient to disable a single LSM, instead of typing
> in a whole list of what to enable. If we have to have conflicting
> kernel parameters I would prefer that the conflict throw up a warning
> and leaving the LSM with the conflicting config disabled.

Alright, let's drill down a bit more. I thought I had all the
requirements sorted out here. :)

AppArmor and SELinux are "special" here in that they have both:

- CONFIG for enable-ness
- boot param for enable-ness

Now, the way this worked in the past was that combined with
CONFIG_DEFAULT_SECURITY and the link-time ordering, this resulted in a
way to get the LSM enabled, skipped, etc. But it was highly CONFIG
dependent.

SELinux does:

#ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM
int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE;

static int __init selinux_enabled_setup(char *str)
{
unsigned long enabled;
if (!kstrtoul(str, 0, &enabled))
selinux_enabled = enabled ? 1 : 0;
return 1;
}
__setup("selinux=", selinux_enabled_setup);
#else
int selinux_enabled = 1;
#endif
...
if (!security_module_enable("selinux")) {
selinux_enabled = 0;
return 0;
}

if (!selinux_enabled) {
pr_info("SELinux:  Disabled at boot.\n");
return 0;
}


AppArmor does:

/* Boot time disable flag */
static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);

static int __init apparmor_enabled_setup(char *str)
{
unsigned long enabled;
int error = kstrtoul(str, 0, &enabled);
if (!error)
apparmor_enabled = enabled ? 1 : 0;
return 1;
}

__setup("apparmor=", apparmor_enabled_setup);
...
if (!apparmor_enabled || !security_module_enable("apparmor")) {
aa_info_message("AppArmor disabled by boot time parameter");
apparmor_enabled = false;
return 0;
}


Smack and TOMOYO each do:

if (!security_module_enable("smack"))
return 0;

if (!security_module_enable("tomoyo"))
return 0;


Capability, Integrity, Yama, and LoadPin always run init. (This series
fixes LoadPin to separate enable vs enforce, so we can ignore its
"enable" setting, which isn't an "am I active?" boolean -- its init
was always run.) With the enable logic is lifted out of the LSMs, we
want to have "implicit enable" for 6 of 8 of the LSMs. (Which is why I
had originally suggested CONFIG_LSM_DISABLE, since the normal state is
enabled.) But given your feedback, I made this "implicit disable" and
added CONFIG_LSM_ENABLE instead. (For which "CONFIG_LSM_ENABLE=all"
gets the same results.)


I think, then, the first question (mainly for you and Paul) is:

Should we remove CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE and
CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE in favor of only
CONFIG_LSM_ENABLE?

The answer will affect the next question: what should be done with the
boot parameters? AppArmor has two ways to change enablement:
apparmor=0/1 and apparmor.enabled=0/1. SELinux just has selinux=0/1.
Should those be removed in favor of "lsm.enable=..."? (And if they're
not removed, how do people imagine they should interact?)

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH security-next v3 14/29] LSM: Plumb visibility into optional "enabled" state

[Kees Cook]

On Mon, Oct 1, 2018 at 3:20 PM, John Johansen
 wrote:
> On 10/01/2018 02:56 PM, Kees Cook wrote:
>> On Mon, Oct 1, 2018 at 2:47 PM, James Morris  wrote:
>>> On Mon, 24 Sep 2018, Kees Cook wrote:
>>>
 In preparation for lifting the "is this LSM enabled?" logic out of the
 individual LSMs, pass in any special enabled state tracking (as needed
 for SELinux, AppArmor, and LoadPin). This should be an "int" to include
 handling any future cases where "enabled" is exposed via sysctl which
 has no "bool" type.

 Signed-off-by: Kees Cook 
 ---
  include/linux/lsm_hooks.h | 1 +
  security/apparmor/lsm.c   | 5 +++--
  security/selinux/hooks.c  | 1 +
  3 files changed, 5 insertions(+), 2 deletions(-)

 diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
 index 5056f7374b3d..2a41e8e6f6e5 100644
 --- a/include/linux/lsm_hooks.h
 +++ b/include/linux/lsm_hooks.h
 @@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct 
 security_hook_list *hooks, int count,
  struct lsm_info {
   const char *name;   /* Populated automatically. */
   unsigned long flags;/* Optional: flags describing LSM */
 + int *enabled;   /* Optional: NULL means enabled. */
>>>
>>> This seems potentially confusing.
>>>
>>> Perhaps initialize 'enabled' to a default int pointer, like:
>>>
>>> static int lsm_default_enabled = 1;
>>>
>>> Then,
>>>
>>> DEFINE_LSM(foobar)
>>> flags = LSM_FLAG_LEGACY_MAJOR,
>>> .enabled = &lsm_default_enabled,
>>> .init = foobar_init,
>>> END_LSM;
>>
>> The reason I didn't do this is because there are only two LSMs that
>> expose this "enabled" variable, so I didn't like making the other LSMs
>> have to declare this. Internally, though, this is exactly what the
>> infrastructure does: if it finds a NULL, it aims it at
>> &lsm_default_enabled (in a later patch).
>>
>> However, it seems more discussion is needed on the "enable" bit of
>> this, so I'll reply to John in a moment...
>>
> fwiw the apparmor.enabled config is really only a meant to be used to
> disable apparmor. I'd drop it entirely except its part of the userspace
> api now and needs to show up in
>
>   /sys/module/apparmor/parameters/enabled

Showing the enabled-ness there can be wired up. What should happen if
someone sets apparmor.enabled=0/1 in new-series-world? (See other
thread...)

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH security-next v3 04/29] LSM: Remove initcall tracing

[Kees Cook]

On Mon, Oct 1, 2018 at 2:23 PM, Steven Rostedt  wrote:
> On Mon, 1 Oct 2018 14:07:55 -0700
> John Johansen  wrote:
>
>> On 09/24/2018 05:18 PM, Kees Cook wrote:
>> > This partially reverts commit 58eacfffc417 ("init, tracing: instrument
>> > security and console initcall trace events") since security init calls
>> > are about to no longer resemble regular init calls.
>> >
>> > Cc: James Morris 
>> > Cc: "Serge E. Hallyn" 
>> > Cc: Abderrahmane Benbachir 
>> > Cc: Steven Rostedt (VMware) 
>> > Cc: linux-security-mod...@vger.kernel.org
>> > Signed-off-by: Kees Cook 
>>
>>
>> Reviewed-by: John Johansen 
>>
>> though I do think it would be a good idea to add a new set
>> of trace points, but that can come as a separate patch
>>
>>
>
> Agreed.

BTW, how would this look? I'm not familiar with adding new tracepoints...

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable=

[John Johansen]

On 10/01/2018 03:27 PM, Kees Cook wrote:
> On Mon, Oct 1, 2018 at 2:46 PM, John Johansen
>  wrote:
>> On 09/24/2018 05:18 PM, Kees Cook wrote:
>>> This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
>>> which each can contain a comma-separated list of LSMs to enable or
>>> disable, respectively. The string "all" matches all LSMs.
>>>
>>> This has very similar functionality to the existing per-LSM enable
>>> handling ("apparmor.enabled=...", etc), but provides a centralized
>>> place to perform the changes. These parameters take precedent over any
>>> LSM-specific boot parameters.
>>>
>>> Disabling an LSM means it will not be considered when performing
>>> initializations. Enabling an LSM means either undoing a previous
>>> LSM-specific boot parameter disabling or a undoing a default-disabled
>>> CONFIG setting.
>>>
>>> For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
>>> AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
>>> result in SELinux being enabled.
>>>
>>> Signed-off-by: Kees Cook 
>>
>> I don't like this. It brings about conflicting kernel params that are
>> bound to confuse users. Its pretty easy for a user to understand that
>> when they specify a parameter manually at boot, that  it overrides the
>> build time default. But conflicting kernel parameters are a lot harder
>> to deal with.
>>
>> I prefer a plain enabled= list being an override of the default build
>> time value. Where conflicts with LSM-specific configs always result in
>> the LSM being disabled with a complaint about the conflict.
>>
>> Though I have yet to be convinced its worth the cost, I do recognize
>> it is sometimes convenient to disable a single LSM, instead of typing
>> in a whole list of what to enable. If we have to have conflicting
>> kernel parameters I would prefer that the conflict throw up a warning
>> and leaving the LSM with the conflicting config disabled.
> 
> Alright, let's drill down a bit more. I thought I had all the
> requirements sorted out here. :)
> 
> AppArmor and SELinux are "special" here in that they have both:
> 
> - CONFIG for enable-ness
> - boot param for enable-ness
> 
> Now, the way this worked in the past was that combined with
> CONFIG_DEFAULT_SECURITY and the link-time ordering, this resulted in a
> way to get the LSM enabled, skipped, etc. But it was highly CONFIG
> dependent.
> 
> SELinux does:
> 
> #ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM
> int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE;
> 
> static int __init selinux_enabled_setup(char *str)
> {
> unsigned long enabled;
> if (!kstrtoul(str, 0, &enabled))
> selinux_enabled = enabled ? 1 : 0;
> return 1;
> }
> __setup("selinux=", selinux_enabled_setup);
> #else
> int selinux_enabled = 1;
> #endif
> ...
> if (!security_module_enable("selinux")) {
> selinux_enabled = 0;
> return 0;
> }
> 
> if (!selinux_enabled) {
> pr_info("SELinux:  Disabled at boot.\n");
> return 0;
> }
> 
> 
> AppArmor does:
> 
> /* Boot time disable flag */
> static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
> module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
> 
> static int __init apparmor_enabled_setup(char *str)
> {
> unsigned long enabled;
> int error = kstrtoul(str, 0, &enabled);
> if (!error)
> apparmor_enabled = enabled ? 1 : 0;
> return 1;
> }
> 
> __setup("apparmor=", apparmor_enabled_setup);
> ...
> if (!apparmor_enabled || !security_module_enable("apparmor")) {
> aa_info_message("AppArmor disabled by boot time parameter");
> apparmor_enabled = false;
> return 0;
> }
> 
> 
> Smack and TOMOYO each do:
> 
> if (!security_module_enable("smack"))
> return 0;
> 
> if (!security_module_enable("tomoyo"))
> return 0;
> 
> 
> Capability, Integrity, Yama, and LoadPin always run init. (This series
> fixes LoadPin to separate enable vs enforce, so we can ignore its
> "enable" setting, which isn't an "am I active?" boolean -- its init
> was always run.) With the enable logic is lifted out of the LSMs, we
> want to have "implicit enable" for 6 of 8 of the LSMs. (Which is why I
> had originally suggested CONFIG_LSM_DISABLE, since the normal state is
> enabled.) But given your feedback, I made this "implicit disable" and
> added CONFIG_LSM_ENABLE instead. (For which "CONFIG_LSM_ENABLE=all"
> gets the same results.)
> 
> 
> I think, then, the first question (mainly for you and Paul) is:
> 
> Should we remove CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE and
> CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE in favor of only
> CONFIG_LSM_ENABLE?
> 

We can remove the Kconfig for the apparmor bootparam value. In fact I
will attach that patch below. I can't get rid of the parameter as it
is 

Re: [PATCH security-next v3 14/29] LSM: Plumb visibility into optional "enabled" state

[John Johansen]

On 10/01/2018 03:29 PM, Kees Cook wrote:
> On Mon, Oct 1, 2018 at 3:20 PM, John Johansen
>  wrote:
>> On 10/01/2018 02:56 PM, Kees Cook wrote:
>>> On Mon, Oct 1, 2018 at 2:47 PM, James Morris  wrote:
 On Mon, 24 Sep 2018, Kees Cook wrote:

> In preparation for lifting the "is this LSM enabled?" logic out of the
> individual LSMs, pass in any special enabled state tracking (as needed
> for SELinux, AppArmor, and LoadPin). This should be an "int" to include
> handling any future cases where "enabled" is exposed via sysctl which
> has no "bool" type.
>
> Signed-off-by: Kees Cook 
> ---
>  include/linux/lsm_hooks.h | 1 +
>  security/apparmor/lsm.c   | 5 +++--
>  security/selinux/hooks.c  | 1 +
>  3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 5056f7374b3d..2a41e8e6f6e5 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct 
> security_hook_list *hooks, int count,
>  struct lsm_info {
>   const char *name;   /* Populated automatically. */
>   unsigned long flags;/* Optional: flags describing LSM */
> + int *enabled;   /* Optional: NULL means enabled. */

 This seems potentially confusing.

 Perhaps initialize 'enabled' to a default int pointer, like:

 static int lsm_default_enabled = 1;

 Then,

 DEFINE_LSM(foobar)
 flags = LSM_FLAG_LEGACY_MAJOR,
 .enabled = &lsm_default_enabled,
 .init = foobar_init,
 END_LSM;
>>>
>>> The reason I didn't do this is because there are only two LSMs that
>>> expose this "enabled" variable, so I didn't like making the other LSMs
>>> have to declare this. Internally, though, this is exactly what the
>>> infrastructure does: if it finds a NULL, it aims it at
>>> &lsm_default_enabled (in a later patch).
>>>
>>> However, it seems more discussion is needed on the "enable" bit of
>>> this, so I'll reply to John in a moment...
>>>
>> fwiw the apparmor.enabled config is really only a meant to be used to
>> disable apparmor. I'd drop it entirely except its part of the userspace
>> api now and needs to show up in
>>
>>   /sys/module/apparmor/parameters/enabled
> 
> Showing the enabled-ness there can be wired up. What should happen if
> someone sets apparmor.enabled=0/1 in new-series-world? (See other
> thread...)
> 
I am open to either just making apparmor=0/apparmor.enabled=0 a means
of only disabling apparmor, thats how it is currently used. Or even
potentially getting rid of it as an available kernel boot config
parameter and running with just lsm.enabled/disabled.

The important bit that applications are relying on is having
  /sys/module/apparmor/parameters/enabled

set to the the correct value.

Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable=

[Kees Cook]

On Mon, Oct 1, 2018 at 3:48 PM, John Johansen
 wrote:
> On 10/01/2018 03:27 PM, Kees Cook wrote:
>> On Mon, Oct 1, 2018 at 2:46 PM, John Johansen
>>  wrote:
>>> On 09/24/2018 05:18 PM, Kees Cook wrote:
 This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
 which each can contain a comma-separated list of LSMs to enable or
 disable, respectively. The string "all" matches all LSMs.

 This has very similar functionality to the existing per-LSM enable
 handling ("apparmor.enabled=...", etc), but provides a centralized
 place to perform the changes. These parameters take precedent over any
 LSM-specific boot parameters.

 Disabling an LSM means it will not be considered when performing
 initializations. Enabling an LSM means either undoing a previous
 LSM-specific boot parameter disabling or a undoing a default-disabled
 CONFIG setting.

 For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
 AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
 result in SELinux being enabled.

 Signed-off-by: Kees Cook 
>>>
>>> I don't like this. It brings about conflicting kernel params that are
>>> bound to confuse users. Its pretty easy for a user to understand that
>>> when they specify a parameter manually at boot, that  it overrides the
>>> build time default. But conflicting kernel parameters are a lot harder
>>> to deal with.
>>>
>>> I prefer a plain enabled= list being an override of the default build
>>> time value. Where conflicts with LSM-specific configs always result in
>>> the LSM being disabled with a complaint about the conflict.
>>>
>>> Though I have yet to be convinced its worth the cost, I do recognize
>>> it is sometimes convenient to disable a single LSM, instead of typing
>>> in a whole list of what to enable. If we have to have conflicting
>>> kernel parameters I would prefer that the conflict throw up a warning
>>> and leaving the LSM with the conflicting config disabled.
>>
>> Alright, let's drill down a bit more. I thought I had all the
>> requirements sorted out here. :)
>>
>> AppArmor and SELinux are "special" here in that they have both:
>>
>> - CONFIG for enable-ness
>> - boot param for enable-ness
>>
>> Now, the way this worked in the past was that combined with
>> CONFIG_DEFAULT_SECURITY and the link-time ordering, this resulted in a
>> way to get the LSM enabled, skipped, etc. But it was highly CONFIG
>> dependent.
>>
>> SELinux does:
>>
>> #ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM
>> int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE;
>>
>> static int __init selinux_enabled_setup(char *str)
>> {
>> unsigned long enabled;
>> if (!kstrtoul(str, 0, &enabled))
>> selinux_enabled = enabled ? 1 : 0;
>> return 1;
>> }
>> __setup("selinux=", selinux_enabled_setup);
>> #else
>> int selinux_enabled = 1;
>> #endif
>> ...
>> if (!security_module_enable("selinux")) {
>> selinux_enabled = 0;
>> return 0;
>> }
>>
>> if (!selinux_enabled) {
>> pr_info("SELinux:  Disabled at boot.\n");
>> return 0;
>> }
>>
>>
>> AppArmor does:
>>
>> /* Boot time disable flag */
>> static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
>> module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
>>
>> static int __init apparmor_enabled_setup(char *str)
>> {
>> unsigned long enabled;
>> int error = kstrtoul(str, 0, &enabled);
>> if (!error)
>> apparmor_enabled = enabled ? 1 : 0;
>> return 1;
>> }
>>
>> __setup("apparmor=", apparmor_enabled_setup);
>> ...
>> if (!apparmor_enabled || !security_module_enable("apparmor")) {
>> aa_info_message("AppArmor disabled by boot time parameter");
>> apparmor_enabled = false;
>> return 0;
>> }
>>
>>
>> Smack and TOMOYO each do:
>>
>> if (!security_module_enable("smack"))
>> return 0;
>>
>> if (!security_module_enable("tomoyo"))
>> return 0;
>>
>>
>> Capability, Integrity, Yama, and LoadPin always run init. (This series
>> fixes LoadPin to separate enable vs enforce, so we can ignore its
>> "enable" setting, which isn't an "am I active?" boolean -- its init
>> was always run.) With the enable logic is lifted out of the LSMs, we
>> want to have "implicit enable" for 6 of 8 of the LSMs. (Which is why I
>> had originally suggested CONFIG_LSM_DISABLE, since the normal state is
>> enabled.) But given your feedback, I made this "implicit disable" and
>> added CONFIG_LSM_ENABLE instead. (For which "CONFIG_LSM_ENABLE=all"
>> gets the same results.)
>>
>>
>> I think, then, the first question (mainly for you and Paul) is:
>>
>> Should we remove CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE and
>> CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE in favor of only
>> CO

Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable=

[Kees Cook]

On Mon, Oct 1, 2018 at 4:30 PM, Kees Cook  wrote:
> If we keep it, "apparmor=0 lsm_enable=apparmor" would mean it's
> enabled. Is that okay?

Actually, what the v3 series does right now is leaves AppArmor and
SELinux alone -- whatever they configured for enable/disable is left
alone.

The problem I have is when processing CONFIG_LSM_ENABLE ... what do I
do with the existing "enable" flag? It's set by both
CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE and apparmor=0/1.

Right now I can't tell the difference between someone booting with
apparmor=0 or CONFIG_LSM_ENABLE not including apparmor.

i.e. how do I mix CONFIG_LSM_ENABLE with apparmor=0/1? (assuming
CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE has been removed)

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable=

[John Johansen]

On 10/01/2018 04:30 PM, Kees Cook wrote:
> On Mon, Oct 1, 2018 at 3:48 PM, John Johansen
>  wrote:
>> On 10/01/2018 03:27 PM, Kees Cook wrote:
>>> On Mon, Oct 1, 2018 at 2:46 PM, John Johansen
>>>  wrote:
 On 09/24/2018 05:18 PM, Kees Cook wrote:
> This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
> which each can contain a comma-separated list of LSMs to enable or
> disable, respectively. The string "all" matches all LSMs.
>
> This has very similar functionality to the existing per-LSM enable
> handling ("apparmor.enabled=...", etc), but provides a centralized
> place to perform the changes. These parameters take precedent over any
> LSM-specific boot parameters.
>
> Disabling an LSM means it will not be considered when performing
> initializations. Enabling an LSM means either undoing a previous
> LSM-specific boot parameter disabling or a undoing a default-disabled
> CONFIG setting.
>
> For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
> AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
> result in SELinux being enabled.
>
> Signed-off-by: Kees Cook 

 I don't like this. It brings about conflicting kernel params that are
 bound to confuse users. Its pretty easy for a user to understand that
 when they specify a parameter manually at boot, that  it overrides the
 build time default. But conflicting kernel parameters are a lot harder
 to deal with.

 I prefer a plain enabled= list being an override of the default build
 time value. Where conflicts with LSM-specific configs always result in
 the LSM being disabled with a complaint about the conflict.

 Though I have yet to be convinced its worth the cost, I do recognize
 it is sometimes convenient to disable a single LSM, instead of typing
 in a whole list of what to enable. If we have to have conflicting
 kernel parameters I would prefer that the conflict throw up a warning
 and leaving the LSM with the conflicting config disabled.
>>>
>>> Alright, let's drill down a bit more. I thought I had all the
>>> requirements sorted out here. :)
>>>
>>> AppArmor and SELinux are "special" here in that they have both:
>>>
>>> - CONFIG for enable-ness
>>> - boot param for enable-ness
>>>
>>> Now, the way this worked in the past was that combined with
>>> CONFIG_DEFAULT_SECURITY and the link-time ordering, this resulted in a
>>> way to get the LSM enabled, skipped, etc. But it was highly CONFIG
>>> dependent.
>>>
>>> SELinux does:
>>>
>>> #ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM
>>> int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE;
>>>
>>> static int __init selinux_enabled_setup(char *str)
>>> {
>>> unsigned long enabled;
>>> if (!kstrtoul(str, 0, &enabled))
>>> selinux_enabled = enabled ? 1 : 0;
>>> return 1;
>>> }
>>> __setup("selinux=", selinux_enabled_setup);
>>> #else
>>> int selinux_enabled = 1;
>>> #endif
>>> ...
>>> if (!security_module_enable("selinux")) {
>>> selinux_enabled = 0;
>>> return 0;
>>> }
>>>
>>> if (!selinux_enabled) {
>>> pr_info("SELinux:  Disabled at boot.\n");
>>> return 0;
>>> }
>>>
>>>
>>> AppArmor does:
>>>
>>> /* Boot time disable flag */
>>> static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
>>> module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
>>>
>>> static int __init apparmor_enabled_setup(char *str)
>>> {
>>> unsigned long enabled;
>>> int error = kstrtoul(str, 0, &enabled);
>>> if (!error)
>>> apparmor_enabled = enabled ? 1 : 0;
>>> return 1;
>>> }
>>>
>>> __setup("apparmor=", apparmor_enabled_setup);
>>> ...
>>> if (!apparmor_enabled || !security_module_enable("apparmor")) {
>>> aa_info_message("AppArmor disabled by boot time parameter");
>>> apparmor_enabled = false;
>>> return 0;
>>> }
>>>
>>>
>>> Smack and TOMOYO each do:
>>>
>>> if (!security_module_enable("smack"))
>>> return 0;
>>>
>>> if (!security_module_enable("tomoyo"))
>>> return 0;
>>>
>>>
>>> Capability, Integrity, Yama, and LoadPin always run init. (This series
>>> fixes LoadPin to separate enable vs enforce, so we can ignore its
>>> "enable" setting, which isn't an "am I active?" boolean -- its init
>>> was always run.) With the enable logic is lifted out of the LSMs, we
>>> want to have "implicit enable" for 6 of 8 of the LSMs. (Which is why I
>>> had originally suggested CONFIG_LSM_DISABLE, since the normal state is
>>> enabled.) But given your feedback, I made this "implicit disable" and
>>> added CONFIG_LSM_ENABLE instead. (For which "CONFIG_LSM_ENABLE=all"
>>> gets the same results.)
>>>
>>>
>>> I think, then, the first q

Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable=

[Kees Cook]

On Mon, Oct 1, 2018 at 4:44 PM, John Johansen
 wrote:
> On 10/01/2018 04:30 PM, Kees Cook wrote:
>> If we keep it, "apparmor=0 lsm_enable=apparmor" would mean it's
>> enabled. Is that okay?
>>
> ugh I would rather get rid of apparmor=0 or to emit a warning with apparmor
> disabled, but if we have to live with it then yes I can live with last
> option wins

Removing it would be much preferred! :)

Assuming Paul is okay with the same results in SELinux, I'll prepare patches...

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable=

[John Johansen]

On 10/01/2018 04:38 PM, Kees Cook wrote:
> On Mon, Oct 1, 2018 at 4:30 PM, Kees Cook  wrote:
>> If we keep it, "apparmor=0 lsm_enable=apparmor" would mean it's
>> enabled. Is that okay?
> 
> Actually, what the v3 series does right now is leaves AppArmor and
> SELinux alone -- whatever they configured for enable/disable is left
> alone.
> 
> The problem I have is when processing CONFIG_LSM_ENABLE ... what do I
> do with the existing "enable" flag? It's set by both
> CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE and apparmor=0/1.
> 
> Right now I can't tell the difference between someone booting with
> apparmor=0 or CONFIG_LSM_ENABLE not including apparmor.
> 
> i.e. how do I mix CONFIG_LSM_ENABLE with apparmor=0/1? (assuming
> CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE has been removed)
> 
right, Its a mess and confusing not just us but for the user. I think
it is worth considering removing the apparmor= and apparmor.enabled
options so that we can clean this up.

If we do keep it, there are three options
1. last option wins (above)
2. add more states so we can track the different combination (more
  complex and probably not worth it)
3. we ignore any apparmor=1 (he default state) and only look at
  whether apparmor.enabled has been toggled to 0 during setup. 
  At which point it stays that way.

if we keep it, I think an explicit disable should win, so
option 3, but I can live with 1.

[PATCH security-next v4 14/32] LSM: Plumb visibility into optional "enabled" state

[Kees Cook]

In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 include/linux/lsm_hooks.h | 1 +
 security/apparmor/lsm.c   | 5 +++--
 security/selinux/hooks.c  | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 531e219a49b9..6ec5a0266f21 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct security_hook_list 
*hooks, int count,
 struct lsm_info {
const char *name;   /* Required. */
unsigned long flags;/* Optional: flags describing LSM */
+   int *enabled;   /* Optional: NULL means enabled. */
int (*init)(void);  /* Required. */
 };
 
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 768cb539fb6c..6ace45704cb6 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1303,8 +1303,8 @@ bool aa_g_paranoid_load = true;
 module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
 
 /* Boot time disable flag */
-static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
-module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
+static int apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
+module_param_named(enabled, apparmor_enabled, int, 0444);
 
 static int __init apparmor_enabled_setup(char *str)
 {
@@ -1609,5 +1609,6 @@ static int __init apparmor_init(void)
 DEFINE_LSM(apparmor) = {
.name = "apparmor",
.flags = LSM_FLAG_LEGACY_MAJOR,
+   .enabled = &apparmor_enabled,
.init = apparmor_init,
 };
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 020886895819..e8da99550b67 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7205,6 +7205,7 @@ void selinux_complete_init(void)
 DEFINE_LSM(selinux) = {
.name = "selinux",
.flags = LSM_FLAG_LEGACY_MAJOR,
+   .enabled = &selinux_enabled,
.init = selinux_init,
 };
 
-- 
2.17.1

[PATCH security-next v4 15/32] LSM: Lift LSM selection out of individual LSMs

[Kees Cook]

As a prerequisite to adjusting LSM selection logic in the future, this
moves the selection logic up out of the individual major LSMs, making
their init functions only run when actually enabled. This considers all
LSMs enabled by default unless they specified an external "enable"
variable.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 include/linux/lsm_hooks.h  |  1 -
 security/apparmor/lsm.c|  6 ---
 security/security.c| 84 --
 security/selinux/hooks.c   | 10 -
 security/smack/smack_lsm.c |  3 --
 security/tomoyo/tomoyo.c   |  2 -
 6 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 6ec5a0266f21..9ecb623fb39d 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2085,7 +2085,6 @@ static inline void security_delete_hooks(struct 
security_hook_list *hooks,
 #define __lsm_ro_after_init__ro_after_init
 #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
 
-extern int __init security_module_enable(const char *module);
 extern void __init capability_add_hooks(void);
 #ifdef CONFIG_SECURITY_YAMA
 extern void __init yama_add_hooks(void);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 6ace45704cb6..bc56b058dc75 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1542,12 +1542,6 @@ static int __init apparmor_init(void)
 {
int error;
 
-   if (!apparmor_enabled || !security_module_enable("apparmor")) {
-   aa_info_message("AppArmor disabled by boot time parameter");
-   apparmor_enabled = false;
-   return 0;
-   }
-
aa_secids_init();
 
error = aa_setup_dfa_engine();
diff --git a/security/security.c b/security/security.c
index e672ced5..4e5e67b82b7b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -52,33 +52,78 @@ static __initdata bool debug;
pr_info(__VA_ARGS__);   \
} while (0)
 
+static bool __init is_enabled(struct lsm_info *lsm)
+{
+   if (!lsm->enabled || *lsm->enabled)
+   return true;
+
+   return false;
+}
+
+/* Mark an LSM's enabled flag, if it exists. */
+static void __init set_enabled(struct lsm_info *lsm, bool enabled)
+{
+   if (lsm->enabled)
+   *lsm->enabled = enabled;
+}
+
+/* Is an LSM allowed to be initialized? */
+static bool __init lsm_allowed(struct lsm_info *lsm)
+{
+   /* Skip if the LSM is disabled. */
+   if (!is_enabled(lsm))
+   return false;
+
+   /* Skip major-specific checks if not a major LSM. */
+   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
+   return true;
+
+   /* Disabled if this LSM isn't the chosen one. */
+   if (strcmp(lsm->name, chosen_lsm) != 0)
+   return false;
+
+   return true;
+}
+
+/* Check if LSM should be enabled. Mark any that are disabled. */
+static void __init maybe_initialize_lsm(struct lsm_info *lsm)
+{
+   int enabled = lsm_allowed(lsm);
+
+   /* Record enablement. */
+   set_enabled(lsm, enabled);
+
+   /* If selected, initialize the LSM. */
+   if (enabled) {
+   int ret;
+
+   init_debug("initializing %s\n", lsm->name);
+   ret = lsm->init();
+   WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
+   }
+}
+
 static void __init ordered_lsm_init(void)
 {
struct lsm_info *lsm;
-   int ret;
 
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) != 0)
continue;
 
-   init_debug("initializing %s\n", lsm->name);
-   ret = lsm->init();
-   WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
+   maybe_initialize_lsm(lsm);
}
 }
 
 static void __init major_lsm_init(void)
 {
struct lsm_info *lsm;
-   int ret;
 
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
continue;
 
-   init_debug("initializing %s\n", lsm->name);
-   ret = lsm->init();
-   WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
+   maybe_initialize_lsm(lsm);
}
 }
 
@@ -168,29 +213,6 @@ static int lsm_append(char *new, char **result)
return 0;
 }
 
-/**
- * security_module_enable - Load given security module on boot ?
- * @module: the name of the module
- *
- * Each LSM must pass this method before registering its own operations
- * to avoid security registration races. This method may also be used
- * to check if your LSM is currently loaded during kernel initialization.
- *
- * Returns:
- *
- * true if:
- *
- * - The passed LSM is the one chosen by user at boot time,
- * - or the passed LSM is configured as th

[PATCH security-next v4 04/32] LSM: Remove initcall tracing

[Kees Cook]

This partially reverts commit 58eacfffc417 ("init, tracing: instrument
security and console initcall trace events") since security init calls
are about to no longer resemble regular init calls.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 security/security.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/security/security.c b/security/security.c
index 892fe6b691cf..41a5da2c7faf 100644
--- a/security/security.c
+++ b/security/security.c
@@ -30,8 +30,6 @@
 #include 
 #include 
 
-#include 
-
 #define MAX_LSM_EVM_XATTR  2
 
 /* Maximum number of letters for an LSM name string */
@@ -47,17 +45,13 @@ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
 
 static void __init do_security_initcalls(void)
 {
-   int ret;
initcall_t call;
initcall_entry_t *ce;
 
ce = __start_lsm_info;
-   trace_initcall_level("security");
while (ce < __end_lsm_info) {
call = initcall_from_entry(ce);
-   trace_initcall_start(call);
-   ret = call();
-   trace_initcall_finish(call, ret);
+   call();
ce++;
}
 }
-- 
2.17.1

[PATCH security-next v4 03/32] LSM: Rename .security_initcall section to .lsm_info

[Kees Cook]

In preparation for switching from initcall to just a regular set of
pointers in a section, rename the internal section name.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: James Morris 
Reviewed-by: John Johansen 
---
 include/asm-generic/vmlinux.lds.h | 10 +-
 include/linux/init.h  |  4 ++--
 security/security.c   |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 934a45395547..5079a969e612 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -787,14 +787,14 @@
__con_initcall_end = .;
 
 #define SECURITY_INITCALL  \
-   __security_initcall_start = .;  \
-   KEEP(*(.security_initcall.init))\
-   __security_initcall_end = .;
+   __start_lsm_info = .;   \
+   KEEP(*(.lsm_info.init)) \
+   __end_lsm_info = .;
 
 /* Older linker script style for security init. */
 #define SECURITY_INIT  \
-   .security_initcall.init : AT(ADDR(.security_initcall.init) - 
LOAD_OFFSET) { \
-   SECURITY_INITCALL   \
+   .lsm_info.init : AT(ADDR(.lsm_info.init) - LOAD_OFFSET) {   \
+   LSM_INFO\
}
 
 #ifdef CONFIG_BLK_DEV_INITRD
diff --git a/include/linux/init.h b/include/linux/init.h
index 2538d176dd1f..77636539e77c 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -133,7 +133,7 @@ static inline initcall_t 
initcall_from_entry(initcall_entry_t *entry)
 #endif
 
 extern initcall_entry_t __con_initcall_start[], __con_initcall_end[];
-extern initcall_entry_t __security_initcall_start[], __security_initcall_end[];
+extern initcall_entry_t __start_lsm_info[], __end_lsm_info[];
 
 /* Used for contructor calls. */
 typedef void (*ctor_fn_t)(void);
@@ -236,7 +236,7 @@ extern bool initcall_debug;
static exitcall_t __exitcall_##fn __exit_call = fn
 
 #define console_initcall(fn)   ___define_initcall(fn,, .con_initcall)
-#define security_initcall(fn)  ___define_initcall(fn,, .security_initcall)
+#define security_initcall(fn)  ___define_initcall(fn,, .lsm_info)
 
 struct obs_kernel_param {
const char *str;
diff --git a/security/security.c b/security/security.c
index 4cbcf244a965..892fe6b691cf 100644
--- a/security/security.c
+++ b/security/security.c
@@ -51,9 +51,9 @@ static void __init do_security_initcalls(void)
initcall_t call;
initcall_entry_t *ce;
 
-   ce = __security_initcall_start;
+   ce = __start_lsm_info;
trace_initcall_level("security");
-   while (ce < __security_initcall_end) {
+   while (ce < __end_lsm_info) {
call = initcall_from_entry(ce);
trace_initcall_start(call);
ret = call();
-- 
2.17.1

[PATCH security-next v4 01/32] LSM: Correctly announce start of LSM initialization

[Kees Cook]

For a while now, the LSM core has said it was "initializED", rather than
"initializING". This adjust the report to be more accurate (i.e. before
this was reported before any LSMs had been initialized.)

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: James Morris 
Reviewed-by: John Johansen 
---
 security/security.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/security.c b/security/security.c
index 736e78da1ab9..4cbcf244a965 100644
--- a/security/security.c
+++ b/security/security.c
@@ -72,10 +72,11 @@ int __init security_init(void)
int i;
struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
 
+   pr_info("Security Framework initializing\n");
+
for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
 i++)
INIT_HLIST_HEAD(&list[i]);
-   pr_info("Security Framework initialized\n");
 
/*
 * Load minor LSMs, with the capability module always first.
-- 
2.17.1

[PATCH security-next v4 26/32] LSM: Introduce "lsm.order=" for boottime ordering

[Kees Cook]

Provide a way to reorder LSM initialization using the new "lsm.order="
comma-separated list of LSMs. Any LSMs not listed will be added in builtin
order.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 Documentation/admin-guide/kernel-parameters.txt |  6 ++
 security/security.c | 14 +-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 0d10ab3d020e..7e01b7a1e73d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2286,6 +2286,12 @@
at boot time. This overrides any omissions from
CONFIG_LSM_ENABLE.
 
+   lsm.order=lsm1,...,lsmN
+   [SECURITY] Choose order of enabled LSM
+   initialization. Any builtin LSMs not listed here
+   will be implicitly appended to the list in builtin
+   order.
+
machvec=[IA-64] Force the use of a particular machine-vector
(machvec) in a generic kernel.
Example: machvec=hpzx1_swiotlb
diff --git a/security/security.c b/security/security.c
index 0510bb8e0af0..6fafad44b85e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -44,6 +44,7 @@ char *lsm_names;
 /* Boot-time LSM user choice */
 static __initdata const char *chosen_lsm_enable;
 static __initdata const char *chosen_lsm_disable;
+static __initdata const char *chosen_lsm_order;
 static __initdata const char *chosen_major_lsm;
 
 static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
@@ -152,11 +153,14 @@ static void __init parse_lsm_order(const char *order, 
const char *origin)
kfree(sep);
 }
 
-/* Populate ordered LSMs list from builtin list of LSMs. */
+/* Populate ordered LSMs list from commandline and builtin list of LSMs. */
 static void __init prepare_lsm_order(void)
 {
struct lsm_info *lsm;
 
+   /* Parse order from commandline, if present. */
+   parse_lsm_order(chosen_lsm_order, "cmdline");
+
/* Parse order from builtin list. */
parse_lsm_order(builtin_lsm_order, "builtin");
 
@@ -316,6 +320,14 @@ static int __init choose_major_lsm(char *str)
 }
 __setup("security=", choose_major_lsm);
 
+/* Explicitly choose LSM initialization order. */
+static int __init choose_lsm_order(char *str)
+{
+   chosen_lsm_order = str;
+   return 1;
+}
+__setup("lsm.order=", choose_lsm_order);
+
 /* Enable LSM order debugging. */
 static int __init enable_debug(char *str)
 {
-- 
2.17.1

[PATCH security-next v4 30/32] capability: Initialize as LSM_ORDER_FIRST

[Kees Cook]

This converts capabilities to use the new LSM_ORDER_FIRST position.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h | 2 --
 security/commoncap.c  | 9 -
 security/security.c   | 9 -
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 62bc230826e0..36e7a716fdfe 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2092,6 +2092,4 @@ static inline void security_delete_hooks(struct 
security_hook_list *hooks,
 #define __lsm_ro_after_init__ro_after_init
 #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
 
-extern void __init capability_add_hooks(void);
-
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index 2e489d6a3ac8..c928eb3fe784 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1366,10 +1366,17 @@ struct security_hook_list capability_hooks[] 
__lsm_ro_after_init = {
LSM_HOOK_INIT(vm_enough_memory, cap_vm_enough_memory),
 };
 
-void __init capability_add_hooks(void)
+static int __init capability_init(void)
 {
security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks),
"capability");
+   return 0;
 }
 
+DEFINE_LSM(capability) = {
+   .name = "capability",
+   .order = LSM_ORDER_FIRST,
+   .init = capability_init,
+};
+
 #endif /* CONFIG_SECURITY */
diff --git a/security/security.c b/security/security.c
index dac379518e60..813dab3b5b97 100644
--- a/security/security.c
+++ b/security/security.c
@@ -62,6 +62,10 @@ static __initdata bool debug;
 
 static bool __init is_enabled(struct lsm_info *lsm)
 {
+   /* LSM_ORDER_FIRST is always enabled. */
+   if (lsm->order == LSM_ORDER_FIRST)
+   return true;
+
if (WARN_ON(!lsm->enabled))
return false;
 
@@ -306,11 +310,6 @@ int __init security_init(void)
/* Figure out which LSMs are enabled and disabled. */
prepare_lsm_enable();
 
-   /*
-* Load minor LSMs, with the capability module always first.
-*/
-   capability_add_hooks();
-
/* Load LSMs in specified order. */
prepare_lsm_order();
ordered_lsm_init();
-- 
2.17.1

[PATCH security-next v4 07/32] LSM: Convert security_initcall() into DEFINE_LSM()

[Kees Cook]

Instead of using argument-based initializers, switch to defining the
contents of struct lsm_info on a per-LSM basis. This also drops
the final use of the now inaccurate "initcall" naming.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h  | 5 ++---
 security/apparmor/lsm.c| 4 +++-
 security/integrity/iint.c  | 4 +++-
 security/selinux/hooks.c   | 4 +++-
 security/smack/smack_lsm.c | 4 +++-
 security/tomoyo/tomoyo.c   | 4 +++-
 6 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index d13059feca09..9c6b4198ff5a 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2045,11 +2045,10 @@ struct lsm_info {
 
 extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
 
-#define security_initcall(lsm) \
+#define DEFINE_LSM(lsm)
\
static struct lsm_info __lsm_##lsm  \
__used __section(.lsm_info.init)\
-   __aligned(sizeof(unsigned long))\
-   = { .init = lsm, }
+   __aligned(sizeof(unsigned long))
 
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 /*
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 8b8b70620bbe..c4863956c832 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1606,4 +1606,6 @@ static int __init apparmor_init(void)
return error;
 }
 
-security_initcall(apparmor_init);
+DEFINE_LSM(apparmor) = {
+   .init = apparmor_init,
+};
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 70d21b566955..94e8e1820748 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -175,7 +175,9 @@ static int __init integrity_iintcache_init(void)
  0, SLAB_PANIC, init_once);
return 0;
 }
-security_initcall(integrity_iintcache_init);
+DEFINE_LSM(integrity) = {
+   .init = integrity_iintcache_init,
+};
 
 
 /*
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad9a9b8e9979..6ca2e89ddbd6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7202,7 +7202,9 @@ void selinux_complete_init(void)
 
 /* SELinux requires early initialization in order to label
all processes and objects when they are created. */
-security_initcall(selinux_init);
+DEFINE_LSM(selinux) = {
+   .init = selinux_init,
+};
 
 #if defined(CONFIG_NETFILTER)
 
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 340fc30ad85d..c62e26939a69 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4882,4 +4882,6 @@ static __init int smack_init(void)
  * Smack requires early initialization in order to label
  * all processes and objects when they are created.
  */
-security_initcall(smack_init);
+DEFINE_LSM(smack) = {
+   .init = smack_init,
+};
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 9f932e2d6852..b2d83310 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -550,4 +550,6 @@ static int __init tomoyo_init(void)
return 0;
 }
 
-security_initcall(tomoyo_init);
+DEFINE_LSM(tomoyo) = {
+   .init = tomoyo_init,
+};
-- 
2.17.1

[PATCH security-next v4 10/32] LSM: Don't ignore initialization failures

[Kees Cook]

LSM initialization failures have traditionally been ignored. We should
at least WARN when something goes wrong.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 security/security.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/security/security.c b/security/security.c
index 395f804f6a91..2055af907eba 100644
--- a/security/security.c
+++ b/security/security.c
@@ -55,10 +55,12 @@ static __initdata bool debug;
 static void __init major_lsm_init(void)
 {
struct lsm_info *lsm;
+   int ret;
 
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
init_debug("initializing %s\n", lsm->name);
-   lsm->init();
+   ret = lsm->init();
+   WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
}
 }
 
-- 
2.17.1

[PATCH security-next v4 31/32] LSM: Separate idea of "major" LSM from "exclusive" LSM

[Kees Cook]

In order to both support old "security=" Legacy Major LSM selection, and
handling real exclusivity, this creates LSM_FLAG_EXCLUSIVE and updates
the selection logic to handle them.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h  |  1 +
 security/apparmor/lsm.c|  2 +-
 security/security.c| 12 
 security/selinux/hooks.c   |  2 +-
 security/smack/smack_lsm.c |  2 +-
 security/tomoyo/tomoyo.c   |  2 +-
 6 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 36e7a716fdfe..2c9cf89a20ad 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2040,6 +2040,7 @@ extern void security_add_hooks(struct security_hook_list 
*hooks, int count,
char *lsm);
 
 #define LSM_FLAG_LEGACY_MAJOR  BIT(0)
+#define LSM_FLAG_EXCLUSIVE BIT(1)
 
 enum lsm_order {
LSM_ORDER_FIRST = -1,   /* This is only for capabilities. */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 4cd96a66ed6f..4eb74a6f2020 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1599,7 +1599,7 @@ static int __init apparmor_init(void)
 
 DEFINE_LSM(apparmor) = {
.name = "apparmor",
-   .flags = LSM_FLAG_LEGACY_MAJOR,
+   .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
.enabled = &apparmor_enabled,
.init = apparmor_init,
 };
diff --git a/security/security.c b/security/security.c
index 813dab3b5b97..7d542e78b7e8 100644
--- a/security/security.c
+++ b/security/security.c
@@ -52,6 +52,7 @@ static __initconst const char * const builtin_lsm_order = 
CONFIG_LSM_ORDER;
 
 /* Ordered list of LSMs to initialize. */
 static __initdata struct lsm_info **ordered_lsms;
+static __initdata struct lsm_info *exclusive;
 
 static __initdata bool debug;
 #define init_debug(...)\
@@ -196,6 +197,12 @@ static bool __init lsm_allowed(struct lsm_info *lsm)
if (!is_enabled(lsm))
return false;
 
+   /* Not allowed if another exclusive LSM already initialized. */
+   if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && exclusive) {
+   init_debug("exclusive disabled: %s\n", lsm->name);
+   return false;
+   }
+
return true;
 }
 
@@ -211,6 +218,11 @@ static void __init maybe_initialize_lsm(struct lsm_info 
*lsm)
if (enabled) {
int ret;
 
+   if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && !exclusive) {
+   exclusive = lsm;
+   init_debug("exclusive: %s\n", lsm->name);
+   }
+
init_debug("initializing %s\n", lsm->name);
ret = lsm->init();
WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8f5eea097612..c070d3761ddc 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7181,7 +7181,7 @@ void selinux_complete_init(void)
all processes and objects when they are created. */
 DEFINE_LSM(selinux) = {
.name = "selinux",
-   .flags = LSM_FLAG_LEGACY_MAJOR,
+   .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
.enabled = &selinux_enabled,
.init = selinux_init,
 };
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index f243044d5a55..92e4baa342f8 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4881,6 +4881,6 @@ static __init int smack_init(void)
  */
 DEFINE_LSM(smack) = {
.name = "smack",
-   .flags = LSM_FLAG_LEGACY_MAJOR,
+   .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
.init = smack_init,
 };
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index a46f6bc1e97c..daff7d7897ad 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -550,6 +550,6 @@ static int __init tomoyo_init(void)
 
 DEFINE_LSM(tomoyo) = {
.name = "tomoyo",
-   .flags = LSM_FLAG_LEGACY_MAJOR,
+   .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
.init = tomoyo_init,
 };
-- 
2.17.1

[PATCH security-next v4 13/32] LoadPin: Rename "enable" to "enforce"

[Kees Cook]

LoadPin's "enable" setting is really about enforcement, not whether
or not the LSM is using LSM hooks. Instead, split this out so that LSM
enabling can be logically distinct from whether enforcement is happening
(for example, the pinning happens when the LSM is enabled, but the pin
is only checked when "enforce" is set). This allows LoadPin to continue
to operate sanely in test environments once LSM enable/disable is
centrally handled (i.e. we want LoadPin to be enabled separately from
its enforcement).

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 security/loadpin/Kconfig   |  4 ++--
 security/loadpin/loadpin.c | 21 +++--
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
index dd01aa91e521..8653608a3693 100644
--- a/security/loadpin/Kconfig
+++ b/security/loadpin/Kconfig
@@ -10,10 +10,10 @@ config SECURITY_LOADPIN
  have a root filesystem backed by a read-only device such as
  dm-verity or a CDROM.
 
-config SECURITY_LOADPIN_ENABLED
+config SECURITY_LOADPIN_ENFORCING
bool "Enforce LoadPin at boot"
depends on SECURITY_LOADPIN
help
  If selected, LoadPin will enforce pinning at boot. If not
  selected, it can be enabled at boot with the kernel parameter
- "loadpin.enabled=1".
+ "loadpin.enforcing=1".
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 0716af28808a..d8a68a6f6fef 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -44,7 +44,7 @@ static void report_load(const char *origin, struct file 
*file, char *operation)
kfree(pathname);
 }
 
-static int enabled = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENABLED);
+static int enforcing = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENFORCING);
 static struct super_block *pinned_root;
 static DEFINE_SPINLOCK(pinned_root_spinlock);
 
@@ -60,8 +60,8 @@ static struct ctl_path loadpin_sysctl_path[] = {
 
 static struct ctl_table loadpin_sysctl_table[] = {
{
-   .procname   = "enabled",
-   .data   = &enabled,
+   .procname   = "enforcing",
+   .data   = &enforcing,
.maxlen = sizeof(int),
.mode   = 0644,
.proc_handler   = proc_dointvec_minmax,
@@ -97,7 +97,7 @@ static void check_pinning_enforcement(struct super_block 
*mnt_sb)
   loadpin_sysctl_table))
pr_notice("sysctl registration failed!\n");
else
-   pr_info("load pinning can be disabled.\n");
+   pr_info("enforcement can be disabled.\n");
} else
pr_info("load pinning engaged.\n");
 }
@@ -128,7 +128,7 @@ static int loadpin_read_file(struct file *file, enum 
kernel_read_file_id id)
 
/* This handles the older init_module API that has a NULL file. */
if (!file) {
-   if (!enabled) {
+   if (!enforcing) {
report_load(origin, NULL, "old-api-pinning-ignored");
return 0;
}
@@ -151,7 +151,7 @@ static int loadpin_read_file(struct file *file, enum 
kernel_read_file_id id)
 * Unlock now since it's only pinned_root we care about.
 * In the worst case, we will (correctly) report pinning
 * failures before we have announced that pinning is
-* enabled. This would be purely cosmetic.
+* enforcing. This would be purely cosmetic.
 */
spin_unlock(&pinned_root_spinlock);
check_pinning_enforcement(pinned_root);
@@ -161,7 +161,7 @@ static int loadpin_read_file(struct file *file, enum 
kernel_read_file_id id)
}
 
if (IS_ERR_OR_NULL(pinned_root) || load_root != pinned_root) {
-   if (unlikely(!enabled)) {
+   if (unlikely(!enforcing)) {
report_load(origin, file, "pinning-ignored");
return 0;
}
@@ -186,10 +186,11 @@ static struct security_hook_list loadpin_hooks[] 
__lsm_ro_after_init = {
 
 void __init loadpin_add_hooks(void)
 {
-   pr_info("ready to pin (currently %sabled)", enabled ? "en" : "dis");
+   pr_info("ready to pin (currently %senforcing)\n",
+   enforcing ? "" : "not ");
security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
 }
 
 /* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
-module_param(enabled, int, 0);
-MODULE_PARM_DESC(enabled, "Pin module/firmware loading (default: true)");
+module_param(enforcing, int, 0);
+MODULE_PARM_DESC(enforcing, "Enforce module/firmware pinning");
-- 
2.17.1

[PATCH security-next v4 32/32] LSM: Add all exclusive LSMs to ordered initialization

[Kees Cook]

This removes CONFIG_DEFAULT_SECURITY in favor of the explicit build-time
ordering offered by CONFIG_LSM_ORDER, and adds all the exclusive LSMs to
the ordered LSM initialization. The old meaning of CONFIG_DEFAULT_SECURITY
is now captured by which exclusive LSM is listed first in the LSM order.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 security/Kconfig| 43 ---
 security/security.c | 23 +--
 2 files changed, 5 insertions(+), 61 deletions(-)

diff --git a/security/Kconfig b/security/Kconfig
index c459d2b4c7bd..cc8bb1c344f5 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -239,43 +239,6 @@ source security/yama/Kconfig
 
 source security/integrity/Kconfig
 
-choice
-   prompt "Default security module"
-   default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
-   default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
-   default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
-   default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
-   default DEFAULT_SECURITY_DAC
-
-   help
- Select the security module that will be used by default if the
- kernel parameter security= is not specified.
-
-   config DEFAULT_SECURITY_SELINUX
-   bool "SELinux" if SECURITY_SELINUX=y
-
-   config DEFAULT_SECURITY_SMACK
-   bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
-
-   config DEFAULT_SECURITY_TOMOYO
-   bool "TOMOYO" if SECURITY_TOMOYO=y
-
-   config DEFAULT_SECURITY_APPARMOR
-   bool "AppArmor" if SECURITY_APPARMOR=y
-
-   config DEFAULT_SECURITY_DAC
-   bool "Unix Discretionary Access Controls"
-
-endchoice
-
-config DEFAULT_SECURITY
-   string
-   default "selinux" if DEFAULT_SECURITY_SELINUX
-   default "smack" if DEFAULT_SECURITY_SMACK
-   default "tomoyo" if DEFAULT_SECURITY_TOMOYO
-   default "apparmor" if DEFAULT_SECURITY_APPARMOR
-   default "" if DEFAULT_SECURITY_DAC
-
 config LSM_ENABLE
string "LSMs to enable at boot time"
default "all"
@@ -293,12 +256,14 @@ config LSM_ENABLE
 
 config LSM_ORDER
string "Default initialization order of builtin LSMs"
-   default "yama,loadpin,integrity"
+   default "yama,loadpin,integrity,selinux,smack,tomoyo,apparmor"
help
  A comma-separated list of LSMs, in initialization order.
  Any LSMs left off this list will be link-order initialized
  after any listed LSMs. Any LSMs listed here but not built in
- the kernel will be ignored.
+ the kernel will be ignored. If the boot parameter
+ "lsm.order=" is used, it will override this order, with any
+ unlisted LSMs falling back to the order of this config, etc.
 
  If unsure, leave this as the default.
 
diff --git a/security/security.c b/security/security.c
index 7d542e78b7e8..d682342b6450 100644
--- a/security/security.c
+++ b/security/security.c
@@ -146,7 +146,6 @@ static void __init parse_lsm_order(const char *order, const 
char *origin)
 
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
if (lsm->order == LSM_ORDER_MUTABLE &&
-   (lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0 &&
strcmp(lsm->name, name) == 0) {
append_ordered_lsm(lsm, origin);
found = true;
@@ -178,8 +177,7 @@ static void __init prepare_lsm_order(void)
 
/* Add any missing LSMs, in link order. */
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
-   if (lsm->order == LSM_ORDER_MUTABLE &&
-   (lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
+   if (lsm->order == LSM_ORDER_MUTABLE)
append_ordered_lsm(lsm, "link-time");
}
 
@@ -237,18 +235,6 @@ static void __init ordered_lsm_init(void)
maybe_initialize_lsm(*lsm);
 }
 
-static void __init major_lsm_init(void)
-{
-   struct lsm_info *lsm;
-
-   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
-   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
-   continue;
-
-   maybe_initialize_lsm(lsm);
-   }
-}
-
 static void __init parse_lsm_enable(const char *str,
bool enabled)
 {
@@ -282,8 +268,6 @@ static void __init prepare_lsm_enable(void)
parse_lsm_enable(chosen_lsm_disable, false);
 
/* Process "security=", if given. */
-   if (!chosen_major_lsm)
-   chosen_major_lsm = CONFIG_DEFAULT_SECURITY;
if (chosen_major_lsm) {
struct lsm_info *lsm;
 
@@ -326,11 +310,6 @@ int __init security_init(void)
prepare_lsm_order();
ordered_lsm_init();
 
-   /*
-* Load all the remaining security modules.
-*/
-   major_lsm_init();
-
kfree(order

[PATCH security-next v4 09/32] LSM: Provide init debugging infrastructure

[Kees Cook]

Booting with "lsm.debug" will report future details on how LSM ordering
decisions are being made.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 .../admin-guide/kernel-parameters.txt  |  2 ++
 security/security.c| 18 ++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 9871e649ffef..32d323ee9218 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2274,6 +2274,8 @@
ltpc=   [NET]
Format: ,,
 
+   lsm.debug   [SECURITY] Enable LSM initialization debugging output.
+
machvec=[IA-64] Force the use of a particular machine-vector
(machvec) in a generic kernel.
Example: machvec=hpzx1_swiotlb
diff --git a/security/security.c b/security/security.c
index e74f46fba591..395f804f6a91 100644
--- a/security/security.c
+++ b/security/security.c
@@ -12,6 +12,8 @@
  * (at your option) any later version.
  */
 
+#define pr_fmt(fmt) "LSM: " fmt
+
 #include 
 #include 
 #include 
@@ -43,11 +45,19 @@ char *lsm_names;
 static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
CONFIG_DEFAULT_SECURITY;
 
+static __initdata bool debug;
+#define init_debug(...)\
+   do {\
+   if (debug)  \
+   pr_info(__VA_ARGS__);   \
+   } while (0)
+
 static void __init major_lsm_init(void)
 {
struct lsm_info *lsm;
 
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   init_debug("initializing %s\n", lsm->name);
lsm->init();
}
 }
@@ -91,6 +101,14 @@ static int __init choose_lsm(char *str)
 }
 __setup("security=", choose_lsm);
 
+/* Enable LSM order debugging. */
+static int __init enable_debug(char *str)
+{
+   debug = true;
+   return 1;
+}
+__setup("lsm.debug", enable_debug);
+
 static bool match_last_lsm(const char *list, const char *lsm)
 {
const char *last;
-- 
2.17.1

[PATCH security-next v4 11/32] LSM: Introduce LSM_FLAG_LEGACY_MAJOR

[Kees Cook]

This adds a flag for the current "major" LSMs to distinguish them when
we have a universal method for ordering all LSMs. It's called "legacy"
since the distinction of "major" will go away in the blob-sharing world.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 include/linux/lsm_hooks.h  | 3 +++
 security/apparmor/lsm.c| 1 +
 security/selinux/hooks.c   | 1 +
 security/smack/smack_lsm.c | 1 +
 security/tomoyo/tomoyo.c   | 1 +
 5 files changed, 7 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index ae159b02f3ab..531e219a49b9 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2039,8 +2039,11 @@ extern char *lsm_names;
 extern void security_add_hooks(struct security_hook_list *hooks, int count,
char *lsm);
 
+#define LSM_FLAG_LEGACY_MAJOR  BIT(0)
+
 struct lsm_info {
const char *name;   /* Required. */
+   unsigned long flags;/* Optional: flags describing LSM */
int (*init)(void);  /* Required. */
 };
 
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index dca4b7dbf368..768cb539fb6c 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1608,5 +1608,6 @@ static int __init apparmor_init(void)
 
 DEFINE_LSM(apparmor) = {
.name = "apparmor",
+   .flags = LSM_FLAG_LEGACY_MAJOR,
.init = apparmor_init,
 };
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9651bccae270..020886895819 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7204,6 +7204,7 @@ void selinux_complete_init(void)
all processes and objects when they are created. */
 DEFINE_LSM(selinux) = {
.name = "selinux",
+   .flags = LSM_FLAG_LEGACY_MAJOR,
.init = selinux_init,
 };
 
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 2fb56bcf1316..db8bc6b6d8b0 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4884,5 +4884,6 @@ static __init int smack_init(void)
  */
 DEFINE_LSM(smack) = {
.name = "smack",
+   .flags = LSM_FLAG_LEGACY_MAJOR,
.init = smack_init,
 };
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 1b5b5097efd7..09f7af130d3a 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -552,5 +552,6 @@ static int __init tomoyo_init(void)
 
 DEFINE_LSM(tomoyo) = {
.name = "tomoyo",
+   .flags = LSM_FLAG_LEGACY_MAJOR,
.init = tomoyo_init,
 };
-- 
2.17.1

[PATCH security-next v4 12/32] LSM: Provide separate ordered initialization

[Kees Cook]

This provides a place for ordered LSMs to be initialized, separate from
the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
ordered_lsm_init(), but it will change drastically in later patches.

What is not obvious in the patch is that this change moves the integrity
LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
list, there is no reordering yet created.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 security/security.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/security/security.c b/security/security.c
index 2055af907eba..e672ced5 100644
--- a/security/security.c
+++ b/security/security.c
@@ -52,12 +52,30 @@ static __initdata bool debug;
pr_info(__VA_ARGS__);   \
} while (0)
 
+static void __init ordered_lsm_init(void)
+{
+   struct lsm_info *lsm;
+   int ret;
+
+   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) != 0)
+   continue;
+
+   init_debug("initializing %s\n", lsm->name);
+   ret = lsm->init();
+   WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
+   }
+}
+
 static void __init major_lsm_init(void)
 {
struct lsm_info *lsm;
int ret;
 
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
+   continue;
+
init_debug("initializing %s\n", lsm->name);
ret = lsm->init();
WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
@@ -87,6 +105,9 @@ int __init security_init(void)
yama_add_hooks();
loadpin_add_hooks();
 
+   /* Load LSMs in specified order. */
+   ordered_lsm_init();
+
/*
 * Load all the remaining security modules.
 */
-- 
2.17.1

[PATCH security-next v4 06/32] vmlinux.lds.h: Move LSM_TABLE into INIT_DATA

[Kees Cook]

Since the struct lsm_info table is not an initcall, we can just move it
into INIT_DATA like all the other tables.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 arch/arc/kernel/vmlinux.lds.S|  1 -
 arch/arm/kernel/vmlinux-xip.lds.S|  1 -
 arch/arm64/kernel/vmlinux.lds.S  |  1 -
 arch/h8300/kernel/vmlinux.lds.S  |  1 -
 arch/microblaze/kernel/vmlinux.lds.S |  2 --
 arch/powerpc/kernel/vmlinux.lds.S|  2 --
 arch/um/include/asm/common.lds.S |  2 --
 arch/xtensa/kernel/vmlinux.lds.S |  1 -
 include/asm-generic/vmlinux.lds.h| 24 +++-
 9 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/arch/arc/kernel/vmlinux.lds.S b/arch/arc/kernel/vmlinux.lds.S
index f35ed578e007..8fb16bdabdcf 100644
--- a/arch/arc/kernel/vmlinux.lds.S
+++ b/arch/arc/kernel/vmlinux.lds.S
@@ -71,7 +71,6 @@ SECTIONS
INIT_SETUP(L1_CACHE_BYTES)
INIT_CALLS
CON_INITCALL
-   SECURITY_INITCALL
}
 
.init.arch.info : {
diff --git a/arch/arm/kernel/vmlinux-xip.lds.S 
b/arch/arm/kernel/vmlinux-xip.lds.S
index 3593d5c1acd2..8c74037ade22 100644
--- a/arch/arm/kernel/vmlinux-xip.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S
@@ -96,7 +96,6 @@ SECTIONS
INIT_SETUP(16)
INIT_CALLS
CON_INITCALL
-   SECURITY_INITCALL
INIT_RAM_FS
}
 
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 605d1b60469c..7d23d591b03c 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -166,7 +166,6 @@ SECTIONS
INIT_SETUP(16)
INIT_CALLS
CON_INITCALL
-   SECURITY_INITCALL
INIT_RAM_FS
*(.init.rodata.* .init.bss) /* from the EFI stub */
}
diff --git a/arch/h8300/kernel/vmlinux.lds.S b/arch/h8300/kernel/vmlinux.lds.S
index 35716a3048de..49f716c0a1df 100644
--- a/arch/h8300/kernel/vmlinux.lds.S
+++ b/arch/h8300/kernel/vmlinux.lds.S
@@ -56,7 +56,6 @@ SECTIONS
__init_begin = .;
INIT_TEXT_SECTION(4)
INIT_DATA_SECTION(4)
-   SECURITY_INIT
__init_end = .;
_edata = . ;
_begin_data = LOADADDR(.data);
diff --git a/arch/microblaze/kernel/vmlinux.lds.S 
b/arch/microblaze/kernel/vmlinux.lds.S
index 289d0e7f3e3a..e1f3e8741292 100644
--- a/arch/microblaze/kernel/vmlinux.lds.S
+++ b/arch/microblaze/kernel/vmlinux.lds.S
@@ -117,8 +117,6 @@ SECTIONS {
CON_INITCALL
}
 
-   SECURITY_INIT
-
__init_end_before_initramfs = .;
 
.init.ramfs : AT(ADDR(.init.ramfs) - LOAD_OFFSET) {
diff --git a/arch/powerpc/kernel/vmlinux.lds.S 
b/arch/powerpc/kernel/vmlinux.lds.S
index 07ae018e550e..105a976323aa 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -212,8 +212,6 @@ SECTIONS
CON_INITCALL
}
 
-   SECURITY_INIT
-
. = ALIGN(8);
__ftr_fixup : AT(ADDR(__ftr_fixup) - LOAD_OFFSET) {
__start___ftr_fixup = .;
diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S
index 7adb4e6b658a..4049f2c46387 100644
--- a/arch/um/include/asm/common.lds.S
+++ b/arch/um/include/asm/common.lds.S
@@ -53,8 +53,6 @@
CON_INITCALL
   }
 
-  SECURITY_INIT
-
   .exitcall : {
__exitcall_begin = .;
*(.exitcall.exit)
diff --git a/arch/xtensa/kernel/vmlinux.lds.S b/arch/xtensa/kernel/vmlinux.lds.S
index a1c3edb8ad56..b727b18a68ac 100644
--- a/arch/xtensa/kernel/vmlinux.lds.S
+++ b/arch/xtensa/kernel/vmlinux.lds.S
@@ -197,7 +197,6 @@ SECTIONS
 INIT_SETUP(XCHAL_ICACHE_LINESIZE)
 INIT_CALLS
 CON_INITCALL
-SECURITY_INITCALL
 INIT_RAM_FS
   }
 
diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 5079a969e612..b31ea8bdfef9 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -203,6 +203,15 @@
 #define EARLYCON_TABLE()
 #endif
 
+#ifdef CONFIG_SECURITY
+#define LSM_TABLE(). = ALIGN(8);   \
+   __start_lsm_info = .;   \
+   KEEP(*(.lsm_info.init)) \
+   __end_lsm_info = .;
+#else
+#define LSM_TABLE()
+#endif
+
 #define ___OF_TABLE(cfg, name) _OF_TABLE_##cfg(name)
 #define __OF_TABLE(cfg, name)  ___OF_TABLE(cfg, name)
 #define OF_TABLE(cfg, name)__OF_TABLE(IS_ENABLED(cfg), name)
@@ -597,7 +606,8 @@
IRQCHIP_OF_MATCH_TABLE()\
ACPI_PROBE_TABLE(irqchip)   \
ACPI_PROBE_TABLE(timer) \
-   EARLYCON_TABLE()
+   EARLYCON_TABLE()\
+   LSM_TABLE()
 
 #define INIT_TEXT

[PATCH security-next v4 05/32] LSM: Convert from initcall to struct lsm_info

[Kees Cook]

In preparation for doing more interesting LSM init probing, this converts
the existing initcall system into an explicit call into a function pointer
from a section-collected struct lsm_info array.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: James Morris 
Reviewed-by: John Johansen 
---
 include/linux/init.h  |  2 --
 include/linux/lsm_hooks.h | 12 
 include/linux/module.h|  1 -
 security/integrity/iint.c |  1 +
 security/security.c   | 14 +-
 5 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/include/linux/init.h b/include/linux/init.h
index 77636539e77c..9c2aba1dbabf 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -133,7 +133,6 @@ static inline initcall_t 
initcall_from_entry(initcall_entry_t *entry)
 #endif
 
 extern initcall_entry_t __con_initcall_start[], __con_initcall_end[];
-extern initcall_entry_t __start_lsm_info[], __end_lsm_info[];
 
 /* Used for contructor calls. */
 typedef void (*ctor_fn_t)(void);
@@ -236,7 +235,6 @@ extern bool initcall_debug;
static exitcall_t __exitcall_##fn __exit_call = fn
 
 #define console_initcall(fn)   ___define_initcall(fn,, .con_initcall)
-#define security_initcall(fn)  ___define_initcall(fn,, .lsm_info)
 
 struct obs_kernel_param {
const char *str;
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 97a020c616ad..d13059feca09 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2039,6 +2039,18 @@ extern char *lsm_names;
 extern void security_add_hooks(struct security_hook_list *hooks, int count,
char *lsm);
 
+struct lsm_info {
+   int (*init)(void);  /* Required. */
+};
+
+extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
+
+#define security_initcall(lsm) \
+   static struct lsm_info __lsm_##lsm  \
+   __used __section(.lsm_info.init)\
+   __aligned(sizeof(unsigned long))\
+   = { .init = lsm, }
+
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 /*
  * Assuring the safety of deleting a security module is up to
diff --git a/include/linux/module.h b/include/linux/module.h
index f807f15bebbe..264979283756 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -123,7 +123,6 @@ extern void cleanup_module(void);
 #define late_initcall_sync(fn) module_init(fn)
 
 #define console_initcall(fn)   module_init(fn)
-#define security_initcall(fn)  module_init(fn)
 
 /* Each module must use one module_init(). */
 #define module_init(initfn)\
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 5a6810041e5c..70d21b566955 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "integrity.h"
 
 static struct rb_root integrity_iint_tree = RB_ROOT;
diff --git a/security/security.c b/security/security.c
index 41a5da2c7faf..e74f46fba591 100644
--- a/security/security.c
+++ b/security/security.c
@@ -43,16 +43,12 @@ char *lsm_names;
 static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
CONFIG_DEFAULT_SECURITY;
 
-static void __init do_security_initcalls(void)
+static void __init major_lsm_init(void)
 {
-   initcall_t call;
-   initcall_entry_t *ce;
+   struct lsm_info *lsm;
 
-   ce = __start_lsm_info;
-   while (ce < __end_lsm_info) {
-   call = initcall_from_entry(ce);
-   call();
-   ce++;
+   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   lsm->init();
}
 }
 
@@ -82,7 +78,7 @@ int __init security_init(void)
/*
 * Load all the remaining security modules.
 */
-   do_security_initcalls();
+   major_lsm_init();
 
return 0;
 }
-- 
2.17.1

[PATCH security-next v4 08/32] LSM: Record LSM name in struct lsm_info

[Kees Cook]

In preparation for making LSM selections outside of the LSMs, include
the name of LSMs in struct lsm_info.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h  | 1 +
 security/apparmor/lsm.c| 1 +
 security/integrity/iint.c  | 1 +
 security/selinux/hooks.c   | 1 +
 security/smack/smack_lsm.c | 1 +
 security/tomoyo/tomoyo.c   | 1 +
 6 files changed, 6 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9c6b4198ff5a..ae159b02f3ab 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2040,6 +2040,7 @@ extern void security_add_hooks(struct security_hook_list 
*hooks, int count,
char *lsm);
 
 struct lsm_info {
+   const char *name;   /* Required. */
int (*init)(void);  /* Required. */
 };
 
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c4863956c832..dca4b7dbf368 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1607,5 +1607,6 @@ static int __init apparmor_init(void)
 }
 
 DEFINE_LSM(apparmor) = {
+   .name = "apparmor",
.init = apparmor_init,
 };
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 94e8e1820748..1ea05da2323d 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -176,6 +176,7 @@ static int __init integrity_iintcache_init(void)
return 0;
 }
 DEFINE_LSM(integrity) = {
+   .name = "integrity",
.init = integrity_iintcache_init,
 };
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 6ca2e89ddbd6..9651bccae270 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7203,6 +7203,7 @@ void selinux_complete_init(void)
 /* SELinux requires early initialization in order to label
all processes and objects when they are created. */
 DEFINE_LSM(selinux) = {
+   .name = "selinux",
.init = selinux_init,
 };
 
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index c62e26939a69..2fb56bcf1316 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4883,5 +4883,6 @@ static __init int smack_init(void)
  * all processes and objects when they are created.
  */
 DEFINE_LSM(smack) = {
+   .name = "smack",
.init = smack_init,
 };
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index b2d83310..1b5b5097efd7 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -551,5 +551,6 @@ static int __init tomoyo_init(void)
 }
 
 DEFINE_LSM(tomoyo) = {
+   .name = "tomoyo",
.init = tomoyo_init,
 };
-- 
2.17.1

[PATCH security-next v4 00/32] LSM: Explict LSM ordering

[Kees Cook]

v4:
- add Reviewed-bys.
- cosmetic tweaks.
- New patches to fully centralize LSM "enable" decisions:
LSM: Finalize centralized LSM enabling logic
apparmor: Remove boot parameter
selinux: Remove boot parameter

v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic

v2:
- add "lsm.order=" and CONFIG_LSM_ORDER instead of overloading "security="
- reorganize introduction of ordering logic code

Overview:

This refactors the LSM registration and initialization infrastructure to
more centrally support different LSM types for more cleanly supporting the
future expansion of LSM stacking via the "blob-sharing" patch series. What
was considered a "major" LSM is kept for legacy use of the "security="
boot parameter, and now overlaps with the new class of "exclusive" LSMs
for the future blob sharing. The "minor" LSMs become more well defined
as a result of the refactoring.

Approach:

To better show LSMs activation some debug reporting was added (enabled
with the "lsm.debug" boot commandline option).

I added a WARN() around LSM initialization failures, which appear to
have always been silently ignored. (Realistically any LSM init failures
would have only been due to catastrophic kernel issues that would render
a system unworkable anyway, but it'd be better to expose the problem as
early as possible.)

Instead of continuing to (somewhat improperly) overload the kernel's
initcall system, this changes the LSM infrastructure to store a
registration structure (struct lsm_info) table instead, where metadata
about each LSM can be recorded (name, flags, order, enable flag, init
function). This can be extended in the future to include things like
required blob size for the coming "blob sharing" LSMs.

The "major" LSMs had to individually negotiate which of them should be
enabled. This didn't provide a way to negotiate combinations of other
LSMs (as will be needed for "blob sharing" LSMs). This is solved by
providing the LSM infrastructure with all the details needed to make
the choice (exposing the per-LSM "enabled" flag, if used, the LSM
characteristics, and ordering expectations).

As a result of the refactoring, the "minor" LSMs are able to remove
the open-coded security_add_hooks() calls for "capability", "yama",
and "loadpin", and to redefine "integrity" properly as a general LSM.
(Note that "integrity" actually defined _no_ hooks, but needs the early
initialization).

With all LSMs being proessed centrally, it was possible to implement
a new boot parameter "lsm.order=" to provide explicit ordering, which
is helpful for the future "blob sharing" LSMs. Matching this is the
new CONFIG_LSM_ORDER, which replaces CONFIG_DEFAULT_SECURITY, as it
provides a higher granularity of control.

Breakdown of patches:

Infrastructure improvements (no logical changes):
  LSM: Correctly announce start of LSM initialization
  vmlinux.lds.h: Avoid copy/paste of security_init section
  LSM: Rename .security_initcall section to .lsm_info
  LSM: Remove initcall tracing
  LSM: Convert from initcall to struct lsm_info
  vmlinux.lds.h: Move LSM_TABLE into INIT_DATA
  LSM: Convert security_initcall() into DEFINE_LSM()
  LSM: Record LSM name in struct lsm_info
  LSM: Provide init debugging infrastructure
  LSM: Don't ignore initialization failures

Split "integrity" out into "ordered initialization" (no logical changes):
  LSM: Introduce LSM_FLAG_LEGACY_MAJOR
  LSM: Provide separate ordered initialization

Provide centralized LSM enable/disable infrastructure:
  LoadPin: Rename "enable" to "enforce"
  LSM: Plumb visibility into optional "enabled" state
  LSM: Lift LSM selection out of individual LSMs
  LSM: Prepare for arbitrary LSM enabling
  LSM: Introduce CONFIG_LSM_ENABLE
  LSM: Introduce lsm.enable= and lsm.disable=
  LSM: Prepare for reorganizing "security=" logic
  LSM: Refactor "security=" in terms of enable/disable
  LSM: Finalize centralized LSM enabling logic
  apparmor: Remove boot parameter
  selinux: Remove boot parameter

Provide centralized LSM ordering infrastructure:
  LSM: Build ordered list of ordered LSMs for init
  LSM: Introduce CONFIG_LSM_ORDER
  LSM: Introduce "lsm.order=" for boottime ordering

Move minor LSMs into ordered LSM initialization:
  LoadPin: Initialize as ordered LSM
  Yama: Initialize as ordered LSM
  LSM: Introduce enum lsm_order
  capability: Initialize as LSM_ORDER_FIRST

Move major LSMs into ordered LSM initialization:
  LSM: Separate idea of "major" LSM from "exclusive" LSM
  LSM: Add all exclusive LSMs to ordered initialization

-Kees

 .../admin-guide/kernel-parameters.txt |  34 +-
 arch/arc/kernel/vmlinux.lds.S |   1 -
 arch/arm/kernel/vmlinux-xip.lds.S |   1 -
 arch/arm64/kernel/vmlinux.lds.S   |   1 -
 arch/h8300/kernel/vmlinux.lds.S   |   1 -
 arch/microblaze/kernel/vmlinux.lds.S  |   2 -
 arch/powerpc/kernel/vmlinux.lds.S |   2 -
 arch/um/include/asm/common.lds.S  |   2 -
 arch/xtensa/kernel/v

[PATCH security-next v4 02/32] vmlinux.lds.h: Avoid copy/paste of security_init section

[Kees Cook]

Avoid copy/paste by defining SECURITY_INIT in terms of SECURITY_INITCALL.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: James Morris 
Reviewed-by: John Johansen 
---
 include/asm-generic/vmlinux.lds.h | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 7b75ff6e2fce..934a45395547 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -473,13 +473,6 @@
 #define RODATA  RO_DATA_SECTION(4096)
 #define RO_DATA(align)  RO_DATA_SECTION(align)
 
-#define SECURITY_INIT  \
-   .security_initcall.init : AT(ADDR(.security_initcall.init) - 
LOAD_OFFSET) { \
-   __security_initcall_start = .;  \
-   KEEP(*(.security_initcall.init))\
-   __security_initcall_end = .;\
-   }
-
 /*
  * .text section. Map to function alignment to avoid address changes
  * during second ld run in second ld pass when generating System.map
@@ -798,6 +791,12 @@
KEEP(*(.security_initcall.init))\
__security_initcall_end = .;
 
+/* Older linker script style for security init. */
+#define SECURITY_INIT  \
+   .security_initcall.init : AT(ADDR(.security_initcall.init) - 
LOAD_OFFSET) { \
+   SECURITY_INITCALL   \
+   }
+
 #ifdef CONFIG_BLK_DEV_INITRD
 #define INIT_RAM_FS\
. = ALIGN(4);   \
-- 
2.17.1

[PATCH security-next v4 19/32] LSM: Prepare for reorganizing "security=" logic

[Kees Cook]

This moves the string handling for "security=" boot parameter into
a stored pointer instead of a string duplicate. This will allow
easier handling of the string when switching logic to use the coming
enable/disable infrastructure.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 security/security.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/security/security.c b/security/security.c
index 3fff2d1d1ec4..455ba2767965 100644
--- a/security/security.c
+++ b/security/security.c
@@ -34,18 +34,14 @@
 
 #define MAX_LSM_EVM_XATTR  2
 
-/* Maximum number of letters for an LSM name string */
-#define SECURITY_NAME_MAX  10
-
 struct security_hook_heads security_hook_heads __lsm_ro_after_init;
 static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
 
 char *lsm_names;
 /* Boot-time LSM user choice */
-static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
-   CONFIG_DEFAULT_SECURITY;
 static __initdata const char *chosen_lsm_enable;
 static __initdata const char *chosen_lsm_disable;
+static __initdata const char *chosen_major_lsm;
 
 static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
 
@@ -112,7 +108,7 @@ static bool __init lsm_allowed(struct lsm_info *lsm)
return true;
 
/* Disabled if this LSM isn't the chosen one. */
-   if (strcmp(lsm->name, chosen_lsm) != 0)
+   if (strcmp(lsm->name, chosen_major_lsm) != 0)
return false;
 
return true;
@@ -191,6 +187,9 @@ static void __init prepare_lsm_enable(void)
/* Process "lsm.enable=" and "lsm.disable=", if given. */
parse_lsm_enable(chosen_lsm_enable, set_enabled, true);
parse_lsm_enable(chosen_lsm_disable, set_enabled, false);
+
+   if (!chosen_major_lsm)
+   chosen_major_lsm = CONFIG_DEFAULT_SECURITY;
 }
 
 /**
@@ -231,12 +230,12 @@ int __init security_init(void)
 }
 
 /* Save user chosen LSM */
-static int __init choose_lsm(char *str)
+static int __init choose_major_lsm(char *str)
 {
-   strncpy(chosen_lsm, str, SECURITY_NAME_MAX);
+   chosen_major_lsm = str;
return 1;
 }
-__setup("security=", choose_lsm);
+__setup("security=", choose_major_lsm);
 
 /* Enable LSM order debugging. */
 static int __init enable_debug(char *str)
-- 
2.17.1

[PATCH security-next v4 28/32] Yama: Initialize as ordered LSM

[Kees Cook]

This converts Yama from being a direct "minor" LSM into an ordered LSM.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h | 5 -
 security/Kconfig  | 2 +-
 security/security.c   | 1 -
 security/yama/yama_lsm.c  | 8 +++-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 098ccf2caa0e..63a6caaee8e6 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2086,10 +2086,5 @@ static inline void security_delete_hooks(struct 
security_hook_list *hooks,
 #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
 
 extern void __init capability_add_hooks(void);
-#ifdef CONFIG_SECURITY_YAMA
-extern void __init yama_add_hooks(void);
-#else
-static inline void __init yama_add_hooks(void) { }
-#endif
 
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/Kconfig b/security/Kconfig
index e59cb9296316..c459d2b4c7bd 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -293,7 +293,7 @@ config LSM_ENABLE
 
 config LSM_ORDER
string "Default initialization order of builtin LSMs"
-   default "loadpin,integrity"
+   default "yama,loadpin,integrity"
help
  A comma-separated list of LSMs, in initialization order.
  Any LSMs left off this list will be link-order initialized
diff --git a/security/security.c b/security/security.c
index 6957f5f50483..44c23d23158e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -296,7 +296,6 @@ int __init security_init(void)
 * Load minor LSMs, with the capability module always first.
 */
capability_add_hooks();
-   yama_add_hooks();
 
/* Load LSMs in specified order. */
prepare_lsm_order();
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index ffda91a4a1aa..eb1da1303d2e 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -477,9 +477,15 @@ static void __init yama_init_sysctl(void)
 static inline void yama_init_sysctl(void) { }
 #endif /* CONFIG_SYSCTL */
 
-void __init yama_add_hooks(void)
+static int __init yama_init(void)
 {
pr_info("Yama: becoming mindful.\n");
security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama");
yama_init_sysctl();
+   return 0;
 }
+
+DEFINE_LSM(yama) = {
+   .name = "yama",
+   .init = yama_init,
+};
-- 
2.17.1

[PATCH security-next v4 27/32] LoadPin: Initialize as ordered LSM

[Kees Cook]

This converts LoadPin from being a direct "minor" LSM into an ordered LSM.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h  | 5 -
 security/Kconfig   | 2 +-
 security/loadpin/loadpin.c | 8 +++-
 security/security.c| 1 -
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index b026ea93ff01..098ccf2caa0e 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2091,10 +2091,5 @@ extern void __init yama_add_hooks(void);
 #else
 static inline void __init yama_add_hooks(void) { }
 #endif
-#ifdef CONFIG_SECURITY_LOADPIN
-void __init loadpin_add_hooks(void);
-#else
-static inline void loadpin_add_hooks(void) { };
-#endif
 
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/Kconfig b/security/Kconfig
index c68520d97fd7..e59cb9296316 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -293,7 +293,7 @@ config LSM_ENABLE
 
 config LSM_ORDER
string "Default initialization order of builtin LSMs"
-   default "integrity"
+   default "loadpin,integrity"
help
  A comma-separated list of LSMs, in initialization order.
  Any LSMs left off this list will be link-order initialized
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index d8a68a6f6fef..dab42bfa1e4a 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -184,13 +184,19 @@ static struct security_hook_list loadpin_hooks[] 
__lsm_ro_after_init = {
LSM_HOOK_INIT(kernel_load_data, loadpin_load_data),
 };
 
-void __init loadpin_add_hooks(void)
+static int __init loadpin_init(void)
 {
pr_info("ready to pin (currently %senforcing)\n",
enforcing ? "" : "not ");
security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
+   return 0;
 }
 
+DEFINE_LSM(loadpin) = {
+   .name = "loadpin",
+   .init = loadpin_init,
+};
+
 /* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
 module_param(enforcing, int, 0);
 MODULE_PARM_DESC(enforcing, "Enforce module/firmware pinning");
diff --git a/security/security.c b/security/security.c
index 6fafad44b85e..6957f5f50483 100644
--- a/security/security.c
+++ b/security/security.c
@@ -297,7 +297,6 @@ int __init security_init(void)
 */
capability_add_hooks();
yama_add_hooks();
-   loadpin_add_hooks();
 
/* Load LSMs in specified order. */
prepare_lsm_order();
-- 
2.17.1

[PATCH security-next v4 17/32] LSM: Introduce CONFIG_LSM_ENABLE

[Kees Cook]

To provide a set of default-enabled LSMs at boot, this introduces the
new CONFIG_LSM_ENABLE. A value of "all" means all builtin LSMs are
enabled by default. Any unlisted LSMs will be implicitly disabled
(excepting those with LSM-specific CONFIGs for enabling/disabling).

The behavior of the LSM-specific CONFIGs for SELinux are AppArmor
unchanged: the default-enabled state for those LSMs remains controlled
through their LSM-specific "enable" CONFIGs.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h | 2 +-
 security/Kconfig  | 8 
 security/security.c   | 4 +++-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9ecb623fb39d..fd85637a1931 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2044,7 +2044,7 @@ extern void security_add_hooks(struct security_hook_list 
*hooks, int count,
 struct lsm_info {
const char *name;   /* Required. */
unsigned long flags;/* Optional: flags describing LSM */
-   int *enabled;   /* Optional: NULL means enabled. */
+   int *enabled;   /* Optional: NULL checks CONFIG_LSM_ENABLE */
int (*init)(void);  /* Required. */
 };
 
diff --git a/security/Kconfig b/security/Kconfig
index 27d8b2688f75..ac23feba584d 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -276,5 +276,13 @@ config DEFAULT_SECURITY
default "apparmor" if DEFAULT_SECURITY_APPARMOR
default "" if DEFAULT_SECURITY_DAC
 
+config LSM_ENABLE
+   string "LSMs to enable at boot time"
+   default "all"
+   help
+ A comma-separated list of LSMs to enable by default at boot. The
+ default is "all", to enable all LSM modules at boot. Any LSMs
+ not listed here will be disabled by default.
+
 endmenu
 
diff --git a/security/security.c b/security/security.c
index 9459b4ee4fd9..35601000176b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -45,6 +45,8 @@ char *lsm_names;
 static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
CONFIG_DEFAULT_SECURITY;
 
+static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
+
 static __initdata bool debug;
 #define init_debug(...)\
do {\
@@ -182,7 +184,7 @@ static void __init parse_lsm_enable(const char *str,
 static void __init prepare_lsm_enable(void)
 {
/* Prepare defaults. */
-   parse_lsm_enable("all", default_enabled, true);
+   parse_lsm_enable(builtin_lsm_enable, default_enabled, true);
 }
 
 /**
-- 
2.17.1

[PATCH security-next v4 20/32] LSM: Refactor "security=" in terms of enable/disable

[Kees Cook]

For what are marked as the Legacy Major LSMs, make them effectively
exclusive when selected on the "security=" boot parameter, to handle
the future case of when a previously major LSMs become non-exclusive
(e.g. when TOMOYO starts blob-sharing).

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 security/security.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/security/security.c b/security/security.c
index 455ba2767965..d7132c181ea6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -103,14 +103,6 @@ static bool __init lsm_allowed(struct lsm_info *lsm)
if (!is_enabled(lsm))
return false;
 
-   /* Skip major-specific checks if not a major LSM. */
-   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
-   return true;
-
-   /* Disabled if this LSM isn't the chosen one. */
-   if (strcmp(lsm->name, chosen_major_lsm) != 0)
-   return false;
-
return true;
 }
 
@@ -188,8 +180,24 @@ static void __init prepare_lsm_enable(void)
parse_lsm_enable(chosen_lsm_enable, set_enabled, true);
parse_lsm_enable(chosen_lsm_disable, set_enabled, false);
 
+   /* Process "security=", if given. */
if (!chosen_major_lsm)
chosen_major_lsm = CONFIG_DEFAULT_SECURITY;
+   if (chosen_major_lsm) {
+   struct lsm_info *lsm;
+
+   /*
+* To match the original "security=" behavior, this
+* explicitly does NOT fallback to another Legacy Major
+* if the selected one was separately disabled: disable
+* all non-matching Legacy Major LSMs.
+*/
+   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) &&
+   strcmp(lsm->name, chosen_major_lsm) != 0)
+   set_enabled(lsm, false);
+   }
+   }
 }
 
 /**
-- 
2.17.1

[PATCH security-next v4 29/32] LSM: Introduce enum lsm_order

[Kees Cook]

In preparation for distinguishing the "capability" LSM from other LSMs,
it must be ordered first. This introduces LSM_ORDER_MUTABLE for the
general LSMs, LSM_ORDER_FIRST for capabilities, and LSM_ORDER_LAST for
anything that must run last (e.g. Landlock may use this in the future).

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 include/linux/lsm_hooks.h |  7 +++
 security/security.c   | 18 --
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 63a6caaee8e6..62bc230826e0 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2041,8 +2041,15 @@ extern void security_add_hooks(struct security_hook_list 
*hooks, int count,
 
 #define LSM_FLAG_LEGACY_MAJOR  BIT(0)
 
+enum lsm_order {
+   LSM_ORDER_FIRST = -1,   /* This is only for capabilities. */
+   LSM_ORDER_MUTABLE = 0,
+   LSM_ORDER_LAST,
+};
+
 struct lsm_info {
const char *name;   /* Required. */
+   enum lsm_order order;   /* Optional: default is LSM_ORDER_MUTABLE */
unsigned long flags;/* Optional: flags describing LSM */
int *enabled;   /* Optional: set based on CONFIG_LSM_ENABLE */
int (*init)(void);  /* Required. */
diff --git a/security/security.c b/security/security.c
index 44c23d23158e..dac379518e60 100644
--- a/security/security.c
+++ b/security/security.c
@@ -140,7 +140,8 @@ static void __init parse_lsm_order(const char *order, const 
char *origin)
bool found = false;
 
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
-   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0 &&
+   if (lsm->order == LSM_ORDER_MUTABLE &&
+   (lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0 &&
strcmp(lsm->name, name) == 0) {
append_ordered_lsm(lsm, origin);
found = true;
@@ -158,6 +159,12 @@ static void __init prepare_lsm_order(void)
 {
struct lsm_info *lsm;
 
+   /* LSM_ORDER_FIRST is always first. */
+   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   if (lsm->order == LSM_ORDER_FIRST)
+   append_ordered_lsm(lsm, "first");
+   }
+
/* Parse order from commandline, if present. */
parse_lsm_order(chosen_lsm_order, "cmdline");
 
@@ -166,9 +173,16 @@ static void __init prepare_lsm_order(void)
 
/* Add any missing LSMs, in link order. */
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
-   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
+   if (lsm->order == LSM_ORDER_MUTABLE &&
+   (lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
append_ordered_lsm(lsm, "link-time");
}
+
+   /* LSM_ORDER_LAST is always last. */
+   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   if (lsm->order == LSM_ORDER_LAST)
+   append_ordered_lsm(lsm, "last");
+   }
 }
 
 /* Is an LSM allowed to be initialized? */
-- 
2.17.1

[PATCH security-next v4 23/32] selinux: Remove boot parameter

[Kees Cook]

Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and
"lsm.enable=...", this removes the LSM-specific enabling logic from
SELinux.

Signed-off-by: Kees Cook 
---
 .../admin-guide/kernel-parameters.txt |  9 --
 security/selinux/Kconfig  | 29 ---
 security/selinux/hooks.c  | 15 +-
 3 files changed, 1 insertion(+), 52 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index cf963febebb0..0d10ab3d020e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4045,15 +4045,6 @@
loaded. An invalid security module name will be treated
as if no module has been chosen.
 
-   selinux=[SELINUX] Disable or enable SELinux at boot time.
-   Format: { "0" | "1" }
-   See security/selinux/Kconfig help text.
-   0 -- disable.
-   1 -- enable.
-   Default value is set via kernel config option.
-   If enabled at boot time, /selinux/disable can be used
-   later to disable prior to initial policy load.
-
serialnumber[BUGS=X86-32]
 
shapers=[NET]
diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
index 8af7a690eb40..86936528a0bb 100644
--- a/security/selinux/Kconfig
+++ b/security/selinux/Kconfig
@@ -8,35 +8,6 @@ config SECURITY_SELINUX
  You will also need a policy configuration and a labeled filesystem.
  If you are unsure how to answer this question, answer N.
 
-config SECURITY_SELINUX_BOOTPARAM
-   bool "NSA SELinux boot parameter"
-   depends on SECURITY_SELINUX
-   default n
-   help
- This option adds a kernel parameter 'selinux', which allows SELinux
- to be disabled at boot.  If this option is selected, SELinux
- functionality can be disabled with selinux=0 on the kernel
- command line.  The purpose of this option is to allow a single
- kernel image to be distributed with SELinux built in, but not
- necessarily enabled.
-
- If you are unsure how to answer this question, answer N.
-
-config SECURITY_SELINUX_BOOTPARAM_VALUE
-   int "NSA SELinux boot parameter default value"
-   depends on SECURITY_SELINUX_BOOTPARAM
-   range 0 1
-   default 1
-   help
- This option sets the default value for the kernel parameter
- 'selinux', which allows SELinux to be disabled at boot.  If this
- option is set to 0 (zero), the SELinux kernel parameter will
- default to 0, disabling SELinux at bootup.  If this option is
- set to 1 (one), the SELinux kernel parameter will default to 1,
- enabling SELinux at bootup.
-
- If you are unsure how to answer this question, answer 1.
-
 config SECURITY_SELINUX_DISABLE
bool "NSA SELinux runtime disable"
depends on SECURITY_SELINUX
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 71a10fedecb3..8f5eea097612 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -120,20 +120,7 @@ __setup("enforcing=", enforcing_setup);
 #define selinux_enforcing_boot 1
 #endif
 
-#ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM
-int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE;
-
-static int __init selinux_enabled_setup(char *str)
-{
-   unsigned long enabled;
-   if (!kstrtoul(str, 0, &enabled))
-   selinux_enabled = enabled ? 1 : 0;
-   return 1;
-}
-__setup("selinux=", selinux_enabled_setup);
-#else
-int selinux_enabled = 1;
-#endif
+int selinux_enabled __lsm_ro_after_init;
 
 static unsigned int selinux_checkreqprot_boot =
CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE;
-- 
2.17.1

[PATCH security-next v4 21/32] LSM: Finalize centralized LSM enabling logic

[Kees Cook]

Prior to this patch, default "enable" behavior was unchanged: SELinux
and AppArmor were controlled separately from the centralized control
defined by CONFIG_LSM_ENABLE and "lsm.enable=...". This changes the
logic to give all control over to the central logic.

Instead of allowing SELinux and AppArmor to override the central LSM
enabling logic, by having separate CONFIG and boot parameters, this
forces all "enable" variables to disabled, then enables any listed in
the CONFIG_LSM_ENABLE and "lsm.enable=..." settings, and finally disables
any listed in "lsm.disable=...".

Signed-off-by: Kees Cook 
---
 .../admin-guide/kernel-parameters.txt |  6 ++--
 include/linux/lsm_hooks.h |  2 +-
 security/security.c   | 32 +++
 3 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 67c90985d2b8..f646cfab5613 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2279,14 +2279,12 @@
lsm.disable=lsm1,...,lsmN
[SECURITY] Comma-separated list of LSMs to disable
at boot time. This overrides "lsm.enable=",
-   CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot
-   parameters.
+   CONFIG_LSM_ENABLE.
 
lsm.enable=lsm1,...,lsmN
[SECURITY] Comma-separated list of LSMs to enable
at boot time. This overrides any omissions from
-   CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and
-   boot parameters.
+   CONFIG_LSM_ENABLE.
 
machvec=[IA-64] Force the use of a particular machine-vector
(machvec) in a generic kernel.
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index fd85637a1931..b026ea93ff01 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2044,7 +2044,7 @@ extern void security_add_hooks(struct security_hook_list 
*hooks, int count,
 struct lsm_info {
const char *name;   /* Required. */
unsigned long flags;/* Optional: flags describing LSM */
-   int *enabled;   /* Optional: NULL checks CONFIG_LSM_ENABLE */
+   int *enabled;   /* Optional: set based on CONFIG_LSM_ENABLE */
int (*init)(void);  /* Required. */
 };
 
diff --git a/security/security.c b/security/security.c
index d7132c181ea6..40b9f508b856 100644
--- a/security/security.c
+++ b/security/security.c
@@ -63,27 +63,19 @@ static bool __init is_enabled(struct lsm_info *lsm)
 /* Mark an LSM's enabled flag, if it exists. */
 static int lsm_enabled_true __initdata = 1;
 static int lsm_enabled_false __initdata = 0;
-
-static void __init default_enabled(struct lsm_info *lsm, bool enabled)
+static void __init set_enabled(struct lsm_info *lsm, bool enabled)
 {
-   /* If storage location already set, skip this one. */
-   if (lsm->enabled)
-   return;
-
/*
 * When an LSM hasn't configured an enable variable, we can use
 * a hard-coded location for storing the default enabled state.
 */
-   if (enabled)
-   lsm->enabled = &lsm_enabled_true;
-   else
-   lsm->enabled = &lsm_enabled_false;
-}
-
-static void __init set_enabled(struct lsm_info *lsm, bool enabled)
-{
-   if (WARN_ON(!lsm->enabled))
+   if (!lsm->enabled) {
+   if (enabled)
+   lsm->enabled = &lsm_enabled_true;
+   else
+   lsm->enabled = &lsm_enabled_false;
return;
+   }
 
if (lsm->enabled == &lsm_enabled_true) {
if (!enabled)
@@ -149,7 +141,6 @@ static void __init major_lsm_init(void)
 }
 
 static void __init parse_lsm_enable(const char *str,
-   void (*set)(struct lsm_info *, bool),
bool enabled)
 {
char *sep, *name, *next;
@@ -165,7 +156,7 @@ static void __init parse_lsm_enable(const char *str,
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
if (strcmp(name, "all") == 0 ||
strcmp(name, lsm->name) == 0)
-   set(lsm, enabled);
+   set_enabled(lsm, enabled);
}
}
kfree(sep);
@@ -174,11 +165,12 @@ static void __init parse_lsm_enable(const char *str,
 static void __init prepare_lsm_enable(void)
 {
/* Prepare defaults. */
-   parse_lsm_enable(builtin_lsm_enable, default_enabled, true);
+   parse_lsm_enable("all", false);
+   parse_lsm_enable(builtin_lsm_enable, true);
 
/* Process "lsm.enable=" and "lsm.disable=", if given. */
-   parse_lsm_enable(c

[PATCH security-next v4 22/32] apparmor: Remove boot parameter

[Kees Cook]

Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and
"lsm.enable=...", this removes the LSM-specific enabling logic from
AppArmor, though it leaves the existing userspace API visibility into
/sys/module/apparmor/parameters/enabled.

Co-developed-by: John Johansen 
Signed-off-by: Kees Cook 
---
 Documentation/admin-guide/kernel-parameters.txt |  7 ---
 security/apparmor/Kconfig   | 16 
 security/apparmor/lsm.c |  7 ++-
 3 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index f646cfab5613..cf963febebb0 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4054,13 +4054,6 @@
If enabled at boot time, /selinux/disable can be used
later to disable prior to initial policy load.
 
-   apparmor=   [APPARMOR] Disable or enable AppArmor at boot time
-   Format: { "0" | "1" }
-   See security/apparmor/Kconfig help text
-   0 -- disable.
-   1 -- enable.
-   Default value is set via kernel config option.
-
serialnumber[BUGS=X86-32]
 
shapers=[NET]
diff --git a/security/apparmor/Kconfig b/security/apparmor/Kconfig
index b6b68a7750ce..3de21f46c82a 100644
--- a/security/apparmor/Kconfig
+++ b/security/apparmor/Kconfig
@@ -14,22 +14,6 @@ config SECURITY_APPARMOR
 
  If you are unsure how to answer this question, answer N.
 
-config SECURITY_APPARMOR_BOOTPARAM_VALUE
-   int "AppArmor boot parameter default value"
-   depends on SECURITY_APPARMOR
-   range 0 1
-   default 1
-   help
- This option sets the default value for the kernel parameter
- 'apparmor', which allows AppArmor to be enabled or disabled
-  at boot.  If this option is set to 0 (zero), the AppArmor
- kernel parameter will default to 0, disabling AppArmor at
- boot.  If this option is set to 1 (one), the AppArmor
- kernel parameter will default to 1, enabling AppArmor at
- boot.
-
- If you are unsure how to answer this question, answer 1.
-
 config SECURITY_APPARMOR_HASH
bool "Enable introspection of sha1 hashes for loaded profiles"
depends on SECURITY_APPARMOR
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index bc56b058dc75..4cd96a66ed6f 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1303,15 +1303,12 @@ bool aa_g_paranoid_load = true;
 module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
 
 /* Boot time disable flag */
-static int apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
+static int apparmor_enabled __lsm_ro_after_init;
 module_param_named(enabled, apparmor_enabled, int, 0444);
 
 static int __init apparmor_enabled_setup(char *str)
 {
-   unsigned long enabled;
-   int error = kstrtoul(str, 0, &enabled);
-   if (!error)
-   apparmor_enabled = enabled ? 1 : 0;
+   pr_err("Boot param 'apparmor=' ignored. Use 'lsm.disable=apparmor'\n");
return 1;
 }
 
-- 
2.17.1

[PATCH security-next v4 16/32] LSM: Prepare for arbitrary LSM enabling

[Kees Cook]

Before now, all the LSMs that did not specify an "enable" variable in their
struct lsm_info were considered enabled by default. This prepares to make
LSM enabling more explicit. For all LSMs without an explicit "enable"
variable, a hard-coded storage location is chosen, and all LSMs without
an external "enable" state have their state explicitly set to "enabled".

This code appears more complex than it needs to be (comma-separated
list parsing and "set" function parameter) because its use will be
expanded on in the following patches to provide more explicit enabling.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
Reviewed-by: John Johansen 
---
 security/security.c | 69 ++---
 1 file changed, 65 insertions(+), 4 deletions(-)

diff --git a/security/security.c b/security/security.c
index 4e5e67b82b7b..9459b4ee4fd9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -54,17 +54,46 @@ static __initdata bool debug;
 
 static bool __init is_enabled(struct lsm_info *lsm)
 {
-   if (!lsm->enabled || *lsm->enabled)
-   return true;
+   if (WARN_ON(!lsm->enabled))
+   return false;
 
-   return false;
+   return *lsm->enabled;
 }
 
 /* Mark an LSM's enabled flag, if it exists. */
-static void __init set_enabled(struct lsm_info *lsm, bool enabled)
+static int lsm_enabled_true __initdata = 1;
+static int lsm_enabled_false __initdata = 0;
+
+static void __init default_enabled(struct lsm_info *lsm, bool enabled)
 {
+   /* If storage location already set, skip this one. */
if (lsm->enabled)
+   return;
+
+   /*
+* When an LSM hasn't configured an enable variable, we can use
+* a hard-coded location for storing the default enabled state.
+*/
+   if (enabled)
+   lsm->enabled = &lsm_enabled_true;
+   else
+   lsm->enabled = &lsm_enabled_false;
+}
+
+static void __init set_enabled(struct lsm_info *lsm, bool enabled)
+{
+   if (WARN_ON(!lsm->enabled))
+   return;
+
+   if (lsm->enabled == &lsm_enabled_true) {
+   if (!enabled)
+   lsm->enabled = &lsm_enabled_false;
+   } else if (lsm->enabled == &lsm_enabled_false) {
+   if (enabled)
+   lsm->enabled = &lsm_enabled_true;
+   } else {
*lsm->enabled = enabled;
+   }
 }
 
 /* Is an LSM allowed to be initialized? */
@@ -127,6 +156,35 @@ static void __init major_lsm_init(void)
}
 }
 
+static void __init parse_lsm_enable(const char *str,
+   void (*set)(struct lsm_info *, bool),
+   bool enabled)
+{
+   char *sep, *name, *next;
+
+   if (!str)
+   return;
+
+   sep = kstrdup(str, GFP_KERNEL);
+   next = sep;
+   while ((name = strsep(&next, ",")) != NULL) {
+   struct lsm_info *lsm;
+
+   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   if (strcmp(name, "all") == 0 ||
+   strcmp(name, lsm->name) == 0)
+   set(lsm, enabled);
+   }
+   }
+   kfree(sep);
+}
+
+static void __init prepare_lsm_enable(void)
+{
+   /* Prepare defaults. */
+   parse_lsm_enable("all", default_enabled, true);
+}
+
 /**
  * security_init - initializes the security framework
  *
@@ -143,6 +201,9 @@ int __init security_init(void)
 i++)
INIT_HLIST_HEAD(&list[i]);
 
+   /* Figure out which LSMs are enabled and disabled. */
+   prepare_lsm_enable();
+
/*
 * Load minor LSMs, with the capability module always first.
 */
-- 
2.17.1

[PATCH security-next v4 25/32] LSM: Introduce CONFIG_LSM_ORDER

[Kees Cook]

This provides a way to declare LSM initialization order via Kconfig.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 security/Kconfig| 16 
 security/security.c | 40 +---
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/security/Kconfig b/security/Kconfig
index 1e57619fd561..c68520d97fd7 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -286,5 +286,21 @@ config LSM_ENABLE
  changed with the "lsm.enable=" and "lsm.disable=" boot
  parameters.
 
+ Note that any enabled exclusive LSM modules will be initialized
+ based on LSM ordering, automatically disabling any following
+ exclusive LSMs. See CONFIG_LSM_ORDER for more details on
+ changing LSM initialization order.
+
+config LSM_ORDER
+   string "Default initialization order of builtin LSMs"
+   default "integrity"
+   help
+ A comma-separated list of LSMs, in initialization order.
+ Any LSMs left off this list will be link-order initialized
+ after any listed LSMs. Any LSMs listed here but not built in
+ the kernel will be ignored.
+
+ If unsure, leave this as the default.
+
 endmenu
 
diff --git a/security/security.c b/security/security.c
index 8706b42b4d44..0510bb8e0af0 100644
--- a/security/security.c
+++ b/security/security.c
@@ -47,6 +47,7 @@ static __initdata const char *chosen_lsm_disable;
 static __initdata const char *chosen_major_lsm;
 
 static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
+static __initconst const char * const builtin_lsm_order = CONFIG_LSM_ORDER;
 
 /* Ordered list of LSMs to initialize. */
 static __initdata struct lsm_info **ordered_lsms;
@@ -122,14 +123,47 @@ static void __init append_ordered_lsm(struct lsm_info 
*lsm, const char *from)
   is_enabled(lsm) ? "en" : "dis");
 }
 
-/* Populate ordered LSMs list from hard-coded list of LSMs. */
+/* Populate ordered LSMs list from given string. */
+static void __init parse_lsm_order(const char *order, const char *origin)
+{
+   struct lsm_info *lsm;
+   char *sep, *name, *next;
+
+   if (!order)
+   return;
+
+   sep = kstrdup(order, GFP_KERNEL);
+   next = sep;
+   /* Walk the list, looking for matching LSMs. */
+   while ((name = strsep(&next, ",")) != NULL) {
+   bool found = false;
+
+   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0 &&
+   strcmp(lsm->name, name) == 0) {
+   append_ordered_lsm(lsm, origin);
+   found = true;
+   }
+   }
+
+   if (!found)
+   init_debug("%s ignored: %s\n", origin, name);
+   }
+   kfree(sep);
+}
+
+/* Populate ordered LSMs list from builtin list of LSMs. */
 static void __init prepare_lsm_order(void)
 {
struct lsm_info *lsm;
 
+   /* Parse order from builtin list. */
+   parse_lsm_order(builtin_lsm_order, "builtin");
+
+   /* Add any missing LSMs, in link order. */
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
-   if (strcmp(lsm->name, "integrity") == 0)
-   append_ordered_lsm(lsm, "builtin");
+   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
+   append_ordered_lsm(lsm, "link-time");
}
 }
 
-- 
2.17.1

[PATCH security-next v4 24/32] LSM: Build ordered list of ordered LSMs for init

[Kees Cook]

This constructs a list of ordered LSMs to initialize, using a hard-coded
list of only "integrity": minor LSMs continue to have direct hook calls,
and major LSMs continue to initialize separately.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 security/security.c | 59 +++--
 1 file changed, 52 insertions(+), 7 deletions(-)

diff --git a/security/security.c b/security/security.c
index 40b9f508b856..8706b42b4d44 100644
--- a/security/security.c
+++ b/security/security.c
@@ -34,6 +34,9 @@
 
 #define MAX_LSM_EVM_XATTR  2
 
+/* How many LSMs were built into the kernel? */
+#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
+
 struct security_hook_heads security_hook_heads __lsm_ro_after_init;
 static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
 
@@ -45,6 +48,9 @@ static __initdata const char *chosen_major_lsm;
 
 static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
 
+/* Ordered list of LSMs to initialize. */
+static __initdata struct lsm_info **ordered_lsms;
+
 static __initdata bool debug;
 #define init_debug(...)\
do {\
@@ -88,6 +94,45 @@ static void __init set_enabled(struct lsm_info *lsm, bool 
enabled)
}
 }
 
+/* Is an LSM already listed in the ordered LSMs list? */
+static bool __init exists_ordered_lsm(struct lsm_info *lsm)
+{
+   struct lsm_info **check;
+
+   for (check = ordered_lsms; *check; check++)
+   if (*check == lsm)
+   return true;
+
+   return false;
+}
+
+/* Append an LSM to the list of ordered LSMs to initialize. */
+static int last_lsm __initdata;
+static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from)
+{
+   /* Ignore duplicate selections. */
+   if (exists_ordered_lsm(lsm))
+   return;
+
+   if (WARN(last_lsm == LSM_COUNT, "%s: out of LSM slots!?\n", from))
+   return;
+
+   ordered_lsms[last_lsm++] = lsm;
+   init_debug("%s ordering: %s (%sabled)\n", from, lsm->name,
+  is_enabled(lsm) ? "en" : "dis");
+}
+
+/* Populate ordered LSMs list from hard-coded list of LSMs. */
+static void __init prepare_lsm_order(void)
+{
+   struct lsm_info *lsm;
+
+   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+   if (strcmp(lsm->name, "integrity") == 0)
+   append_ordered_lsm(lsm, "builtin");
+   }
+}
+
 /* Is an LSM allowed to be initialized? */
 static bool __init lsm_allowed(struct lsm_info *lsm)
 {
@@ -118,14 +163,10 @@ static void __init maybe_initialize_lsm(struct lsm_info 
*lsm)
 
 static void __init ordered_lsm_init(void)
 {
-   struct lsm_info *lsm;
-
-   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
-   if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) != 0)
-   continue;
+   struct lsm_info **lsm;
 
-   maybe_initialize_lsm(lsm);
-   }
+   for (lsm = ordered_lsms; *lsm; lsm++)
+   maybe_initialize_lsm(*lsm);
 }
 
 static void __init major_lsm_init(void)
@@ -207,6 +248,8 @@ int __init security_init(void)
for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
 i++)
INIT_HLIST_HEAD(&list[i]);
+   ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
+   GFP_KERNEL);
 
/* Figure out which LSMs are enabled and disabled. */
prepare_lsm_enable();
@@ -219,6 +262,7 @@ int __init security_init(void)
loadpin_add_hooks();
 
/* Load LSMs in specified order. */
+   prepare_lsm_order();
ordered_lsm_init();
 
/*
@@ -226,6 +270,7 @@ int __init security_init(void)
 */
major_lsm_init();
 
+   kfree(ordered_lsms);
return 0;
 }
 
-- 
2.17.1

[PATCH v9 1/2] dt-bindings: hwmon: Add ina3221 documentation

[Nicolin Chen]

Texas Instruments INA3221 is a triple-channel shunt and bus
voltage monitor. This patch adds a DT binding doc for it.

Signed-off-by: Nicolin Chen 
---
Changelog
v7->v9:
 * N/A
v6->v7:
 * Restored three channel examples and merged them with the parent one
v5->v6:
 * Removed status property as no need to explicitly list it.
 * Combined all examples into a complete one.
v4->v5:
 * Replaced "input-id" with "reg" and added address-cells and size-cells
 * Replaced "input-label" with "label"
 * Replaced "shunt-resistor" with "shunt-resistor-micro-ohms"
v3->v4:
 * Removed the attempt of putting labels in the node names
 * Added a new optional label property in the child node
 * Updated examples accordingly
v2->v3:
 * Added a simple subject in the line 1
 * Fixed the shunt resistor value in the example
v1->v2:
 * Dropped channel name properties
 * Added child node definitions.
 * * Added shunt resistor property in the child node
 * * Added status property to indicate connection status
 * * Changed to use child node name as the label of input source

 .../devicetree/bindings/hwmon/ina3221.txt | 44 +++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt

diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt 
b/Documentation/devicetree/bindings/hwmon/ina3221.txt
new file mode 100644
index ..a7b25caa2b8e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ina3221.txt
@@ -0,0 +1,44 @@
+Texas Instruments INA3221 Device Tree Bindings
+
+1) ina3221 node
+  Required properties:
+  - compatible: Must be "ti,ina3221"
+  - reg: I2C address
+
+  Optional properties:
+  = The node contains optional child nodes for three channels =
+  = Each child node describes the information of input source =
+
+  - #address-cells: Required only if a child node is present. Must be 1.
+  - #size-cells: Required only if a child node is present. Must be 0.
+
+2) child nodes
+  Required properties:
+  - reg: Must be 0, 1 or 2, corresponding to IN1, IN2 or IN3 port of INA3221
+
+  Optional properties:
+  - label: Name of the input source
+  - shunt-resistor-micro-ohms: Shunt resistor value in micro-Ohm
+
+Example:
+
+ina3221@40 {
+   compatible = "ti,ina3221";
+   reg = <0x40>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   input@0 {
+   reg = <0x0>;
+   status = "disabled";
+   };
+   input@1 {
+   reg = <0x1>;
+   shunt-resistor-micro-ohms = <5000>;
+   };
+   input@2 {
+   reg = <0x2>;
+   label = "VDD_5V";
+   shunt-resistor-micro-ohms = <5000>;
+   };
+};
-- 
2.17.1

[PATCH security-next v4 18/32] LSM: Introduce lsm.enable= and lsm.disable=

[Kees Cook]

This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
which each can contain a comma-separated list of LSMs to enable or
disable, respectively. The string "all" matches all LSMs.

This has very similar functionality to the existing per-LSM enable
handling ("apparmor.enabled=...", etc), but provides a centralized
place to perform the changes. These parameters take precedent over any
LSM-specific boot parameters.

Disabling an LSM means it will not be considered when performing
initializations. Enabling an LSM means either undoing a previous
LSM-specific boot parameter disabling or a undoing a default-disabled
CONFIG setting.

For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
result in SELinux being enabled.

Signed-off-by: Kees Cook 
Reviewed-by: Casey Schaufler 
---
 .../admin-guide/kernel-parameters.txt | 12 ++
 security/Kconfig  |  4 +++-
 security/security.c   | 22 +++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 32d323ee9218..67c90985d2b8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2276,6 +2276,18 @@
 
lsm.debug   [SECURITY] Enable LSM initialization debugging output.
 
+   lsm.disable=lsm1,...,lsmN
+   [SECURITY] Comma-separated list of LSMs to disable
+   at boot time. This overrides "lsm.enable=",
+   CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot
+   parameters.
+
+   lsm.enable=lsm1,...,lsmN
+   [SECURITY] Comma-separated list of LSMs to enable
+   at boot time. This overrides any omissions from
+   CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and
+   boot parameters.
+
machvec=[IA-64] Force the use of a particular machine-vector
(machvec) in a generic kernel.
Example: machvec=hpzx1_swiotlb
diff --git a/security/Kconfig b/security/Kconfig
index ac23feba584d..1e57619fd561 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -282,7 +282,9 @@ config LSM_ENABLE
help
  A comma-separated list of LSMs to enable by default at boot. The
  default is "all", to enable all LSM modules at boot. Any LSMs
- not listed here will be disabled by default.
+ not listed here will be disabled by default. This can be
+ changed with the "lsm.enable=" and "lsm.disable=" boot
+ parameters.
 
 endmenu
 
diff --git a/security/security.c b/security/security.c
index 35601000176b..3fff2d1d1ec4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -44,6 +44,8 @@ char *lsm_names;
 /* Boot-time LSM user choice */
 static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
CONFIG_DEFAULT_SECURITY;
+static __initdata const char *chosen_lsm_enable;
+static __initdata const char *chosen_lsm_disable;
 
 static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE;
 
@@ -185,6 +187,10 @@ static void __init prepare_lsm_enable(void)
 {
/* Prepare defaults. */
parse_lsm_enable(builtin_lsm_enable, default_enabled, true);
+
+   /* Process "lsm.enable=" and "lsm.disable=", if given. */
+   parse_lsm_enable(chosen_lsm_enable, set_enabled, true);
+   parse_lsm_enable(chosen_lsm_disable, set_enabled, false);
 }
 
 /**
@@ -240,6 +246,22 @@ static int __init enable_debug(char *str)
 }
 __setup("lsm.debug", enable_debug);
 
+/* Explicitly enable a list of LSMs. */
+static int __init enable_lsm(char *str)
+{
+   chosen_lsm_enable = str;
+   return 1;
+}
+__setup("lsm.enable=", enable_lsm);
+
+/* Explicitly disable a list of LSMs. */
+static int __init disable_lsm(char *str)
+{
+   chosen_lsm_disable = str;
+   return 1;
+}
+__setup("lsm.disable=", disable_lsm);
+
 static bool match_last_lsm(const char *list, const char *lsm)
 {
const char *last;
-- 
2.17.1

[PATCH v9 2/2] hwmon: (ina3221) Read channel input source info from DT

[Nicolin Chen]

An ina3221 chip has three input ports. Each port is used
to measure the voltage and current of its input source.

The DT binding now has defined bindings for their input
sources, so the driver should read these information and
handle accordingly.

This patch adds a new structure of input source specific
information including input source label, shunt resistor
value and its connection status. It exposes these labels
via in[123]_label sysfs nodes upon available, and also
disables those channels where there are no input source
being connected. Meanwhile, it also adds in[123]_enable
sysfs nodes for users to get control of three channels,
and returns -ENODATA code for any sensor read according
to hwmon ABI.

Signed-off-by: Nicolin Chen 
---
Changelog
v8->v9:
 * Renamed the subject by following hwmon naming convention
 * Rebased the patch after the merged ina3221 changes
 * Fixed the potential race condition in ina3221_set_enable()
v7->v8:
 * Rebased the change after the new suspend-resume patch
 * * Please apply suspend-resume patch prior to this one
 * Renamed ina3221_is_enable() to ina3221_is_enabled()
 * Removed "> 0" check in the is_enabled() function
 * Replaced regmap_read() with ina->reg_config in is_enabled()
 * Used kstrtobool() in the ina3221_set_enable() function
v6->v7:
 * N/A
v5->v6:
 * Removed the code of hiding disconnected channels
 * Added in[123]_label sysfs nodes to control channels
 * Added -ENODATA return for sensor read at disabled channels
v4->v5:
 * Replaced "shunt-resistor" with "shunt-resistor-micro-ohms"
 * Replaced "input-label" with "label"
 * Replaced "input-id" with "reg"
 * Simplified is_visible() by using index of the attr
 * Moved inX_label to index-0 and added comments for safety
v3->v4:
 * Added is_visible callback function to hide sysfs nodes
v2->v3:
 * N/A
v1->v2:
 * Added a structure for input source corresponding to DT bindings
 * Moved shunt resistor value to the structure
 * Defined in[123]_label sysfs nodes instead of name[123]_input
 * Updated probe() function to get information from DT
 * Updated ina3221 hwmon documentation for the labels
 * Dropped dynamical group definition to keep all groups as they were

 Documentation/hwmon/ina3221 |   2 +
 drivers/hwmon/ina3221.c | 235 +---
 2 files changed, 223 insertions(+), 14 deletions(-)

diff --git a/Documentation/hwmon/ina3221 b/Documentation/hwmon/ina3221
index 0ff74854cb2e..4b82cbfb551c 100644
--- a/Documentation/hwmon/ina3221
+++ b/Documentation/hwmon/ina3221
@@ -21,6 +21,8 @@ and power are calculated host-side from these.
 Sysfs entries
 -
 
+in[123]_label   Voltage channel labels
+in[123]_enable  Voltage channel enable controls
 in[123]_input   Bus voltage(mV) channels
 curr[123]_input Current(mA) measurement channels
 shunt[123]_resistor Shunt resistance(uOhm) channels
diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index 1e38b4c43fbf..88e65d2b4fa5 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -43,6 +43,7 @@
 #define INA3221_CONFIG_MODE_SHUNT  BIT(0)
 #define INA3221_CONFIG_MODE_BUSBIT(1)
 #define INA3221_CONFIG_MODE_CONTINUOUS BIT(2)
+#define INA3221_CONFIG_CHx_EN(x)   BIT(14 - (x))
 
 #define INA3221_RSHUNT_DEFAULT 1
 
@@ -77,6 +78,9 @@ enum ina3221_channels {
 };
 
 static const unsigned int register_channel[] = {
+   [INA3221_BUS1] = INA3221_CHANNEL1,
+   [INA3221_BUS2] = INA3221_CHANNEL2,
+   [INA3221_BUS3] = INA3221_CHANNEL3,
[INA3221_SHUNT1] = INA3221_CHANNEL1,
[INA3221_SHUNT2] = INA3221_CHANNEL2,
[INA3221_SHUNT3] = INA3221_CHANNEL3,
@@ -88,20 +92,89 @@ static const unsigned int register_channel[] = {
[INA3221_WARN3] = INA3221_CHANNEL3,
 };
 
+/**
+ * struct ina3221_input - channel input source specific information
+ * @label: label of channel input source
+ * @shunt_resistor: shunt resistor value of channel input source
+ * @disconnected: connection status of channel input source
+ */
+struct ina3221_input {
+   const char *label;
+   int shunt_resistor;
+   bool disconnected;
+};
+
 /**
  * struct ina3221_data - device specific information
  * @regmap: Register map of the device
  * @fields: Register fields of the device
- * @shunt_resistors: Array of resistor values per channel
+ * @inputs: Array of channel input source specific structures
  * @reg_config: Register value of INA3221_CONFIG
  */
 struct ina3221_data {
struct regmap *regmap;
struct regmap_field *fields[F_MAX_FIELDS];
-   int shunt_resistors[INA3221_NUM_CHANNELS];
+   struct ina3221_input inputs[INA3221_NUM_CHANNELS];
u32 reg_config;
 };
 
+static inline bool ina3221_is_enabled(struct ina3221_data *ina, int channel)
+{
+   return ina->reg_config & INA3221_CONFIG_CHx_EN(channel);
+}
+
+static ssize_t ina3221_show_label(struct device *dev,
+ struct device_attribute *

[PATCH v9 0/2] Add an initial DT binding doc for ina3221

[Nicolin Chen]

This series adds a initial DT binding doc for ina3221. It defines
a child node to describe the input source of each ina3221 channel.
Then it changes the driver to handle the information properly.

Changelog
v8->v9:
 * Fixed a potential race condition (PATCH-2)
v7->v8:
 * Refined two places in ina3221 driver (PATCH-2)
v6->v7:
 * Refined the example in the binding doc (PATCH-1)
v5->v6:
 * Removed status property and merged examples (PATCH-1)
 * Added in[123]_enable sysfs nodes (PATCH-2)
v4->v5:
 * Changed two property names of child node (PATCH-1)
 * Changed the driver accordingly and simplified is_visible (PATCH-2)
v3->v4:
 * Fixed one place in child DT node bindings (PATCH-1)
 * Changed the driver accordingly and added is_visible (PATCH-2)
v2->v3:
 * Fixed two places in DT bindings (PATCH-1)
v1->v2:
 * Redefined DT bindings (detail in PATCH-1)
 * Changed the driver code accordingly (detail in PATCH-2)

Nicolin Chen (2):
  dt-bindings: hwmon: Add ina3221 documentation
  hwmon: (ina3221) Read channel input source info from DT

 .../devicetree/bindings/hwmon/ina3221.txt |  44 
 Documentation/hwmon/ina3221   |   2 +
 drivers/hwmon/ina3221.c   | 235 --
 3 files changed, 267 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt

-- 
2.17.1

Re: [PATCH security-next v4 13/32] LoadPin: Rename "enable" to "enforce"

[Randy Dunlap]

On 10/1/18 5:54 PM, Kees Cook wrote:
> LoadPin's "enable" setting is really about enforcement, not whether
> or not the LSM is using LSM hooks. Instead, split this out so that LSM
> enabling can be logically distinct from whether enforcement is happening
> (for example, the pinning happens when the LSM is enabled, but the pin
> is only checked when "enforce" is set). This allows LoadPin to continue

ISTB: when "enforcing" is set).  ??

> to operate sanely in test environments once LSM enable/disable is
> centrally handled (i.e. we want LoadPin to be enabled separately from
> its enforcement).
> 
> Signed-off-by: Kees Cook 
> Reviewed-by: Casey Schaufler 
> Reviewed-by: John Johansen 
> ---
>  security/loadpin/Kconfig   |  4 ++--
>  security/loadpin/loadpin.c | 21 +++--
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
> index dd01aa91e521..8653608a3693 100644
> --- a/security/loadpin/Kconfig
> +++ b/security/loadpin/Kconfig
> @@ -10,10 +10,10 @@ config SECURITY_LOADPIN
> have a root filesystem backed by a read-only device such as
> dm-verity or a CDROM.
>  
> -config SECURITY_LOADPIN_ENABLED
> +config SECURITY_LOADPIN_ENFORCING
>   bool "Enforce LoadPin at boot"
>   depends on SECURITY_LOADPIN
>   help
> If selected, LoadPin will enforce pinning at boot. If not
> selected, it can be enabled at boot with the kernel parameter
> -   "loadpin.enabled=1".
> +   "loadpin.enforcing=1".
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 0716af28808a..d8a68a6f6fef 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -44,7 +44,7 @@ static void report_load(const char *origin, struct file 
> *file, char *operation)
>   kfree(pathname);
>  }
>  
> -static int enabled = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENABLED);
> +static int enforcing = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENFORCING);
>  static struct super_block *pinned_root;
>  static DEFINE_SPINLOCK(pinned_root_spinlock);
>  
> @@ -60,8 +60,8 @@ static struct ctl_path loadpin_sysctl_path[] = {
>  
>  static struct ctl_table loadpin_sysctl_table[] = {
>   {
> - .procname   = "enabled",
> - .data   = &enabled,
> + .procname   = "enforcing",
> + .data   = &enforcing,
>   .maxlen = sizeof(int),
>   .mode   = 0644,
>   .proc_handler   = proc_dointvec_minmax,
> @@ -97,7 +97,7 @@ static void check_pinning_enforcement(struct super_block 
> *mnt_sb)
>  loadpin_sysctl_table))
>   pr_notice("sysctl registration failed!\n");
>   else
> - pr_info("load pinning can be disabled.\n");
> + pr_info("enforcement can be disabled.\n");
>   } else
>   pr_info("load pinning engaged.\n");
>  }
> @@ -128,7 +128,7 @@ static int loadpin_read_file(struct file *file, enum 
> kernel_read_file_id id)
>  
>   /* This handles the older init_module API that has a NULL file. */
>   if (!file) {
> - if (!enabled) {
> + if (!enforcing) {
>   report_load(origin, NULL, "old-api-pinning-ignored");
>   return 0;
>   }
> @@ -151,7 +151,7 @@ static int loadpin_read_file(struct file *file, enum 
> kernel_read_file_id id)
>* Unlock now since it's only pinned_root we care about.
>* In the worst case, we will (correctly) report pinning
>* failures before we have announced that pinning is
> -  * enabled. This would be purely cosmetic.
> +  * enforcing. This would be purely cosmetic.
>*/
>   spin_unlock(&pinned_root_spinlock);
>   check_pinning_enforcement(pinned_root);
> @@ -161,7 +161,7 @@ static int loadpin_read_file(struct file *file, enum 
> kernel_read_file_id id)
>   }
>  
>   if (IS_ERR_OR_NULL(pinned_root) || load_root != pinned_root) {
> - if (unlikely(!enabled)) {
> + if (unlikely(!enforcing)) {
>   report_load(origin, file, "pinning-ignored");
>   return 0;
>   }
> @@ -186,10 +186,11 @@ static struct security_hook_list loadpin_hooks[] 
> __lsm_ro_after_init = {
>  
>  void __init loadpin_add_hooks(void)
>  {
> - pr_info("ready to pin (currently %sabled)", enabled ? "en" : "dis");
> + pr_info("ready to pin (currently %senforcing)\n",
> + enforcing ? "" : "not ");
>   security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
>  }
>  
>  /* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
> -module_param(enabled, int, 0);
> -MODULE_PARM_DESC(enabled, "Pin module/firmware loading (default: true)");
> +module

Re: [PATCH security-next v4 21/32] LSM: Finalize centralized LSM enabling logic

[Randy Dunlap]

On 10/1/18 5:54 PM, Kees Cook wrote:
> Prior to this patch, default "enable" behavior was unchanged: SELinux
> and AppArmor were controlled separately from the centralized control
> defined by CONFIG_LSM_ENABLE and "lsm.enable=...". This changes the
> logic to give all control over to the central logic.
> 
> Instead of allowing SELinux and AppArmor to override the central LSM
> enabling logic, by having separate CONFIG and boot parameters, this
> forces all "enable" variables to disabled, then enables any listed in
> the CONFIG_LSM_ENABLE and "lsm.enable=..." settings, and finally disables
> any listed in "lsm.disable=...".
> 
> Signed-off-by: Kees Cook 
> ---
>  .../admin-guide/kernel-parameters.txt |  6 ++--
>  include/linux/lsm_hooks.h |  2 +-
>  security/security.c   | 32 +++
>  3 files changed, 15 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index 67c90985d2b8..f646cfab5613 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2279,14 +2279,12 @@
>   lsm.disable=lsm1,...,lsmN
>   [SECURITY] Comma-separated list of LSMs to disable
>   at boot time. This overrides "lsm.enable=",

better:   This overrides "lsm.enable=" and

> - CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot
> - parameters.
> + CONFIG_LSM_ENABLE.
>  
>   lsm.enable=lsm1,...,lsmN
>   [SECURITY] Comma-separated list of LSMs to enable
>   at boot time. This overrides any omissions from
> - CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and
> - boot parameters.
> + CONFIG_LSM_ENABLE.
>  
>   machvec=[IA-64] Force the use of a particular machine-vector
>   (machvec) in a generic kernel.


-- 
~Randy

Re: [PATCH security-next v4 13/32] LoadPin: Rename "enable" to "enforce"

[Kees Cook]

On Mon, Oct 1, 2018 at 6:06 PM, Randy Dunlap  wrote:
> On 10/1/18 5:54 PM, Kees Cook wrote:
>> LoadPin's "enable" setting is really about enforcement, not whether
>> or not the LSM is using LSM hooks. Instead, split this out so that LSM
>> enabling can be logically distinct from whether enforcement is happening
>> (for example, the pinning happens when the LSM is enabled, but the pin
>> is only checked when "enforce" is set). This allows LoadPin to continue
>
> ISTB: when "enforcing" is set).  ??

Whoops, thanks. And I need to do s/enable/enabled/ in the log. I'll fix this up.

-Kees

>
>> to operate sanely in test environments once LSM enable/disable is
>> centrally handled (i.e. we want LoadPin to be enabled separately from
>> its enforcement).
>>
>> Signed-off-by: Kees Cook 
>> Reviewed-by: Casey Schaufler 
>> Reviewed-by: John Johansen 
>> ---
>>  security/loadpin/Kconfig   |  4 ++--
>>  security/loadpin/loadpin.c | 21 +++--
>>  2 files changed, 13 insertions(+), 12 deletions(-)
>>
>> diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
>> index dd01aa91e521..8653608a3693 100644
>> --- a/security/loadpin/Kconfig
>> +++ b/security/loadpin/Kconfig
>> @@ -10,10 +10,10 @@ config SECURITY_LOADPIN
>> have a root filesystem backed by a read-only device such as
>> dm-verity or a CDROM.
>>
>> -config SECURITY_LOADPIN_ENABLED
>> +config SECURITY_LOADPIN_ENFORCING
>>   bool "Enforce LoadPin at boot"
>>   depends on SECURITY_LOADPIN
>>   help
>> If selected, LoadPin will enforce pinning at boot. If not
>> selected, it can be enabled at boot with the kernel parameter
>> -   "loadpin.enabled=1".
>> +   "loadpin.enforcing=1".
>> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
>> index 0716af28808a..d8a68a6f6fef 100644
>> --- a/security/loadpin/loadpin.c
>> +++ b/security/loadpin/loadpin.c
>> @@ -44,7 +44,7 @@ static void report_load(const char *origin, struct file 
>> *file, char *operation)
>>   kfree(pathname);
>>  }
>>
>> -static int enabled = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENABLED);
>> +static int enforcing = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENFORCING);
>>  static struct super_block *pinned_root;
>>  static DEFINE_SPINLOCK(pinned_root_spinlock);
>>
>> @@ -60,8 +60,8 @@ static struct ctl_path loadpin_sysctl_path[] = {
>>
>>  static struct ctl_table loadpin_sysctl_table[] = {
>>   {
>> - .procname   = "enabled",
>> - .data   = &enabled,
>> + .procname   = "enforcing",
>> + .data   = &enforcing,
>>   .maxlen = sizeof(int),
>>   .mode   = 0644,
>>   .proc_handler   = proc_dointvec_minmax,
>> @@ -97,7 +97,7 @@ static void check_pinning_enforcement(struct super_block 
>> *mnt_sb)
>>  loadpin_sysctl_table))
>>   pr_notice("sysctl registration failed!\n");
>>   else
>> - pr_info("load pinning can be disabled.\n");
>> + pr_info("enforcement can be disabled.\n");
>>   } else
>>   pr_info("load pinning engaged.\n");
>>  }
>> @@ -128,7 +128,7 @@ static int loadpin_read_file(struct file *file, enum 
>> kernel_read_file_id id)
>>
>>   /* This handles the older init_module API that has a NULL file. */
>>   if (!file) {
>> - if (!enabled) {
>> + if (!enforcing) {
>>   report_load(origin, NULL, "old-api-pinning-ignored");
>>   return 0;
>>   }
>> @@ -151,7 +151,7 @@ static int loadpin_read_file(struct file *file, enum 
>> kernel_read_file_id id)
>>* Unlock now since it's only pinned_root we care about.
>>* In the worst case, we will (correctly) report pinning
>>* failures before we have announced that pinning is
>> -  * enabled. This would be purely cosmetic.
>> +  * enforcing. This would be purely cosmetic.
>>*/
>>   spin_unlock(&pinned_root_spinlock);
>>   check_pinning_enforcement(pinned_root);
>> @@ -161,7 +161,7 @@ static int loadpin_read_file(struct file *file, enum 
>> kernel_read_file_id id)
>>   }
>>
>>   if (IS_ERR_OR_NULL(pinned_root) || load_root != pinned_root) {
>> - if (unlikely(!enabled)) {
>> + if (unlikely(!enforcing)) {
>>   report_load(origin, file, "pinning-ignored");
>>   return 0;
>>   }
>> @@ -186,10 +186,11 @@ static struct security_hook_list loadpin_hooks[] 
>> __lsm_ro_after_init = {
>>
>>  void __init loadpin_add_hooks(void)
>>  {
>> - pr_info("ready to pin (currently %sabled)", enabled ? "en" : "dis");
>> + pr_info("ready to pin (currently %senforcing)\n",
>> + enforcing ? "" : "not ");
>>   security_add_hooks(loadpin_hooks, ARRAY_

Re: [PATCH security-next v4 21/32] LSM: Finalize centralized LSM enabling logic

[Kees Cook]

On Mon, Oct 1, 2018 at 6:18 PM, Randy Dunlap  wrote:
> On 10/1/18 5:54 PM, Kees Cook wrote:
>> Prior to this patch, default "enable" behavior was unchanged: SELinux
>> and AppArmor were controlled separately from the centralized control
>> defined by CONFIG_LSM_ENABLE and "lsm.enable=...". This changes the
>> logic to give all control over to the central logic.
>>
>> Instead of allowing SELinux and AppArmor to override the central LSM
>> enabling logic, by having separate CONFIG and boot parameters, this
>> forces all "enable" variables to disabled, then enables any listed in
>> the CONFIG_LSM_ENABLE and "lsm.enable=..." settings, and finally disables
>> any listed in "lsm.disable=...".
>>
>> Signed-off-by: Kees Cook 
>> ---
>>  .../admin-guide/kernel-parameters.txt |  6 ++--
>>  include/linux/lsm_hooks.h |  2 +-
>>  security/security.c   | 32 +++
>>  3 files changed, 15 insertions(+), 25 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
>> b/Documentation/admin-guide/kernel-parameters.txt
>> index 67c90985d2b8..f646cfab5613 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2279,14 +2279,12 @@
>>   lsm.disable=lsm1,...,lsmN
>>   [SECURITY] Comma-separated list of LSMs to disable
>>   at boot time. This overrides "lsm.enable=",
>
> better:   This overrides "lsm.enable=" and

Eek, yes! Thank you. :)

-Kees

>
>> - CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot
>> - parameters.
>> + CONFIG_LSM_ENABLE.
>>
>>   lsm.enable=lsm1,...,lsmN
>>   [SECURITY] Comma-separated list of LSMs to enable
>>   at boot time. This overrides any omissions from
>> - CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and
>> - boot parameters.
>> + CONFIG_LSM_ENABLE.
>>
>>   machvec=[IA-64] Force the use of a particular machine-vector
>>   (machvec) in a generic kernel.
>
>
> --
> ~Randy



-- 
Kees Cook
Pixel Security