On Thu, 2022-12-01 at 11:41 +0100, Roberto Sassu wrote:

From: Roberto Sassu <roberto.sassu@huawei.com>

Currently, security_inode_init_security() supports only one LSM providing
an xattr and EVM calculating the HMAC on that xattr, plus other inode
metadata.

Allow all LSMs to provide one or multiple xattrs, by extending the security
blob reservation mechanism. Introduce the new lbs_xattr field of the
lsm_blob_sizes structure, so that each LSM can specify how many xattrs it
needs, and the LSM infrastructure knows how many xattr slots it should
allocate.

Dynamically allocate the xattrs array to be populated by LSMs with the
inode_init_security hook, and pass it to the latter instead of the
name/value/len triple. Update the documentation accordingly, and fix the
description of the xattr name, as it is not allocated anymore.

Since the LSM infrastructure, at initialization time, updates the number of
the requested xattrs provided by each LSM with a corresponding offset in
the security blob (in this case the xattr array), it makes straightforward
for an LSM to access the right position in the xattr array.

There is still the issue that an LSM might not fill the xattr, even if it
requests it (legitimate case, for example it might have been loaded but not
initialized with a policy). Since users of the xattr array (e.g. the
initxattrs() callbacks) detect the end of the xattr array by checking if
the xattr name is NULL, not filling an xattr would cause those users to
stop scanning xattrs prematurely.

Solve that issue by introducing security_check_compact_filled_xattrs(),
which does a basic check of the xattr array (if the xattr name is filled,
the xattr value should be too, and viceversa), and compacts the xattr array
by removing the holes.

An alternative solution would be to let users of the xattr array know the
number of elements of that array, so that they don't have to check the
termination. However, this seems more invasive, compared to a simple move
of few array elements.

security_check_compact_filled_xattrs() also determines how many xattrs in
the xattr array have been filled. If there is none, skip
evm_inode_init_security() and initxattrs(). Skipping the former also avoids
EVM to crash the kernel, as it is expecting a filled xattr.

Finally, adapt both SELinux and Smack to use the new definition of the
inode_init_security hook, and to correctly fill the designated slots in the
xattr array. For Smack, reserve space for the other defined xattrs although
they are not set yet in smack_inode_init_security().

Reported-by: Nicolas Bouchinet <redacted> (EVM crash)
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>