2022-12-09 08:57:10

by Roberto Sassu

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

From: Roberto Sassu <[email protected]>

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

Signed-off-by: Roberto Sassu <[email protected]>
---
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


2022-12-09 09:38:14

by Roberto Sassu

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

From: Roberto Sassu <[email protected]>

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 <[email protected]>
---
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

2022-12-09 18:17:10

by Paul Moore

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

On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
<[email protected]> wrote:
>
> From: Roberto Sassu <[email protected]>
>
> 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 <[email protected]>
> ---
> 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

2022-12-09 18:44:08

by Paul Moore

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

On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
<[email protected]> wrote:
>
> From: Roberto Sassu <[email protected]>
>
> The fs_context_parse_param hook already has a description, which seems the
> right one according to the code.
>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
> 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

2022-12-12 08:53:28

by Roberto Sassu

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

On Fri, 2022-12-09 at 12:41 -0500, Paul Moore wrote:
> On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
> <[email protected]> wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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

2022-12-12 08:55:20

by Roberto Sassu

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

On Fri, 2022-12-09 at 12:28 -0500, Paul Moore wrote:
> On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
> <[email protected]> wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > The fs_context_parse_param hook already has a description, which seems the
> > right one according to the code.
> >
> > Signed-off-by: Roberto Sassu <[email protected]>
> > ---
> > 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

2022-12-12 20:38:29

by Paul Moore

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

On Mon, Dec 12, 2022 at 3:34 AM Roberto Sassu
<[email protected]> wrote:
> On Fri, 2022-12-09 at 12:28 -0500, Paul Moore wrote:
> > On Fri, Dec 9, 2022 at 3:30 AM Roberto Sassu
> > <[email protected]> wrote:
> > > From: Roberto Sassu <[email protected]>
> > >
> > > The fs_context_parse_param hook already has a description, which seems the
> > > right one according to the code.
> > >
> > > Signed-off-by: Roberto Sassu <[email protected]>
> > > ---
> > > 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

2022-12-14 01:50:54

by patchwork-bot+netdevbpf

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

Hello:

This series was applied to netdev/net.git (master)
by Paul Moore <[email protected]>:

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

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


2022-12-14 02:53:04

by Paul Moore

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

On Tue, Dec 13, 2022 at 8:08 PM <[email protected]> wrote:
>
> Hello:
>
> This series was applied to netdev/net.git (master)
> by Paul Moore <[email protected]>:
>
> On Fri, 9 Dec 2022 09:29:35 +0100 you wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > The fs_context_parse_param hook already has a description, which seems the
> > right one according to the code.
> >
> > Signed-off-by: Roberto Sassu <[email protected]>
> >
> > [...]
>
> 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