From: Casey Schaufler <casey@schaufler-ca.com>
To: Roberto Sassu <roberto.sassu@huaweicloud.com>,
Paul Moore <paul@paul-moore.com>,
viro@zeniv.linux.org.uk, brauner@kernel.org,
chuck.lever@oracle.com, jlayton@kernel.org, neilb@suse.de,
kolga@netapp.com, Dai.Ngo@oracle.com, tom@talpey.com,
jmorris@namei.org, serge@hallyn.com, zohar@linux.ibm.com,
dmitry.kasatkin@gmail.com, dhowells@redhat.com,
jarkko@kernel.org, stephen.smalley.work@gmail.com,
eparis@parisplace.org, mic@digikod.net
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-nfs@vger.kernel.org, linux-security-module@vger.kernel.org,
linux-integrity@vger.kernel.org, keyrings@vger.kernel.org,
selinux@vger.kernel.org, Roberto Sassu <roberto.sassu@huawei.com>,
Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH v5 23/23] integrity: Switch from rbtree to LSM-managed blob for integrity_iint_cache
Date: Wed, 13 Dec 2023 10:08:48 -0800 [thread overview]
Message-ID: <9dc633d8-65a7-4b97-ab98-a21ada1d4ea5@schaufler-ca.com> (raw)
In-Reply-To: <7c226242-2eda-41cd-9be8-c2c010f3fc49@huaweicloud.com>
On 12/13/2023 2:45 AM, Roberto Sassu wrote:
> On 17.11.23 21:57, Paul Moore wrote:
>> On Nov 7, 2023 Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>>>
>>> ...
>>>
>>> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
>>> index 882fde2a2607..a5edd3c70784 100644
>>> --- a/security/integrity/iint.c
>>> +++ b/security/integrity/iint.c
>>> @@ -231,6 +175,10 @@ static int __init integrity_lsm_init(void)
>>> return 0;
>>> }
>>> +struct lsm_blob_sizes integrity_blob_sizes __ro_after_init = {
>>> + .lbs_inode = sizeof(struct integrity_iint_cache *),
>>> +};
>>
>> I'll admit that I'm likely missing an important detail, but is there
>> a reason why you couldn't stash the integrity_iint_cache struct
>> directly in the inode's security blob instead of the pointer? For
>> example:
>>
>> struct lsm_blob_sizes ... = {
>> .lbs_inode = sizeof(struct integrity_iint_cache),
>> };
>>
>> struct integrity_iint_cache *integrity_inode_get(inode)
>> {
>> if (unlikely(!inode->isecurity))
>> return NULL;
>
> Ok, this caught my attention...
>
> I see that selinux_inode() has it, but smack_inode() doesn't.
>
> Some Smack code assumes that the inode security blob is always non-NULL:
>
> static void init_inode_smack(struct inode *inode, struct smack_known
> *skp)
> {
> struct inode_smack *isp = smack_inode(inode);
>
> isp->smk_inode = skp;
> isp->smk_flags = 0;
> }
>
>
> Is that intended? Should I add the check?
Unless there's a case where inodes are created without calling
security_inode_alloc() there should never be an inode without a
security blob by the time you get to the Smack hook. That said,
people seem inclined to take all sorts of shortcuts and create
various "inodes" that aren't really inodes. I also see that SELinux
doesn't check the blob for cred or file structures. And that I
wrote the code in both cases.
Based on lack of bug reports for Smack on inodes and SELinux on
creds or files, It appears that the check is unnecessary. On the
other hand, it sure looks like good error detection hygiene. I
would be inclined to include the check in new code, but not get
in a panic about existing code.
>
> Thanks
>
> Roberto
>
>> return inode->i_security + integrity_blob_sizes.lbs_inode;
>> }
>>
>> --
>> paul-moore.com
>
>
next prev parent reply other threads:[~2023-12-13 18:08 UTC|newest]
Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-07 13:39 [PATCH v5 00/23] security: Move IMA and EVM to the LSM infrastructure Roberto Sassu
2023-11-07 13:39 ` [PATCH v5 01/23] ima: Align ima_inode_post_setattr() definition with " Roberto Sassu
2023-11-07 17:21 ` Casey Schaufler
2023-11-07 13:39 ` [PATCH v5 02/23] ima: Align ima_file_mprotect() " Roberto Sassu
2023-11-07 17:22 ` Casey Schaufler
2023-11-07 13:39 ` [PATCH v5 03/23] ima: Align ima_inode_setxattr() " Roberto Sassu
2023-11-07 17:23 ` Casey Schaufler
2023-11-07 13:39 ` [PATCH v5 04/23] ima: Align ima_inode_removexattr() " Roberto Sassu
2023-11-07 17:24 ` Casey Schaufler
2023-11-07 13:39 ` [PATCH v5 05/23] ima: Align ima_post_read_file() " Roberto Sassu
2023-11-07 17:25 ` Casey Schaufler
2023-11-07 13:39 ` [PATCH v5 06/23] evm: Align evm_inode_post_setattr() " Roberto Sassu
2023-11-07 17:26 ` Casey Schaufler
2023-11-07 13:39 ` [PATCH v5 07/23] evm: Align evm_inode_setxattr() " Roberto Sassu
2023-11-07 17:27 ` Casey Schaufler
2023-11-07 13:39 ` [PATCH v5 08/23] evm: Align evm_inode_post_setxattr() " Roberto Sassu
2023-11-07 17:28 ` Casey Schaufler
2023-11-07 13:39 ` [PATCH v5 09/23] security: Align inode_setattr hook definition with EVM Roberto Sassu
2023-11-07 13:39 ` [PATCH v5 10/23] security: Introduce inode_post_setattr hook Roberto Sassu
2023-11-07 17:30 ` Casey Schaufler
2023-11-16 4:33 ` Paul Moore
2023-11-16 9:43 ` Roberto Sassu
2023-11-16 18:46 ` Paul Moore
2023-11-07 13:40 ` [PATCH v5 11/23] security: Introduce inode_post_removexattr hook Roberto Sassu
2023-11-07 17:33 ` Casey Schaufler
2023-11-07 17:45 ` Roberto Sassu
2023-11-20 17:31 ` Roberto Sassu
2023-11-20 18:03 ` Casey Schaufler
2023-11-20 20:55 ` Paul Moore
2023-11-16 4:33 ` Paul Moore
2023-11-07 13:40 ` [PATCH v5 12/23] security: Introduce file_post_open hook Roberto Sassu
2023-11-07 17:35 ` Casey Schaufler
2023-11-07 13:40 ` [PATCH v5 13/23] security: Introduce file_pre_free_security hook Roberto Sassu
2023-11-07 17:39 ` Casey Schaufler
2023-11-16 4:33 ` Paul Moore
2023-11-16 9:46 ` Roberto Sassu
2023-11-16 18:41 ` Paul Moore
2023-11-07 13:40 ` [PATCH v5 14/23] security: Introduce path_post_mknod hook Roberto Sassu
2023-11-07 17:41 ` Casey Schaufler
2023-11-07 13:40 ` [PATCH v5 15/23] security: Introduce inode_post_create_tmpfile hook Roberto Sassu
2023-11-07 17:42 ` Casey Schaufler
2023-11-16 4:33 ` Paul Moore
2023-11-07 13:40 ` [PATCH v5 16/23] security: Introduce inode_post_set_acl hook Roberto Sassu
2023-11-07 17:44 ` Casey Schaufler
2023-11-16 4:33 ` Paul Moore
2023-11-07 13:40 ` [PATCH v5 17/23] security: Introduce inode_post_remove_acl hook Roberto Sassu
2023-11-07 17:45 ` Casey Schaufler
2023-11-16 4:33 ` Paul Moore
2023-11-07 13:40 ` [PATCH v5 18/23] security: Introduce key_post_create_or_update hook Roberto Sassu
2023-11-07 17:47 ` Casey Schaufler
2023-11-07 13:40 ` [PATCH v5 19/23] ima: Move to LSM infrastructure Roberto Sassu
2023-11-07 17:52 ` Casey Schaufler
2023-11-07 13:40 ` [PATCH v5 20/23] ima: Move IMA-Appraisal " Roberto Sassu
2023-11-07 18:43 ` Casey Schaufler
2023-11-07 13:40 ` [PATCH v5 21/23] evm: Move " Roberto Sassu
2023-11-07 18:45 ` Casey Schaufler
2023-11-07 13:40 ` [PATCH v5 22/23] integrity: Move integrity functions to the " Roberto Sassu
2023-11-07 18:46 ` Casey Schaufler
2023-11-16 4:33 ` Paul Moore
2023-11-16 10:07 ` Roberto Sassu
2023-11-17 21:22 ` Paul Moore
2023-11-20 13:23 ` Roberto Sassu
2023-11-07 13:40 ` [PATCH v5 23/23] integrity: Switch from rbtree to LSM-managed blob for integrity_iint_cache Roberto Sassu
2023-11-17 20:57 ` Paul Moore
2023-11-20 8:16 ` Roberto Sassu
2023-11-20 21:06 ` Paul Moore
2023-11-29 12:27 ` Roberto Sassu
2023-11-29 13:58 ` Roberto Sassu
2023-11-29 17:22 ` Paul Moore
2023-11-29 18:46 ` Roberto Sassu
2023-11-30 0:41 ` Casey Schaufler
2023-11-30 8:30 ` Petr Tesarik
2023-11-30 16:15 ` Casey Schaufler
2023-11-30 21:34 ` Roberto Sassu
2023-11-30 23:31 ` Casey Schaufler
2023-11-30 23:43 ` Roberto Sassu
2023-12-01 0:12 ` Casey Schaufler
2023-11-30 11:12 ` Mimi Zohar
2023-11-30 16:59 ` Paul Moore
2023-11-30 17:00 ` Paul Moore
2023-11-30 16:34 ` Paul Moore
2023-11-30 21:56 ` Roberto Sassu
2023-12-04 13:26 ` Roberto Sassu
2023-12-04 15:01 ` Mimi Zohar
2023-12-06 13:10 ` Roberto Sassu
2023-12-06 16:11 ` Mimi Zohar
2023-12-06 16:50 ` Roberto Sassu
2023-12-01 1:05 ` Dr. Greg
2023-12-01 18:54 ` Casey Schaufler
2023-12-01 23:53 ` Dr. Greg
2023-12-02 0:17 ` Casey Schaufler
2023-12-13 10:45 ` Roberto Sassu
2023-12-13 18:08 ` Casey Schaufler [this message]
2023-11-07 14:05 ` [PATCH v5 00/23] security: Move IMA and EVM to the LSM infrastructure Roberto Sassu
2023-11-07 19:37 ` Mimi Zohar
2023-11-08 3:14 ` Paul Moore
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9dc633d8-65a7-4b97-ab98-a21ada1d4ea5@schaufler-ca.com \
--to=casey@schaufler-ca.com \
--cc=Dai.Ngo@oracle.com \
--cc=brauner@kernel.org \
--cc=chuck.lever@oracle.com \
--cc=dhowells@redhat.com \
--cc=dmitry.kasatkin@gmail.com \
--cc=eparis@parisplace.org \
--cc=jarkko@kernel.org \
--cc=jlayton@kernel.org \
--cc=jmorris@namei.org \
--cc=keyrings@vger.kernel.org \
--cc=kolga@netapp.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mic@digikod.net \
--cc=neilb@suse.de \
--cc=paul@paul-moore.com \
--cc=roberto.sassu@huawei.com \
--cc=roberto.sassu@huaweicloud.com \
--cc=selinux@vger.kernel.org \
--cc=serge@hallyn.com \
--cc=stephen.smalley.work@gmail.com \
--cc=tom@talpey.com \
--cc=viro@zeniv.linux.org.uk \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).