[PATCH 1/2] lsm: Fix description of fs_context_parse_param

[PATCH 1/2] lsm: Fix description of fs_context_parse_param

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

The fs_context_parse_param hook already has a description, which seems the
right one according to the code.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/lsm_hooks.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index bad3b6baad86..20e70132584c 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -100,9 +100,6 @@
  *	the filesystem.
  *	@fc indicates the filesystem context.
  *	@param The parameter.
- *	Return 0 to indicate that the parameter should be passed on to the
- *	filesystem, 1 to indicate that the parameter should be discarded or an
- *	error to indicate that the parameter should be rejected.
  *
  * Security hooks for filesystem operations.
  *
-- 
2.25.1

[PATCH 2/2] doc: Fix fs_context_parse_param description in mount_api.rst

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Align with the description of fs_context_parse_param in lsm_hooks.h, which
seems the right one according to the code.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/filesystems/mount_api.rst | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/filesystems/mount_api.rst b/Documentation/filesystems/mount_api.rst
index eb358a00be27..fc3b12cb39d1 100644
--- a/Documentation/filesystems/mount_api.rst
+++ b/Documentation/filesystems/mount_api.rst
@@ -349,13 +349,12 @@ number of operations used by the new mount code for this purpose:
 					    struct fs_parameter *param);
 
      Called for each mount parameter, including the source.  The arguments are
-     as for the ->parse_param() method.  It should return 0 to indicate that
-     the parameter should be passed on to the filesystem, 1 to indicate that
-     the parameter should be discarded or an error to indicate that the
-     parameter should be rejected.
+     as for the ->parse_param() method.  It might reject the mount parameter
+     with an error and might return 0, if the mount parameter is used by an LSM;
+     otherwise returns -ENOPARAM to pass it on to the filesystem.
 
      The value pointed to by param may be modified (if a string) or stolen
