[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5758e5c4-9c8b-4492-3ecd-ba6607fc2899@huaweicloud.com>
Date: Fri, 18 Nov 2022 10:04:37 +0100
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: Mimi Zohar <zohar@...ux.ibm.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>,
ocfs2-devel@....oracle.com
Subject: Re: [PATCH v4 2/5] security: Rewrite
security_old_inode_init_security()
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@...wei.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@...wei.com>b
Powered by blists - more mailing lists