> On Nov 14, 2024, at 9:29 AM, Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
[...]
>>
>>
>> The LSM inode information is obviously security sensitive, which I
>> presume would be be the motivation for Casey's concern that a 'mistake
>> by a BPF programmer could cause the whole system to blow up', which in
>> full disclosure is only a rough approximation of his statement.
>>
>> We obviously can't speak directly to Casey's concerns. Casey, any
>> specific technical comments on the challenges of using a common inode
>> specific storage architecture?
>
> My objection to using a union for the BPF and LSM pointer is based
> on the observation that a lot of modern programmers don't know what
> a union does. The BPF programmer would see that there are two ways
> to accomplish their task, one for CONFIG_SECURITY=y and the other
> for when it isn't. The second is much simpler. Not understanding
> how kernel configuration works, nor being "real" C language savvy,
> the programmer installs code using the simpler interfaces on a
> Redhat system. The SELinux inode data is compromised by the BPF
> code, which thinks the data is its own. Hilarity ensues.
There must be some serious misunderstanding here. So let me
explain the idea again.
With CONFIG_SECURITY=y, the code will work the same as right now.
BPF inode storage uses i_security, just as any other LSMs.
With CONFIG_SECURITY=n, i_security does not exist, so the bpf
inode storage will use i_bpf_storage.
Since this is a CONFIG_, all the logic got sorted out at compile
time. Thus the user API (for user space and for bpf programs)
stays the same.
Actually, I can understand the concern with union. Although,
the logic is set at kernel compile time, it is still possible
for kernel source code to use i_bpf_storage when
CONFIG_SECURITY is enabled. (Yes, I guess now I finally understand
the concern).
We can address this with something like following:
#ifdef CONFIG_SECURITY
void *i_security;
#elif CONFIG_BPF_SYSCALL
struct bpf_local_storage __rcu *i_bpf_storage;
#endif
This will help catch all misuse of the i_bpf_storage at compile
time, as i_bpf_storage doesn't exist with CONFIG_SECURITY=y.
Does this make sense?
Thanks,
Song
