On Thu, 2022-12-01 at 11:41 +0100, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > 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 <nicolas.bouchinet@xxxxxxxxxxx> (EVM crash) > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > Reviewed-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx> Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
