Messages in this thread | | | Subject | Re: [PATCH] security: Fix ret values doc for security_inode_init_security() | From | Roberto Sassu <> | Date | Wed, 26 Jul 2023 09:29:42 +0200 |
| |
On Tue, 2023-07-25 at 14:38 -0400, Paul Moore wrote: > 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.
Ok, fine for me. I take your suggestion.
Thanks
Roberto
| |