Messages in this thread | | | From | Paul Moore <> | Date | Tue, 25 Jul 2023 14:38:26 -0400 | Subject | Re: [PATCH] security: Fix ret values doc for security_inode_init_security() |
| |
On Tue, Jul 25, 2023 at 3:02 AM Roberto Sassu <roberto.sassu@huaweicloud.com> wrote: > On Mon, 2023-07-24 at 17:54 -0400, Paul Moore wrote: > > On Mon, Jul 24, 2023 at 10:52 AM Roberto Sassu > > <roberto.sassu@huaweicloud.com> wrote: > > > > > > From: Roberto Sassu <roberto.sassu@huawei.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@huawei.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. > > Maybe this could be better: > > Return 0 if security attributes initialization is successful or not > necessary, a negative value otherwise.
Well, I'm trying to avoid differentiating between the non-zero, but successful attribute initialization and the zero attribute case; from a caller's perspective it doesn't matter (and why we don't differentiate between the two with different error codes). If the distinction between the two states is important from a caller's perspective, there should be different return codes.
-- paul-moore.com
| |