On 5/27/2020 5:38 AM, KP Singh wrote: > On 26-May 22:08, Christoph Hellwig wrote: >> On Tue, May 26, 2020 at 06:33:34PM +0200, KP Singh wrote: >>> From: KP Singh <kpsingh@xxxxxxxxxx> >>> >>> Similar to bpf_local_storage for sockets, add local storage for inodes. >>> The life-cycle of storage is managed with the life-cycle of the inode. >>> i.e. the storage is destroyed along with the owning inode. >>> >>> Since, the intention is to use this in LSM programs, the destruction is >>> done after security_inode_free in __destroy_inode. >> NAK onbloating the inode structure. Please find an out of line way >> to store your information. > The other alternative is to use lbs_inode (security blobs) and we can > do this without adding fields to struct inode. This is the correct approach, and always has been. This isn't the first ( or second :( ) case where the correct behavior for an LSM has been pretty darn obvious, but you've taken a different approach for no apparent reason. > Here is a rough diff (only illustrative, won't apply cleanly) of the > changes needed to this patch: > > https://gist.github.com/sinkap/1d213d17fb82a5e8ffdc3f320ec37d79 To do just a little nit-picking, please use bpf_inode() instead of bpf_inode_storage(). This is in keeping with the convention used by the other security modules. Sticking with the existing convention makes it easier for people (and tools) that work with multiple security modules. > Once tracing has gets a whitelist based access to inode storage, I > guess it, too, can use bpf_local_storage for inodes Only within the BPF module. Your sentence above is slightly garbled, so I'm not really sure what you're saying, but if you're suggesting that tracing code outside of the BPF security module can use the BPF inode data, the answer is a resounding "no". > if CONFIG_BPF_LSM > is enabled. Does this sound reasonable to the BPF folks? > > - KP > >
