lkml.org 
[lkml]   [2022]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v4 2/5] security: Rewrite security_old_inode_init_security()
    From
    Date
    On Mon, 2022-11-21 at 18:55 -0500, Paul Moore wrote:
    > On Mon, Nov 21, 2022 at 3:54 PM Mimi Zohar <zohar@linux.ibm.com> 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

    \
     
     \ /
      Last update: 2022-11-22 09:33    [W:3.548 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site