On Wed, May 27, 2020 at 6:41 PM Casey Schaufler [off-list ref] wrote:

On 5/27/2020 5:38 AM, KP Singh wrote:

quoted

On 26-May 22:08, Christoph Hellwig wrote:

quoted

On Tue, May 26, 2020 at 06:33:34PM +0200, KP Singh wrote:

quoted

From: KP Singh <redacted>

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.

quoted

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.

quoted

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".

This is why I wanted to add a separate pointer in struct inode so that
we could share the implementation with tracing. bpf_local_storage
is managed (per-program+per-type of storage) with separate BPF maps.
So, it can be easily shared between two programs (and
program types) without them clobbering over each other.

I guess we can have separate pointers for tracing,
use the pointer in the security blob for the LSM and discuss this separately
if and when we use this for tracing and keep this series patches scoped to
BPF_PROG_TYPE_LSM.

- KP

quoted

 if CONFIG_BPF_LSM
is enabled. Does this sound reasonable to the BPF folks?

- KP