On Thu, May 09, 2024 at 10:14:21PM +0200, KP Singh wrote: > BPF LSM hooks have side-effects (even when a default value's returned) > as some hooks end up behaving differently due to the very presence of > the hook. > > The static keys guarding the BPF LSM hooks are disabled by default and > enabled only when a BPF program is attached implementing the hook > logic. This avoids the issue of the side-effects and also the minor > overhead associated with the empty callback. > > security_file_ioctl: > 0xff...0e30 <+0>: endbr64 > 0xff...0e34 <+4>: nopl 0x0(%rax,%rax,1) > 0xff...0e39 <+9>: push %rbp > 0xff...0e3a <+10>: push %r14 > 0xff...0e3c <+12>: push %rbx > 0xff...0e3d <+13>: mov %rdx,%rbx > 0xff...0e40 <+16>: mov %esi,%ebp > 0xff...0e42 <+18>: mov %rdi,%r14 > 0xff...0e45 <+21>: jmp 0xff...0e57 <security_file_ioctl+39> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > Static key enabled for SELinux > > 0xff...0e47 <+23>: xchg %ax,%ax > ^^^^^^^^^^^^^^ > > Static key disabled for BPF. This gets patched when a BPF LSM > program is attached > > 0xff...0e49 <+25>: xor %eax,%eax > 0xff...0e4b <+27>: xchg %ax,%ax > 0xff...0e4d <+29>: pop %rbx > 0xff...0e4e <+30>: pop %r14 > 0xff...0e50 <+32>: pop %rbp > 0xff...0e51 <+33>: cs jmp 0xff...0000 <__x86_return_thunk> > 0xff...0e57 <+39>: endbr64 > 0xff...0e5b <+43>: mov %r14,%rdi > 0xff...0e5e <+46>: mov %ebp,%esi > 0xff...0e60 <+48>: mov %rbx,%rdx > 0xff...0e63 <+51>: call 0xff...33c0 <selinux_file_ioctl> > 0xff...0e68 <+56>: test %eax,%eax > 0xff...0e6a <+58>: jne 0xff...0e4d <security_file_ioctl+29> > 0xff...0e6c <+60>: jmp 0xff...0e47 <security_file_ioctl+23> > 0xff...0e6e <+62>: endbr64 > 0xff...0e72 <+66>: mov %r14,%rdi > 0xff...0e75 <+69>: mov %ebp,%esi > 0xff...0e77 <+71>: mov %rbx,%rdx > 0xff...0e7a <+74>: call 0xff...e3b0 <bpf_lsm_file_ioctl> > 0xff...0e7f <+79>: test %eax,%eax > 0xff...0e81 <+81>: jne 0xff...0e4d <security_file_ioctl+29> > 0xff...0e83 <+83>: jmp 0xff...0e49 <security_file_ioctl+25> > 0xff...0e85 <+85>: endbr64 > 0xff...0e89 <+89>: mov %r14,%rdi > 0xff...0e8c <+92>: mov %ebp,%esi > 0xff...0e8e <+94>: mov %rbx,%rdx > 0xff...0e91 <+97>: pop %rbx > 0xff...0e92 <+98>: pop %r14 > 0xff...0e94 <+100>: pop %rbp > 0xff...0e95 <+101>: ret > > This patch enables this by providing a LSM_HOOK_INIT_TOGGLEABLE > variant which allows the LSMs to opt-in to toggleable hooks which can > be toggled on/off with security_toogle_hook. > > Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx> With the issue Tetsuo noted fixed: Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> -- Kees Cook