Hi Casey, 

Thanks for your input. 

On Nov 12, 2024, at 10:09 AM, Casey Schaufler [off-list ref] wrote:

On 11/12/2024 12:25 AM, Song Liu wrote:

quoted

bpf inode local storage can be useful beyond LSM programs. For example,
bcc/libbpf-tools file* can use inode local storage to simplify the logic.
This set makes inode local storage available to tracing program.

Mixing the storage and scope of LSM data and tracing data leaves all sorts
of opportunities for abuse. Add inode data for tracing if you can get the
patch accepted, but do not move the LSM data out of i_security. Moving
the LSM data would break the integrity (such that there is) of the LSM
model.

I honestly don't see how this would cause any issues. Each bpf inode 
storage maps are independent of each other, and the bpf local storage is 
designed to handle multiple inode storage maps properly. Therefore, if
the user decide to stick with only LSM hooks, there isn't any behavior 
change. OTOH, if the user decides some tracing hooks (on tracepoints, 
etc.) are needed, making a inode storage map available for both tracing 
programs and LSM programs would help simplify the logic. (Alternatively,
the tracing programs need to store per inode data in a hash map, and 
the LSM program would read that instead of the inode storage map.)

Does this answer the question and address the concerns?

Thanks,
Song

quoted

1/4 is missing change for bpf task local storage. 2/4 move inode local
storage from security blob to inode.

Similar to task local storage in tracing program, it is necessary to add
recursion prevention logic for inode local storage. Patch 3/4 adds such
logic, and 4/4 add a test for the recursion prevention logic.

Song Liu (4):
 bpf: lsm: Remove hook to bpf_task_storage_free
 bpf: Make bpf inode storage available to tracing program
 bpf: Add recursion prevention logic for inode storage
 selftest/bpf: Test inode local storage recursion prevention

[...]