linux-kernel - Re: [PATCH bpf-next 0/4] Make inode storage available to tracing prog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <B6C7997A-8A4F-4859-9817-8F73F883CF93@fb.com>
Date: Thu, 14 Nov 2024 22:30:29 +0000
From: Song Liu <songliubraving@...a.com>
To: James Bottomley <James.Bottomley@...senPartnership.com>
CC: Song Liu <songliubraving@...a.com>,
        Casey Schaufler
	<casey@...aufler-ca.com>,
        "Dr. Greg" <greg@...ellic.com>, Song Liu
	<song@...nel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-security-module@...r.kernel.org"
	<linux-security-module@...r.kernel.org>,
        Kernel Team <kernel-team@...a.com>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "eddyz87@...il.com"
	<eddyz87@...il.com>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "martin.lau@...ux.dev"
	<martin.lau@...ux.dev>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "brauner@...nel.org" <brauner@...nel.org>,
        "jack@...e.cz" <jack@...e.cz>,
        "kpsingh@...nel.org" <kpsingh@...nel.org>,
        "mattbobrowski@...gle.com"
	<mattbobrowski@...gle.com>,
        "amir73il@...il.com" <amir73il@...il.com>,
        "repnop@...gle.com" <repnop@...gle.com>,
        "jlayton@...nel.org"
	<jlayton@...nel.org>,
        Josef Bacik <josef@...icpanda.com>,
        "mic@...ikod.net"
	<mic@...ikod.net>,
        "gnoack@...gle.com" <gnoack@...gle.com>
Subject: Re: [PATCH bpf-next 0/4] Make inode storage available to tracing prog

Hi James, 

Thanks for your input!

> On Nov 14, 2024, at 1:49 PM, James Bottomley <James.Bottomley@...senPartnership.com> wrote:

[...]

>> 
>> 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?
> 
> Got to say I'm with Casey here, this will generate horrible and failure
> prone code.

Yes, as I described in another email in the thread [1], this turned
out to cause more troubles than I thought. 

> Since effectively you're making i_security always present anyway,
> simply do that and also pull the allocation code out of security.c in a
> way that it's always available?  

I think this is a very good idea. If folks agree with this approach, 
I am more than happy to draft patch for this. 

Thanks again, 

Song

> That way you don't have to special
> case the code depending on whether CONFIG_SECURITY is defined. 
> Effectively this would give everyone a generic way to attach some
> memory area to an inode.  I know it's more complex than this because
> there are LSM hooks that run from security_inode_alloc() but if you can
> make it work generically, I'm sure everyone will benefit.

[1] https://lore.kernel.org/linux-fsdevel/86C65B85-8167-4D04-BFF5-40FD4F3407A4@fb.com/