lsm: add reserved flag in lsm_prop struct

Commit Message

15074444048 Dec. 6, 2024, 11:41 a.m. UTC

From: lihaojie <[e-mail]>

lsm_prop size is controled by macro, lsm_prop size will be 0
when marco don't define. add flag to alloc sm_prop basic size.

empty struct will make target_ref & target_comm in audit_context
located at the same address, __member_size of target_comm is
same as __member_size of target_ref, so strscpy warn buffer
overflow when compile time.

Signed-off-by: lihaojie <[e-mail]>
---
 include/linux/security.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Casey Schaufler Dec. 6, 2024, 5:31 p.m. UTC | #1

On 12/6/2024 3:41 AM, [e-mail] wrote:
> From: lihaojie <[e-mail]>
>
> lsm_prop size is controled by macro, lsm_prop size will be 0
> when marco don't define. add flag to alloc sm_prop basic size.
>
> empty struct will make target_ref & target_comm in audit_context
> located at the same address, __member_size of target_comm is
> same as __member_size of target_ref, so strscpy warn buffer
> overflow when compile time.

Can you cite where this warning occurs?

>
> Signed-off-by: lihaojie <[e-mail]>
> ---
>  include/linux/security.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index cbdba435b798..f502deecb142 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -164,6 +164,7 @@ struct lsm_prop {
>  	struct lsm_prop_smack smack;
>  	struct lsm_prop_apparmor apparmor;
>  	struct lsm_prop_bpf bpf;
> +	u8 reserved;
>  };

I don't care much for this approach. Increasing the size of the structure
to avoid a warning in the case where it isn't used seems problematic.

>  
>  extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];

15074444048 Dec. 9, 2024, 1:52 a.m. UTC | #2

At 2024-12-07 01:31:11, "Casey Schaufler" <[e-mail]> wrote:

>
>Can you cite where this warning occurs?
>

In file included from ./include/linux/string.h:389,
                 from ./include/linux/bitmap.h:13,
                 from ./include/linux/cpumask.h:12,
                 from ./include/linux/smp.h:13,
                 from ./include/linux/lockdep.h:14,
                 from ./include/linux/spinlock.h:63,
                 from ./include/linux/wait.h:9,
                 from ./include/linux/wait_bit.h:8,
                 from ./include/linux/fs.h:6,
                 from kernel/auditsc.c:37:
In function ‘sized_strscpy’,
    inlined from ‘__audit_ptrace’ at kernel/auditsc.c:2732:2:
./include/linux/fortify-string.h:293:3: error: call to ‘__write_overflow’ declared with attribute error: detected write beyond size of object (1st parameter)
  293 |   __write_overflow();
      |   ^~~~~~~~~~~~~~~~~~
In function ‘sized_strscpy’,
    inlined from ‘audit_signal_info_syscall’ at kernel/auditsc.c:2759:3:
./include/linux/fortify-string.h:293:3: error: call to ‘__write_overflow’ declared with attribute error: detected write beyond size of object (1st parameter)
  293 |   __write_overflow();



>
>I don't care much for this approach. Increasing the size of the structure
>to avoid a warning in the case where it isn't used seems problematic.
>

do you have any good sugestion?

Patch

diff --git a/include/linux/security.h b/include/linux/security.h
index cbdba435b798..f502deecb142 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -164,6 +164,7 @@  struct lsm_prop {
 	struct lsm_prop_smack smack;
 	struct lsm_prop_apparmor apparmor;
 	struct lsm_prop_bpf bpf;
+	u8 reserved;
 };
 
 extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];