| Message ID | 20241112082600.298035-3-song@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Make inode storage available to tracing prog | expand |
On Tue, Nov 12, 2024 at 12:25:56AM -0800, Song Liu wrote: > inode storage can be useful for non-LSM program. For example, file* tools > from bcc/libbpf-tools can use inode storage instead of hash map; fanotify > fastpath [1] can also use inode storage to store useful data. > > Make inode storage available for tracing program. Move bpf inode storage > from a security blob to inode->i_bpf_storage, and adjust related code > accordingly. > > [1] https://lore.kernel.org/linux-fsdevel/20241029231244.2834368-1-song@kernel.org/ > Signed-off-by: Song Liu <song@kernel.org> > --- > fs/inode.c | 1 + > include/linux/bpf.h | 9 +++++++++ > include/linux/bpf_lsm.h | 29 ----------------------------- > include/linux/fs.h | 4 ++++ > kernel/bpf/Makefile | 3 +-- > kernel/bpf/bpf_inode_storage.c | 32 +++++--------------------------- > kernel/bpf/bpf_lsm.c | 4 ---- > kernel/trace/bpf_trace.c | 4 ++++ > security/bpf/hooks.c | 6 ------ > 9 files changed, 24 insertions(+), 68 deletions(-) > > diff --git a/fs/inode.c b/fs/inode.c > index 8dabb224f941..3c679578169f 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -286,6 +286,7 @@ static struct inode *alloc_inode(struct super_block *sb) > void __destroy_inode(struct inode *inode) > { > BUG_ON(inode_has_buffers(inode)); > + bpf_inode_storage_free(inode); > inode_detach_wb(inode); > security_inode_free(inode); > fsnotify_inode_delete(inode); > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 1b84613b10ac..0b31d2e74df6 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -2672,6 +2672,7 @@ struct bpf_link *bpf_link_by_id(u32 id); > const struct bpf_func_proto *bpf_base_func_proto(enum bpf_func_id func_id, > const struct bpf_prog *prog); > void bpf_task_storage_free(struct task_struct *task); > +void bpf_inode_storage_free(struct inode *inode); > void bpf_cgrp_storage_free(struct cgroup *cgroup); > bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog); > const struct btf_func_model * > @@ -2942,6 +2943,10 @@ static inline void bpf_task_storage_free(struct task_struct *task) > { > } > > +static inline void bpf_inode_storage_free(struct inode *inode) > +{ > +} > + > static inline bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog) > { > return false; > @@ -3305,6 +3310,10 @@ extern const struct bpf_func_proto bpf_task_storage_get_recur_proto; > extern const struct bpf_func_proto bpf_task_storage_get_proto; > extern const struct bpf_func_proto bpf_task_storage_delete_recur_proto; > extern const struct bpf_func_proto bpf_task_storage_delete_proto; > +extern const struct bpf_func_proto bpf_inode_storage_get_proto; > +extern const struct bpf_func_proto bpf_inode_storage_get_recur_proto; > +extern const struct bpf_func_proto bpf_inode_storage_delete_proto; > +extern const struct bpf_func_proto bpf_inode_storage_delete_recur_proto; > extern const struct bpf_func_proto bpf_for_each_map_elem_proto; > extern const struct bpf_func_proto bpf_btf_find_by_name_kind_proto; > extern const struct bpf_func_proto bpf_sk_setsockopt_proto; > diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h > index aefcd6564251..a819c2f0a062 100644 > --- a/include/linux/bpf_lsm.h > +++ b/include/linux/bpf_lsm.h > @@ -19,31 +19,12 @@ > #include <linux/lsm_hook_defs.h> > #undef LSM_HOOK > > -struct bpf_storage_blob { > - struct bpf_local_storage __rcu *storage; > -}; > - > -extern struct lsm_blob_sizes bpf_lsm_blob_sizes; > - > int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, > const struct bpf_prog *prog); > > bool bpf_lsm_is_sleepable_hook(u32 btf_id); > bool bpf_lsm_is_trusted(const struct bpf_prog *prog); > > -static inline struct bpf_storage_blob *bpf_inode( > - const struct inode *inode) > -{ > - if (unlikely(!inode->i_security)) > - return NULL; > - > - return inode->i_security + bpf_lsm_blob_sizes.lbs_inode; > -} > - > -extern const struct bpf_func_proto bpf_inode_storage_get_proto; > -extern const struct bpf_func_proto bpf_inode_storage_delete_proto; > -void bpf_inode_storage_free(struct inode *inode); > - > void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func); > > int bpf_lsm_get_retval_range(const struct bpf_prog *prog, > @@ -66,16 +47,6 @@ static inline int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, > return -EOPNOTSUPP; > } > > -static inline struct bpf_storage_blob *bpf_inode( > - const struct inode *inode) > -{ > - return NULL; > -} > - > -static inline void bpf_inode_storage_free(struct inode *inode) > -{ > -} > - > static inline void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, > bpf_func_t *bpf_func) > { > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 3559446279c1..479097e4dd5b 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -79,6 +79,7 @@ struct fs_context; > struct fs_parameter_spec; > struct fileattr; > struct iomap_ops; > +struct bpf_local_storage; > > extern void __init inode_init(void); > extern void __init inode_init_early(void); > @@ -648,6 +649,9 @@ struct inode { > #ifdef CONFIG_SECURITY > void *i_security; > #endif > +#ifdef CONFIG_BPF_SYSCALL > + struct bpf_local_storage __rcu *i_bpf_storage; > +#endif Sorry, we're not growing struct inode for this. It just keeps getting bigger. Last cycle we freed up 8 bytes to shrink it and we're not going to waste them on special-purpose stuff. We already NAKed someone else's pet field here.
Hi Christian, Thanks for your review. > On Nov 13, 2024, at 2:19 AM, Christian Brauner <brauner@kernel.org> wrote: [...] >> diff --git a/include/linux/fs.h b/include/linux/fs.h >> index 3559446279c1..479097e4dd5b 100644 >> --- a/include/linux/fs.h >> +++ b/include/linux/fs.h >> @@ -79,6 +79,7 @@ struct fs_context; >> struct fs_parameter_spec; >> struct fileattr; >> struct iomap_ops; >> +struct bpf_local_storage; >> >> extern void __init inode_init(void); >> extern void __init inode_init_early(void); >> @@ -648,6 +649,9 @@ struct inode { >> #ifdef CONFIG_SECURITY >> void *i_security; >> #endif >> +#ifdef CONFIG_BPF_SYSCALL >> + struct bpf_local_storage __rcu *i_bpf_storage; >> +#endif > > Sorry, we're not growing struct inode for this. It just keeps getting > bigger. Last cycle we freed up 8 bytes to shrink it and we're not going > to waste them on special-purpose stuff. We already NAKed someone else's > pet field here. Would it be acceptable if we union i_bpf_storage with i_security? IOW, if CONFIG_SECURITY is enabled, we will use existing logic. If CONFIG_SECURITY is not enabled, we will use i_bpf_storage. Given majority of default configs have CONFIG_SECURITY=y, this will not grow inode for most users. OTOH, users with CONFIG_SECURITY=n && CONFIG_BPF_SYSCALL=y combination can still use inode local storage in the tracing BPF programs. Does this make sense? Thanks, Song
On 11/13/2024 6:15 AM, Song Liu wrote: > Hi Christian, > > Thanks for your review. > >> On Nov 13, 2024, at 2:19 AM, Christian Brauner <brauner@kernel.org> wrote: > [...] > >>> diff --git a/include/linux/fs.h b/include/linux/fs.h >>> index 3559446279c1..479097e4dd5b 100644 >>> --- a/include/linux/fs.h >>> +++ b/include/linux/fs.h >>> @@ -79,6 +79,7 @@ struct fs_context; >>> struct fs_parameter_spec; >>> struct fileattr; >>> struct iomap_ops; >>> +struct bpf_local_storage; >>> >>> extern void __init inode_init(void); >>> extern void __init inode_init_early(void); >>> @@ -648,6 +649,9 @@ struct inode { >>> #ifdef CONFIG_SECURITY >>> void *i_security; >>> #endif >>> +#ifdef CONFIG_BPF_SYSCALL >>> + struct bpf_local_storage __rcu *i_bpf_storage; >>> +#endif >> Sorry, we're not growing struct inode for this. It just keeps getting >> bigger. Last cycle we freed up 8 bytes to shrink it and we're not going >> to waste them on special-purpose stuff. We already NAKed someone else's >> pet field here. > Would it be acceptable if we union i_bpf_storage with i_security? No! > IOW, if CONFIG_SECURITY is enabled, we will use existing logic. > If CONFIG_SECURITY is not enabled, we will use i_bpf_storage. > Given majority of default configs have CONFIG_SECURITY=y, this > will not grow inode for most users. OTOH, users with > CONFIG_SECURITY=n && CONFIG_BPF_SYSCALL=y combination can still > use inode local storage in the tracing BPF programs. > > Does this make sense? All it would take is one BPF programmer assuming that CONFIG_SECURITY=n is the norm for this to blow up spectacularly. > > Thanks, > Song >
On Wed, Nov 13, 2024 at 10:30 AM Casey Schaufler <casey@schaufler-ca.com> wrote: > > On 11/13/2024 6:15 AM, Song Liu wrote: > > Hi Christian, > > > > Thanks for your review. > > > >> On Nov 13, 2024, at 2:19 AM, Christian Brauner <brauner@kernel.org> wrote: > > [...] > > > >>> diff --git a/include/linux/fs.h b/include/linux/fs.h > >>> index 3559446279c1..479097e4dd5b 100644 > >>> --- a/include/linux/fs.h > >>> +++ b/include/linux/fs.h > >>> @@ -79,6 +79,7 @@ struct fs_context; > >>> struct fs_parameter_spec; > >>> struct fileattr; > >>> struct iomap_ops; > >>> +struct bpf_local_storage; > >>> > >>> extern void __init inode_init(void); > >>> extern void __init inode_init_early(void); > >>> @@ -648,6 +649,9 @@ struct inode { > >>> #ifdef CONFIG_SECURITY > >>> void *i_security; > >>> #endif > >>> +#ifdef CONFIG_BPF_SYSCALL > >>> + struct bpf_local_storage __rcu *i_bpf_storage; > >>> +#endif > >> Sorry, we're not growing struct inode for this. It just keeps getting > >> bigger. Last cycle we freed up 8 bytes to shrink it and we're not going > >> to waste them on special-purpose stuff. We already NAKed someone else's > >> pet field here. > > Would it be acceptable if we union i_bpf_storage with i_security? > > No! > > > IOW, if CONFIG_SECURITY is enabled, we will use existing logic. > > If CONFIG_SECURITY is not enabled, we will use i_bpf_storage. > > Given majority of default configs have CONFIG_SECURITY=y, this > > will not grow inode for most users. OTOH, users with > > CONFIG_SECURITY=n && CONFIG_BPF_SYSCALL=y combination can still > > use inode local storage in the tracing BPF programs. > > > > Does this make sense? > > All it would take is one BPF programmer assuming that CONFIG_SECURITY=n > is the norm for this to blow up spectacularly. I seriously don't understand what would blow up and how. Can you be more specific? Thanks, Song
Hi Christian, > On Nov 13, 2024, at 2:19 AM, Christian Brauner <brauner@kernel.org> wrote: [...] >> static inline void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, >> bpf_func_t *bpf_func) >> { >> diff --git a/include/linux/fs.h b/include/linux/fs.h >> index 3559446279c1..479097e4dd5b 100644 >> --- a/include/linux/fs.h >> +++ b/include/linux/fs.h >> @@ -79,6 +79,7 @@ struct fs_context; >> struct fs_parameter_spec; >> struct fileattr; >> struct iomap_ops; >> +struct bpf_local_storage; >> >> extern void __init inode_init(void); >> extern void __init inode_init_early(void); >> @@ -648,6 +649,9 @@ struct inode { >> #ifdef CONFIG_SECURITY >> void *i_security; >> #endif >> +#ifdef CONFIG_BPF_SYSCALL >> + struct bpf_local_storage __rcu *i_bpf_storage; >> +#endif > > Sorry, we're not growing struct inode for this. It just keeps getting > bigger. Last cycle we freed up 8 bytes to shrink it and we're not going > to waste them on special-purpose stuff. We already NAKed someone else's > pet field here. Per other discussions in this thread, I am implementing the following: #ifdef CONFIG_SECURITY void *i_security; #elif CONFIG_BPF_SYSCALL struct bpf_local_storage __rcu *i_bpf_storage; #endif However, it is a bit trickier than I thought. Specifically, we need to deal with the following scenarios: 1. CONFIG_SECURITY=y && CONFIG_BPF_LSM=n && CONFIG_BPF_SYSCALL=y 2. CONFIG_SECURITY=y && CONFIG_BPF_LSM=y && CONFIG_BPF_SYSCALL=y but bpf lsm is not enabled at boot time. AFAICT, we need to modify how lsm blob are managed with CONFIG_BPF_SYSCALL=y && CONFIG_BPF_LSM=n case. The solution, even if it gets accepted, doesn't really save any memory. Instead of growing struct inode by 8 bytes, the solution will allocate 8 more bytes to inode->i_security. So the total memory consumption is the same, but the memory is more fragmented. Therefore, I think we should really step back and consider adding the i_bpf_storage to struct inode. While this does increase the size of struct inode by 8 bytes, it may end up with less overall memory consumption for the system. This is why. When the user cannot use inode local storage, the alternative is to use hash maps (use inode pointer as key). AFAICT, all hash maps comes with non-trivial overhead, in memory consumption, in access latency, and in extra code to manage the memory. OTOH, inode local storage doesn't have these issue, and is usually much more efficient: - memory is only allocated for inodes with actual data, - O(1) latency, - per inode data is freed automatically when the inode is evicted. Please refer to [1] where Amir mentioned all the work needed to properly manage a hash map, and I explained why we don't need to worry about these with inode local storage. Besides reducing memory consumption, i_bpf_storage also shortens the pointer chain to access inode local storage. Before this set, inode local storage is available at inode->i_security+offset(struct bpf_storage_blob)->storage. After this set, inode local storage is simply at inode->i_bpf_storage. At the moment, we are using bpf local storage with task_struct->bpf_storage, struct sock->sk_bpf_storage, struct cgroup->bpf_cgrp_storage. All of these turned out to be successful and helped users to use memory more efficiently. I think we can see the same benefits with struct inode->i_bpf_storage. I hope these make sense, and you will consider adding i_bpf_storage. Please let me know if anything above is not clear. Thanks, Song [1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxjXjjkKMa1xcPyxE5vxh1U5oGZJWtofRCwp-3ikCA6cgg@mail.gmail.com/
diff --git a/fs/inode.c b/fs/inode.c index 8dabb224f941..3c679578169f 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -286,6 +286,7 @@ static struct inode *alloc_inode(struct super_block *sb) void __destroy_inode(struct inode *inode) { BUG_ON(inode_has_buffers(inode)); + bpf_inode_storage_free(inode); inode_detach_wb(inode); security_inode_free(inode); fsnotify_inode_delete(inode); diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 1b84613b10ac..0b31d2e74df6 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2672,6 +2672,7 @@ struct bpf_link *bpf_link_by_id(u32 id); const struct bpf_func_proto *bpf_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog); void bpf_task_storage_free(struct task_struct *task); +void bpf_inode_storage_free(struct inode *inode); void bpf_cgrp_storage_free(struct cgroup *cgroup); bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog); const struct btf_func_model * @@ -2942,6 +2943,10 @@ static inline void bpf_task_storage_free(struct task_struct *task) { } +static inline void bpf_inode_storage_free(struct inode *inode) +{ +} + static inline bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog) { return false; @@ -3305,6 +3310,10 @@ extern const struct bpf_func_proto bpf_task_storage_get_recur_proto; extern const struct bpf_func_proto bpf_task_storage_get_proto; extern const struct bpf_func_proto bpf_task_storage_delete_recur_proto; extern const struct bpf_func_proto bpf_task_storage_delete_proto; +extern const struct bpf_func_proto bpf_inode_storage_get_proto; +extern const struct bpf_func_proto bpf_inode_storage_get_recur_proto; +extern const struct bpf_func_proto bpf_inode_storage_delete_proto; +extern const struct bpf_func_proto bpf_inode_storage_delete_recur_proto; extern const struct bpf_func_proto bpf_for_each_map_elem_proto; extern const struct bpf_func_proto bpf_btf_find_by_name_kind_proto; extern const struct bpf_func_proto bpf_sk_setsockopt_proto; diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h index aefcd6564251..a819c2f0a062 100644 --- a/include/linux/bpf_lsm.h +++ b/include/linux/bpf_lsm.h @@ -19,31 +19,12 @@ #include <linux/lsm_hook_defs.h> #undef LSM_HOOK -struct bpf_storage_blob { - struct bpf_local_storage __rcu *storage; -}; - -extern struct lsm_blob_sizes bpf_lsm_blob_sizes; - int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, const struct bpf_prog *prog); bool bpf_lsm_is_sleepable_hook(u32 btf_id); bool bpf_lsm_is_trusted(const struct bpf_prog *prog); -static inline struct bpf_storage_blob *bpf_inode( - const struct inode *inode) -{ - if (unlikely(!inode->i_security)) - return NULL; - - return inode->i_security + bpf_lsm_blob_sizes.lbs_inode; -} - -extern const struct bpf_func_proto bpf_inode_storage_get_proto; -extern const struct bpf_func_proto bpf_inode_storage_delete_proto; -void bpf_inode_storage_free(struct inode *inode); - void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func); int bpf_lsm_get_retval_range(const struct bpf_prog *prog, @@ -66,16 +47,6 @@ static inline int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, return -EOPNOTSUPP; } -static inline struct bpf_storage_blob *bpf_inode( - const struct inode *inode) -{ - return NULL; -} - -static inline void bpf_inode_storage_free(struct inode *inode) -{ -} - static inline void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func) { diff --git a/include/linux/fs.h b/include/linux/fs.h index 3559446279c1..479097e4dd5b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -79,6 +79,7 @@ struct fs_context; struct fs_parameter_spec; struct fileattr; struct iomap_ops; +struct bpf_local_storage; extern void __init inode_init(void); extern void __init inode_init_early(void); @@ -648,6 +649,9 @@ struct inode { #ifdef CONFIG_SECURITY void *i_security; #endif +#ifdef CONFIG_BPF_SYSCALL + struct bpf_local_storage __rcu *i_bpf_storage; +#endif /* Stat data, not accessed from path walking */ unsigned long i_ino; diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile index 105328f0b9c0..73604c7130f1 100644 --- a/kernel/bpf/Makefile +++ b/kernel/bpf/Makefile @@ -10,8 +10,7 @@ obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o log.o obj-$(CONFIG_BPF_SYSCALL) += bpf_iter.o map_iter.o task_iter.o prog_iter.o link_iter.o obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o bloom_filter.o obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o ringbuf.o -obj-$(CONFIG_BPF_SYSCALL) += bpf_local_storage.o bpf_task_storage.o -obj-${CONFIG_BPF_LSM} += bpf_inode_storage.o +obj-$(CONFIG_BPF_SYSCALL) += bpf_local_storage.o bpf_task_storage.o bpf_inode_storage.o obj-$(CONFIG_BPF_SYSCALL) += disasm.o mprog.o obj-$(CONFIG_BPF_JIT) += trampoline.o obj-$(CONFIG_BPF_SYSCALL) += btf.o memalloc.o diff --git a/kernel/bpf/bpf_inode_storage.c b/kernel/bpf/bpf_inode_storage.c index 44ccebc745e5..8d5a9bfe6643 100644 --- a/kernel/bpf/bpf_inode_storage.c +++ b/kernel/bpf/bpf_inode_storage.c @@ -21,16 +21,11 @@ DEFINE_BPF_STORAGE_CACHE(inode_cache); -static struct bpf_local_storage __rcu ** -inode_storage_ptr(void *owner) +static struct bpf_local_storage __rcu **inode_storage_ptr(void *owner) { struct inode *inode = owner; - struct bpf_storage_blob *bsb; - bsb = bpf_inode(inode); - if (!bsb) - return NULL; - return &bsb->storage; + return &inode->i_bpf_storage; } static struct bpf_local_storage_data *inode_storage_lookup(struct inode *inode, @@ -39,14 +34,9 @@ static struct bpf_local_storage_data *inode_storage_lookup(struct inode *inode, { struct bpf_local_storage *inode_storage; struct bpf_local_storage_map *smap; - struct bpf_storage_blob *bsb; - - bsb = bpf_inode(inode); - if (!bsb) - return NULL; inode_storage = - rcu_dereference_check(bsb->storage, bpf_rcu_lock_held()); + rcu_dereference_check(inode->i_bpf_storage, bpf_rcu_lock_held()); if (!inode_storage) return NULL; @@ -57,15 +47,10 @@ static struct bpf_local_storage_data *inode_storage_lookup(struct inode *inode, void bpf_inode_storage_free(struct inode *inode) { struct bpf_local_storage *local_storage; - struct bpf_storage_blob *bsb; - - bsb = bpf_inode(inode); - if (!bsb) - return; rcu_read_lock(); - local_storage = rcu_dereference(bsb->storage); + local_storage = rcu_dereference(inode->i_bpf_storage); if (!local_storage) { rcu_read_unlock(); return; @@ -95,8 +80,6 @@ static long bpf_fd_inode_storage_update_elem(struct bpf_map *map, void *key, if (fd_empty(f)) return -EBADF; - if (!inode_storage_ptr(file_inode(fd_file(f)))) - return -EBADF; sdata = bpf_local_storage_update(file_inode(fd_file(f)), (struct bpf_local_storage_map *)map, @@ -136,12 +119,7 @@ BPF_CALL_5(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode, if (flags & ~(BPF_LOCAL_STORAGE_GET_F_CREATE)) return (unsigned long)NULL; - /* explicitly check that the inode_storage_ptr is not - * NULL as inode_storage_lookup returns NULL in this case and - * bpf_local_storage_update expects the owner to have a - * valid storage pointer. - */ - if (!inode || !inode_storage_ptr(inode)) + if (!inode) return (unsigned long)NULL; sdata = inode_storage_lookup(inode, map, true); diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c index 3bc61628ab25..6b6e0730e515 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -231,10 +231,6 @@ bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) } switch (func_id) { - case BPF_FUNC_inode_storage_get: - return &bpf_inode_storage_get_proto; - case BPF_FUNC_inode_storage_delete: - return &bpf_inode_storage_delete_proto; #ifdef CONFIG_NET case BPF_FUNC_sk_storage_get: return &bpf_sk_storage_get_proto; diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 949a3870946c..262bd101ea0b 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1553,6 +1553,10 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) if (bpf_prog_check_recur(prog)) return &bpf_task_storage_delete_recur_proto; return &bpf_task_storage_delete_proto; + case BPF_FUNC_inode_storage_get: + return &bpf_inode_storage_get_proto; + case BPF_FUNC_inode_storage_delete: + return &bpf_inode_storage_delete_proto; case BPF_FUNC_for_each_map_elem: return &bpf_for_each_map_elem_proto; case BPF_FUNC_snprintf: diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c index db759025abe1..67719a04bb0b 100644 --- a/security/bpf/hooks.c +++ b/security/bpf/hooks.c @@ -12,7 +12,6 @@ static struct security_hook_list bpf_lsm_hooks[] __ro_after_init = { LSM_HOOK_INIT(NAME, bpf_lsm_##NAME), #include <linux/lsm_hook_defs.h> #undef LSM_HOOK - LSM_HOOK_INIT(inode_free_security, bpf_inode_storage_free), }; static const struct lsm_id bpf_lsmid = { @@ -28,12 +27,7 @@ static int __init bpf_lsm_init(void) return 0; } -struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = { - .lbs_inode = sizeof(struct bpf_storage_blob), -}; - DEFINE_LSM(bpf) = { .name = "bpf", .init = bpf_lsm_init, - .blobs = &bpf_lsm_blob_sizes };
inode storage can be useful for non-LSM program. For example, file* tools from bcc/libbpf-tools can use inode storage instead of hash map; fanotify fastpath [1] can also use inode storage to store useful data. Make inode storage available for tracing program. Move bpf inode storage from a security blob to inode->i_bpf_storage, and adjust related code accordingly. [1] https://lore.kernel.org/linux-fsdevel/20241029231244.2834368-1-song@kernel.org/ Signed-off-by: Song Liu <song@kernel.org> --- fs/inode.c | 1 + include/linux/bpf.h | 9 +++++++++ include/linux/bpf_lsm.h | 29 ----------------------------- include/linux/fs.h | 4 ++++ kernel/bpf/Makefile | 3 +-- kernel/bpf/bpf_inode_storage.c | 32 +++++--------------------------- kernel/bpf/bpf_lsm.c | 4 ---- kernel/trace/bpf_trace.c | 4 ++++ security/bpf/hooks.c | 6 ------ 9 files changed, 24 insertions(+), 68 deletions(-)