mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Günther Noack" <gnoack@google.com>
To: Christopher Lusk <clusk@northecho.dev>
Cc: "Mickaël Salaün" <mic@digikod.net>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] docs: landlock: clarify TTY signal scoping
Date: Wed, 16 Sep 2026 14:18:09 +0200	[thread overview]
Message-ID: <aqqJAZfG9FC7PgMW@google.com> (raw)
In-Reply-To: <20260914180946.1462099-1-clusk@northecho.dev>

Hello!

Thanks for the review!

On Mon, Sep 14, 2026 at 02:09:46PM -0400, Christopher Lusk wrote:
> LANDLOCK_SCOPE_SIGNAL mediates signal delivery when a sandboxed process
> selects the recipient, including SIGIO through fowner.  It does not mediate
> signals directed by the TTY layer to processes attached to a terminal in
> response to terminal activity.  This distinction was clarified while
> discussing TIOCSIG handling because the PTY master acts as a capability and
> the signal recipients have attached to the terminal.
> 
> Document the TTY-driven signal paths which are outside the scope and advise
> controlling access to the terminal or PTY master instead.  This records the
> outcome of the RFC discussion and avoids implying that
> LANDLOCK_SCOPE_SIGNAL covers every signal-delivery mechanism.
> 
> The documentation text and changelog were drafted with assistance from
> Claude (claude-opus-4-8) and Codex (gpt-5.6-sol).
> 
> The userspace API documentation builds successfully with the kernel-pinned
> Sphinx dependencies.  The remaining warnings are unrelated to the changed
> Landlock text.
> 
> Link: https://lore.kernel.org/r/20260914.b8a029f9abb8@gnoack.org
> Suggested-by: Günther Noack <gnoack@google.com>
> Assisted-by: Claude:claude-opus-4-8
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Christopher Lusk <clusk@northecho.dev>
> ---
>  Documentation/userspace-api/landlock.rst | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 84cb7bf6b3ed..64418b09840d 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -430,6 +430,21 @@ The operations which can be scoped are:
>      This limits the sending of signals to target processes which run within the
>      same or a nested Landlock domain.
>  
> +    This scope does not cover signals delivered by the TTY layer.  A process
> +    holding a PTY master, or otherwise driving a terminal, can cause the TTY
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

What other ways of driving a terminal are you alluding to?  (I assume
the LLM wrote that?  Did it make that up, or does it have another trick
up its sleeve for sending signals that we are overlooking?)


> +    driver to deliver signals to processes attached to that terminal across
> +    Landlock domain boundaries.  This includes ``SIGINT``, ``SIGQUIT``, and
> +    ``SIGTSTP`` via the ``TIOCSIG`` :manpage:`ioctl(2)` command or the
> +    corresponding control characters.  The TTY layer may also deliver
> +    ``SIGWINCH``, ``SIGHUP``, and ``SIGCONT``.
> +
> +    These signals originate from the TTY driver in response to terminal
> +    activity rather than from a :manpage:`kill(2)`-style request.  They can
> +    only reach processes attached to the terminal.  To restrict this
> +    interaction, control possession of the PTY master and terminal attachment.
> +    For example, do not pass a PTY master to a sandboxed process if its slave
> +    has processes from outside the Landlock domain attached to it.
> +

LANDLOCK_SCOPE_SIGNAL was previously described in two lines here.
Now we have 15 lines, 13 of which are talking exclusively about the PTY corner
case.  I am afraid this will water down the main message here.

I understand that LLMs can help in writing good English, but they also
have a tendency to be much more verbose than the existing text and can
direct the reader's attention away from the main points with that.

Suggested replacement:

  Holding a PTY master FD still grants the capability to issue signals
  through that PTY to the processes running under that terminal.

Does that seem reasonable?

Listing the full list of signals that can be sent through a PTY seems like an
excessive level of detail here.  But if you find a reference listing
the same signals in kernel doc or a stable URL, we could link it from
the docs.


Please also add an (even shorter) remark to the landlock.h header
file's description of LANDLOCK_SCOPE_SIGNAL, similar in length to the
one we added for the whiteout objects for LANDLOCK_ACCESS_FS_MAKE_REG
recently.

>  ``LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET``
>      This limits the set of abstract :manpage:`unix(7)` sockets to which we can
>      :manpage:`connect(2)` to socket addresses which were created by a process in
> -- 
> 2.55.0
> 

Some meta-level remarks:

* Please send new patch sets as top-level emails rather than as
  responses to existing mail threads.  (It is not technically wrong to
  do that, but they do get overlooked within mail threads more often.
  To connect the dots, you can link the original mail on lore as
  you've already done here as well.)

* *If you want*, a thing that would still be worthwhile having in code
  would be a regression selftest for this signal sending path.  This
  is similar to the tests you've already created in your initial
  patchset, but would now check that signal sending *works* despite
  the sender being in the scoped domain.

(For transparency, I should remark that the final decision on this
code review is still up to Mickaël in the end.  I do believe that this
approach is the best solution, but the reasoning is less clear-cut
than in other bugs we had before.)

Thanks,
—Günther

  reply	other threads:[~2026-09-16 12:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-13 22:19 [RFC PATCH 0/2] Landlock signal scope and TIOCSIG Christopher Lusk
2026-09-13 22:19 ` [RFC PATCH 1/2] tty: mediate TIOCSIG through task_kill LSM hooks Christopher Lusk
2026-09-13 23:49   ` Christopher Lusk
2026-09-13 22:19 ` [RFC PATCH 2/2] selftests/landlock: cover TIOCSIG signal scoping Christopher Lusk
2026-09-14  9:34 ` [RFC PATCH 0/2] Landlock signal scope and TIOCSIG Günther Noack
2026-09-14 13:40   ` Christopher Lusk
2026-09-14 17:13     ` Günther Noack
2026-09-14 18:09   ` [PATCH] docs: landlock: clarify TTY signal scoping Christopher Lusk
2026-09-16 12:18     ` Günther Noack [this message]
2026-09-16 14:52       ` Christopher Lusk
2026-09-16 15:42         ` Günther Noack

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=aqqJAZfG9FC7PgMW@google.com \
    --to=gnoack@google.com \
    --cc=clusk@northecho.dev \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mic@digikod.net \
    --cc=rdunlap@infradead.org \
    --cc=skhan@linuxfoundation.org \
    /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

all inboxes | Powered by JetHome®