On Thu, Mar 28, 2024 at 03:45:12PM +0100, Mickaël Salaün wrote: > The subject should start with "landlock: Use" instead of "LANDLOCK: use" > > On Thu, Mar 28, 2024 at 01:23:17PM +0530, Ayush Tiwari wrote: > > Hello Paul > > Thanks a lot for the feedback. Apologies for the mistakes. Could you > > help me in some places so that I can correct the errors, like: > > On Wed, Mar 27, 2024 at 07:43:36PM -0400, Paul Moore wrote: > > > On Wed, Mar 27, 2024 at 7:26 PM Ayush Tiwari <ayushtiw0110@xxxxxxxxx> wrote: > > > > > > > > Use kmem_cache replace kzalloc() calls with kmem_cache_zalloc() for > > > > struct landlock_object and update the related dependencies. > > > > > > > > Signed-off-by: Ayush Tiwari <ayushtiw0110@xxxxxxxxx> > > > > --- > > > > security/landlock/fs.c | 2 +- > > > > security/landlock/object.c | 14 ++++++++++++-- > > > > security/landlock/object.h | 4 ++++ > > > > security/landlock/setup.c | 2 ++ > > > > 4 files changed, 19 insertions(+), 3 deletions(-) > > > > > > Hi Ayush, > > > > > > Mickaël has the final say on Landlock patches, but I had a few > > > comments that I've included below ... > > > > > > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c > > > > index fc520a06f9af..227dd67dd902 100644 > > > > --- a/security/landlock/fs.c > > > > +++ b/security/landlock/fs.c > > > > @@ -124,7 +124,7 @@ static struct landlock_object *get_inode_object(struct inode *const inode) > > > > if (unlikely(rcu_access_pointer(inode_sec->object))) { > > > > /* Someone else just created the object, bail out and retry. */ > > > > spin_unlock(&inode->i_lock); > > > > - kfree(new_object); > > > > + kmem_cache_free(landlock_object_cache, new_object); > > > > > > See my comment below, but you may want to wrap this in a Landlock > > > object API function. > > Sure. I will definitely implement this. > > > > > > > rcu_read_lock(); > > > > goto retry; > > > > diff --git a/security/landlock/object.c b/security/landlock/object.c > > > > index 1f50612f0185..df1354215617 100644 > > > > --- a/security/landlock/object.c > > > > +++ b/security/landlock/object.c > > > > @@ -17,6 +17,15 @@ > > > > > > > > #include "object.h" > > > > > > > > +struct kmem_cache *landlock_object_cache; > > > > + > > > > +void __init landlock_object_init(void) > > > > +{ > > > > + landlock_object_cache = kmem_cache_create( > > > > + "landlock_object_cache", sizeof(struct landlock_object), 0, > > No need for the "_cache" name suffix. > > > > > + SLAB_PANIC, NULL); > > > > > > The comments in include/linux/slab.h suggest using the KMEM_CACHE() > > > macro, instead of kmem_cache_create(), as a best practice for creating > > > slab caches. > > > Hello mentors I was trying to work on the above suggestion and I am facing some problem regarding replacing kzalloc with kmem_cache_zalloc calls when using KMEM macro from include/linux/slab.h because for kmem_cache_zalloc I will be needing a cache pointer, but KMEM macro doesn't return any such pointer. So is there any way to do this using macro or do i have to avoid using that macro for this case and use all methods regarding kmem as defined in the linux memory management API doc? > Agree with Paul and Greg unless commented otherwise. Thanks