On Mon, 2022-11-21 at 18:55 -0500, Paul Moore wrote: > On Mon, Nov 21, 2022 at 3:54 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote: > > On Mon, 2022-11-21 at 10:45 +0100, Roberto Sassu wrote: > > > > As ocfs2 already defines initxattrs, that leaves only reiserfs missing > > > > initxattrs(). A better, cleaner solution would be to define one. > > > > > > If I understood why security_old_inode_init_security() is called > > > instead of security_inode_init_security(), the reason seems that the > > > filesystem code uses the length of the obtained xattr to make some > > > calculations (e.g. reserve space). The xattr is written at a later > > > time. > > > > > > Since for reiserfs there is a plan to deprecate it, it probably > > > wouldn't be worth to support the creation of multiple xattrs. I would > > > define a callback to take the first xattr and make a copy, so that > > > calling security_inode_init_security() + reiserfs_initxattrs() is > > > equivalent to calling security_old_inode_init_security(). > > FWIW, reiserfs isn't going to be removed until 2025, I'm hopeful we > can remove the IMA/EVM special cases before then :) Well, we are not that far... > > > But then, this is what anyway I was doing with the > > > security_initxattrs() callback, for all callers of security_old_inode_i > > > nit_security(). > > > > > > Also, security_old_inode_init_security() is exported to kernel modules. > > > Maybe, it is used somewhere. So, unless we plan to remove it > > > completely, it should be probably be fixed to avoid multiple LSMs > > > successfully setting an xattr, and losing the memory of all except the > > > last (which this patch fixes by calling security_inode_init_security()). > > I would much rather remove security_old_inode_init_security() then > worry about what out-of-tree modules might be using it. Hopefully we > can resolve the ocfs2 usage and get ocfs2 exclusively on the new hook > without too much trouble, which means all we have left is reiserfs ... > how difficult would you expect the conversion to be for reiserfs? Ok for removing security_old_inode_init_security(). For reiserfs, I guess maintaining the current behavior of setting only one xattr should be easy. For multiple xattrs, I need to understand exactly how many blocks need to be reserved. > > > If there is still the preference, I will implement the reiserfs > > > callback and make a fix for security_old_inode_init_security(). > > > > There's no sense in doing both, as the purpose of defining a reiserfs > > initxattrs function was to clean up this code making it more readable. The fix for security_old_inode_init_security(), stopping at the first LSM returning zero, was to avoid the memory leak. Will not be needed if the function is removed. Roberto
