Messages in this thread | | | Date | Fri, 18 Nov 2022 10:04:37 +0100 | Subject | Re: [PATCH v4 2/5] security: Rewrite security_old_inode_init_security() | From | Roberto Sassu <> |
| |
On 11/17/2022 2:03 PM, Mimi Zohar wrote: > Hi Roberto, > > On Thu, 2022-11-10 at 10:46 +0100, Roberto Sassu wrote: >> From: Roberto Sassu <roberto.sassu@huawei.com> >> >> Rewrite security_old_inode_init_security() to call >> security_inode_init_security() before making changes to support multiple >> LSMs providing xattrs. Do it so that the required changes are done only in >> one place. > > Only security_inode_init_security() has support for EVM. Making > security_old_inode_init_security() a wrapper for > security_inode_init_security() could result in security.evm extended > attributes being created that previously weren't created.
Hi Mimi
yes, I thought about this problem. In fact, it should not matter too much. Since security_old_inode_init_security() supports setting only one xattr: if there is an LSM xattr, that one will be set, and the EVM one will be discarded; if there is no LSM xattr, EVM would not add one.
> In fact ocfs2 defines ocfs2_init_security_get() as a wrapper for both > the old and new inode_init_security calls based on the caller's > preference. Only mknod and symlink seem to use the old function. > Wondering why do they differentiate between callers? (Cc'ing the ocfs2 > mailing list as they're affected by this change.) > > "[PATCH v4 1/5] reiserfs: Add missing calls to > reiserfs_security_free()" fixed a memory leak. I couldn't tell if > there was a similar memory leak in ocfs2, the only other user of > security_old_inode_init_security().
Will look into it.
> As ocfs2 already defines initxattrs, that leaves only reiserfs missing > initxattrs(). A better, cleaner solution would be to define one.
Yes, great idea!
Thanks
Roberto
> thanks, > > Mimi > >> >> Define the security_initxattrs() callback and pass it to >> security_inode_init_security() as argument, to obtain the first xattr >> provided by LSMs. >> >> This behavior is a bit different from the current one. Before this patch >> calling call_int_hook() could cause multiple LSMs to provide an xattr, >> since call_int_hook() does not stop when an LSM returns zero. The caller of >> security_old_inode_init_security() receives the last xattr set. The pointer >> of the xattr value of previous LSMs is lost, causing memory leaks. >> >> However, in practice, this scenario does not happen as the only in-tree >> LSMs providing an xattr at inode creation time are SELinux and Smack, which >> are mutually exclusive. >> >> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>b
| |