linux-kernel - Re: [PATCH v4 3/5] security: Allow all LSMs to provide xattrs for inode_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3ffb9bb4ab203b5e0459c3892ded4ae0cd80458b.camel@linux.ibm.com>
Date:   Fri, 18 Nov 2022 10:10:14 -0500
From:   Mimi Zohar <zohar@...ux.ibm.com>
To:     Roberto Sassu <roberto.sassu@...weicloud.com>,
        dmitry.kasatkin@...il.com, paul@...l-moore.com, jmorris@...ei.org,
        serge@...lyn.com, stephen.smalley.work@...il.com,
        eparis@...isplace.org, casey@...aufler-ca.com
Cc:     linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org, selinux@...r.kernel.org,
        reiserfs-devel@...r.kernel.org, linux-kernel@...r.kernel.org,
        keescook@...omium.org, nicolas.bouchinet@...p-os.org,
        Roberto Sassu <roberto.sassu@...wei.com>
Subject: Re: [PATCH v4 3/5] security: Allow all LSMs to provide xattrs for
 inode_init_security hook

On Fri, 2022-11-18 at 10:14 +0100, Roberto Sassu wrote:
> >> +static int security_check_compact_xattrs(struct xattr *xattrs,
> >> +                                     int num_xattrs, int *checked_xattrs)
> > 
> > Perhaps the variable naming is off, making it difficult to read.   So
> > although this is a static function, which normally doesn't require a
> > comment, it's definitely needs one.
> 
> Ok, will improve it.
> 
> >> +{
> >> +    int i;
> >> +
> >> +    for (i = *checked_xattrs; i < num_xattrs; i++) {
> > 
> > If the number of "checked" xattrs was kept up to date, removing the
> > empty xattr gaps wouldn't require a loop.  Is the purpose of this loop
> > to support multiple per LSM xattrs?
> 
> An LSM might reserve one or more xattrs, but not set it/them (for 
> example because it is not initialized). In this case, removing the gaps 
> is needed for all subsequent LSMs.

Including this sort of info in the function description or as a comment
in the code would definitely simplify review.

security_check_compact_xattrs() is called in the loop after getting
each LSM's xattr(s).  Only the current LSMs xattrs need to be
compressed, yet the loop goes to the maximum number of xattrs each
time. Just wondering if there is a way of improving it.

-- 
thanks,

Mimi