From: Alejandro Colomar <alx@kernel.org>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: "Günther Noack" <gnoack@google.com>,
"Paul Moore" <paul@paul-moore.com>,
"Serge E . Hallyn" <serge@hallyn.com>,
"Konstantin Meskhidze" <konstantin.meskhidze@huawei.com>,
"Shervin Oloumi" <enlightened@chromium.org>,
linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
"Kees Cook" <keescook@chromium.org>
Subject: Re: [PATCH v2 1/2] landlock: Extend documentation for kernel support
Date: Tue, 19 Mar 2024 12:40:17 +0100 [thread overview]
Message-ID: <Zfl5oT1amymvxjjL@debian> (raw)
In-Reply-To: <20240319.eeb8tajeiPee@digikod.net>
[-- Attachment #1: Type: text/plain, Size: 4852 bytes --]
Hi Mickaël!
On Tue, Mar 19, 2024 at 11:46:34AM +0100, Mickaël Salaün wrote:
> On Mon, Mar 18, 2024 at 10:50:42AM +0100, Alejandro Colomar wrote:
> > Hi Mickaël, Günther,
> >
> > Sorry for the delay!
> >
> > On Thu, Mar 07, 2024 at 11:21:57AM +0100, Mickaël Salaün wrote:
> > > CCing Alejandro
> > >
> > > On Tue, Feb 27, 2024 at 05:32:20PM +0100, Günther Noack wrote:
> > > > On Tue, Feb 27, 2024 at 12:05:49PM +0100, Mickaël Salaün wrote:
> > > > > Extend the kernel support section with one subsection for build time
> > > > > configuration and another for boot time configuration.
> > > > >
> > > > > Extend the boot time subsection with a concrete example.
> > > > >
> > > > > Update the journalctl command to include the boot option.
> > > > >
> > > > > Cc: Günther Noack <gnoack@google.com>
> > > > > Cc: Kees Cook <keescook@chromium.org>
> > > > > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > > > > ---
> > > > >
> > > > > Changes since v1:
> > > > > * New patch, suggested by Kees Cook.
> > > > > ---
> > > > > Documentation/userspace-api/landlock.rst | 57 +++++++++++++++++++++---
> > > > > 1 file changed, 51 insertions(+), 6 deletions(-)
> >
> > [...]
> >
> > > > > +
> > > > > + lsm=landlock,lockdown,yama,integrity,apparmor
> > > > > +
> > > > > +After a reboot, we can check that Landlock is up and running by looking at
> > > > > +kernel logs:
> > > > > +
> > > > > +.. code-block:: console
> > > > > +
> > > > > + # dmesg | grep landlock || journalctl -kb -g landlock
> > > > > + [ 0.000000] Command line: [...] lsm=landlock,lockdown,yama,integrity,apparmor
> > > > > + [ 0.000000] Kernel command line: [...] lsm=landlock,lockdown,yama,integrity,apparmor
> > > > > + [ 0.000000] LSM: initializing lsm=lockdown,capability,landlock,yama,integrity,apparmor
> > > > > + [ 0.000000] landlock: Up and running.
> > > > > +
> > > > > +Note that according to the built time kernel configuration,
> > > >
> > > > s/built time/build time/
> > > > ^
> > >
> > > OK
> >
> > Here, this should actually be "build-time" since it works as an
> > adjective.
>
> Thanks Alex but this was already merged:
> https://git.kernel.org/torvalds/c/35e886e88c803920644c9d3abb45a9ecb7f1e761
>
> Because I picked Günther's below suggestion, it should be good right?
Yeah, it's a minor grammar mistake that is widespread elsewhere. If you
want to patch it, go ahead, if you want to keep it until next time you
revise this text, it's not something that will significantly hurt the
understanding of the text.
See also: <https://www.grammar-monster.com/lessons/hyphens_in_compound_adjectives.htm>
Have a lovely day!
Alex
>
> >
> > >
> > > >
> > > > It feels like the phrase "according to" could be slightly more specific here.
> > > >
> > > > To paraphrase Alejandro Colomar, "Note that" is usually redundant.
> > > > https://lore.kernel.org/all/0aafcdd6-4ac7-8501-c607-9a24a98597d7@gmail.com/
> > > >
> > > > I'd suggest:
> > > >
> > > > The kernel may be configured at build time to always load the ``lockdown`` and
> > > > ``capability`` LSMs. In that case, these LSMs will appear at the beginning of
> > > > the ``LSM: initializing`` log line as well, even if they are not configured in
> > > > the boot loader.
> >
> > LGTM
> >
> > >
> > > OK, I integrated your suggestion. I guess `capability` is not really
> > > considered an LSM but it would be too confusing and out of scope for an
> > > user documentation to explain that.
> > >
> > > >
> > > > > +``lockdown,capability,`` may always stay at the beginning of the ``LSM:
> > > > > +initializing lsm=`` list even if they are not configured with the bootloader,
> > > >
> > > > Nit: The man pages spell this in two words as "boot loader".
> > >
> > > OK, I'll use "boot loader" too.
> > >
> > > >
> > > >
> > > > > +which is OK.
> > > > > +
> > > > > +Network support
> > > > > +---------------
> > > > > +
> > > > > To be able to explicitly allow TCP operations (e.g., adding a network rule with
> > > > > ``LANDLOCK_ACCESS_NET_BIND_TCP``), the kernel must support TCP
> > > > > (``CONFIG_INET=y``). Otherwise, sys_landlock_add_rule() returns an
> > > > >
> > > > > base-commit: b4007fd27206c478a4b76e299bddf4a71787f520
> > > > > --
> > > > > 2.44.0
> > > > >
> > > >
> > > > Reviewed-by: Günther Noack <gnoack@google.com>
> > >
> > > Thanks!
> >
> > Reviewed-by: Alejandro Colomar <alx@kernel.org>
> >
> > Have a lovely day!
> > Alex
> >
> > --
> > <https://www.alejandro-colomar.es/>
> > Looking for a remote C programming job at the moment.
>
>
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
prev parent reply other threads:[~2024-03-19 11:40 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-27 11:05 [PATCH v2 1/2] landlock: Extend documentation for kernel support Mickaël Salaün
2024-02-27 11:05 ` [PATCH v2 2/2] landlock: Warn once if a Landlock action is requested while disabled Mickaël Salaün
2024-02-27 16:32 ` [PATCH v2 1/2] landlock: Extend documentation for kernel support Günther Noack
2024-03-07 10:21 ` Mickaël Salaün
2024-03-18 9:50 ` Alejandro Colomar
2024-03-19 10:46 ` Mickaël Salaün
2024-03-19 11:40 ` Alejandro Colomar [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zfl5oT1amymvxjjL@debian \
--to=alx@kernel.org \
--cc=enlightened@chromium.org \
--cc=gnoack@google.com \
--cc=keescook@chromium.org \
--cc=konstantin.meskhidze@huawei.com \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mic@digikod.net \
--cc=paul@paul-moore.com \
--cc=serge@hallyn.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).