-     (provided the value pointer is NULL'd out).  If it is stolen, 1 must be
+     (provided the value pointer is NULL'd out).  If it is stolen, 0 must be
      returned to prevent it being passed to the filesystem.
 
    * ::
-- 
2.25.1

Re: [PATCH 2/2] doc: Fix fs_context_parse_param description in mount_api.rst

[Paul Moore]

On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Align with the description of fs_context_parse_param in lsm_hooks.h, which
> seems the right one according to the code.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  Documentation/filesystems/mount_api.rst | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

I'm going to leave this patch as a "hold" for right now.  The existing
text is arguably not great, but I'm not really in love with the
replacement text taken from the LSM hook comments; given the merge
window opens in a couple of days, we don't have much time to fiddle
with the wording so let's just hold this for a little bit.

These comment corrections (which are very welcome!) have also reminded
me that we really should move the hook comment blocks out of the
header file and into security.c like every other kernel function.
This should help increase their discoverability while also making it
easier to maintain the comments over time.  I'm going to post a first
pass at this as soon as the merge window closes, and once that is done
we can do further work to cleanup the descriptions and add more detail
(including notes both for the other kernel subsystems that call the
hooks and the LSM devs who provide implementations).

-- 
paul-moore.com

Re: [PATCH 2/2] doc: Fix fs_context_parse_param description in mount_api.rst

[Roberto Sassu]

On Fri, 2022-12-09 at 12:41 -0500, Paul Moore wrote:
> On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > Align with the description of fs_context_parse_param in lsm_hooks.h, which
> > seems the right one according to the code.
> > 
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  Documentation/filesystems/mount_api.rst | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> I'm going to leave this patch as a "hold" for right now.  The existing
> text is arguably not great, but I'm not really in love with the
> replacement text taken from the LSM hook comments; given the merge
> window opens in a couple of days, we don't have much time to fiddle
> with the wording so let's just hold this for a little bit.
> 
> These comment corrections (which are very welcome!) have also reminded
> me that we really should move the hook comment blocks out of the
> header file and into security.c like every other kernel function.
> This should help increase their discoverability while also making it
> easier to maintain the comments over time.  I'm going to post a first
> pass at this as soon as the merge window closes, and once that is done
> we can do further work to cleanup the descriptions and add more detail
> (including notes both for the other kernel subsystems that call the
> hooks and the LSM devs who provide implementations).

Ok, great!

Roberto

Re: [PATCH 1/2] lsm: Fix description of fs_context_parse_param

[Paul Moore]

On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> The fs_context_parse_param hook already has a description, which seems the
> right one according to the code.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  include/linux/lsm_hooks.h | 3 ---
>  1 file changed, 3 deletions(-)

I just merged this into lsm/next with a 'Fixes' tag pointing at the
previous comment block commit, thanks Roberto.

-- 
paul-moore.com

Re: [PATCH 1/2] lsm: Fix description of fs_context_parse_param

[Roberto Sassu]

On Fri, 2022-12-09 at 12:28 -0500, Paul Moore wrote:
> On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > The fs_context_parse_param hook already has a description, which seems the
> > right one according to the code.
> > 
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  include/linux/lsm_hooks.h | 3 ---
> >  1 file changed, 3 deletions(-)
> 
> I just merged this into lsm/next with a 'Fixes' tag pointing at the
> previous comment block commit, thanks Roberto.

Thanks Paul. Didn't include it, as I thought it is part of the stable
kernel process. I guess it is always fine to include it, and to not CC
the stable kernel mailing list, when the patch does not meet the
criteria.

Roberto

Re: [PATCH 1/2] lsm: Fix description of fs_context_parse_param

[Paul Moore]

On Mon, Dec 12, 2022 at 3:34 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
> On Fri, 2022-12-09 at 12:28 -0500, Paul Moore wrote:
> > On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > >
> > > The fs_context_parse_param hook already has a description, which seems the
> > > right one according to the code.
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > ---
> > >  include/linux/lsm_hooks.h | 3 ---
> > >  1 file changed, 3 deletions(-)
> >
> > I just merged this into lsm/next with a 'Fixes' tag pointing at the
> > previous comment block commit, thanks Roberto.
>
> Thanks Paul. Didn't include it, as I thought it is part of the stable
> kernel process. I guess it is always fine to include it, and to not CC
> the stable kernel mailing list, when the patch does not meet the
> criteria.

To be clear, the 'Fixes' tag here was for the previous comment fix
patch which only exists in the lsm/next branch and not any released
kernel, adding a stable/CC to this patch wouldn't have done anything
except throw up a number of automatically generated merge conflicts as
the stable folks tried to merge just this patch.  The 'Fixes' tag is
simply a bit of administrative housekeeping to connect this patch back
to the original, problematic patch; it will largely go unnoticed
unless someone decides to cherry pick patches.

When in doubt it's okay to add a Fixes tag, but leave off the
stable/CC.  In fact I prefer if people leave off the stable/CC as I've
found it to often be misused IMO; I'd rather add it when merging the
patch into one of the stable branches.

-- 
paul-moore.com

Re: [PATCH 1/2] lsm: Fix description of fs_context_parse_param

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by Paul Moore <paul@paul-moore.com>:

On Fri,  9 Dec 2022 09:29:35 +0100 you wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> The fs_context_parse_param hook already has a description, which seems the
> right one according to the code.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> 
> [...]

Here is the summary with links:
  - [1/2] lsm: Fix description of fs_context_parse_param
    https://git.kernel.org/netdev/net/c/577cc1434e4c
  - [2/2] doc: Fix fs_context_parse_param description in mount_api.rst
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 1/2] lsm: Fix description of fs_context_parse_param

[Paul Moore]

On Tue, Dec 13, 2022 at 8:08 PM <patchwork-bot+netdevbpf@kernel.org> wrote:
>
> Hello:
>
> This series was applied to netdev/net.git (master)
> by Paul Moore <paul@paul-moore.com>:
>
> On Fri,  9 Dec 2022 09:29:35 +0100 you wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > The fs_context_parse_param hook already has a description, which seems the
> > right one according to the code.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > [...]
>
> Here is the summary with links:
>   - [1/2] lsm: Fix description of fs_context_parse_param
>     https://git.kernel.org/netdev/net/c/577cc1434e4c
>   - [2/2] doc: Fix fs_context_parse_param description in mount_api.rst
>     (no matching commit)
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html

It looks like the bot has a few screws loose as this went up to Linus
via the LSM tree :)

-- 
paul-moore.com