linux-kernel - Re: [PATCH] security: Fix ret values doc for security_inode_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhQcVSX+kZ3PMJGJ3i-qxv9g3iP_Y4At5VCV8qSoJYj8Cg@mail.gmail.com>
Date:   Mon, 24 Jul 2023 17:54:06 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Roberto Sassu <roberto.sassu@...weicloud.com>
Cc:     jmorris@...ei.org, serge@...lyn.com,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Roberto Sassu <roberto.sassu@...wei.com>
Subject: Re: [PATCH] security: Fix ret values doc for security_inode_init_security()

On Mon, Jul 24, 2023 at 10:52 AM Roberto Sassu
<roberto.sassu@...weicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@...wei.com>
>
> Commit 6bcdfd2cac55 ("security: Allow all LSMs to provide xattrs for
> inode_init_security hook") unified the !initxattrs and initxattrs cases. By
> doing that, security_inode_init_security() cannot return -EOPNOTSUPP
> anymore, as it is always replaced with zero at the end of the function.
>
> Also, mentioning -ENOMEM as the only possible error is not correct. For
> example, evm_inode_init_security() could return -ENOKEY.
>
> Fix these issues in the documentation of security_inode_init_security().
>
> Fixes: 6bcdfd2cac55 ("security: Allow all LSMs to provide xattrs for inode_init_security hook")
> Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> ---
>  security/security.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index cfdd0cbbcb9..5aa9cb91f0f 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1604,8 +1604,8 @@ EXPORT_SYMBOL(security_dentry_create_files_as);
>   * a security attribute on this particular inode, then it should return
>   * -EOPNOTSUPP to skip this processing.
>   *
> - * Return: Returns 0 on success, -EOPNOTSUPP if no security attribute is
> - * needed, or -ENOMEM on memory allocation failure.
> + * Return: Returns 0 on success or on -EOPNOTSUPP error, a negative value other
> + *         than -EOPNOTSUPP otherwise.

How about "Returns 0 if the LSM successfully initialized all of the
inode security attributes that are required, negative values
otherwise."?  The caller doesn't need to worry about the individual
LSMs returning -EOPNOTSUPP in the case of no security attributes, and
if they really care, they are likely reading the description above (or
the code) which explains it in much better detail.

Thoughts?

--
paul-moore.com