Re: [PATCH man-pages v2] madvise.2: add MADV_GUARD_INSTALL, MADV_GUARD_REMOVE description

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Fri, Nov 29, 2024 at 07:13:22PM +0100, Jann Horn wrote:
> On Fri, Nov 29, 2024 at 4:59 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> > Lightweight guard region support has been added to Linux 6.13, which adds
> > MADV_GUARD_INSTALL and MADV_GUARD_REMOVE flags to the madvise() system
> > call. Therefore, update the manpage for madvise() and describe these
> > operations.
> [...]
> > +.TP
> > +.BR MADV_GUARD_INSTALL " (since Linux 6.13)"
> > +Install a lightweight guard region into the range specified by
> > +.I addr
> > +and
> > +.IR size ,
> > +causing any read or write in the range to result in a fatal
> > +.B SIGSEGV
> > +signal being raised.
>
> Single-word nitpick: Maybe remove the word "fatal"?
>
> I think the term "fatal signal" normally refers to a signal that is
> guaranteed to terminate the task (that's how the signal handling code
> uses the term, more or less); but a SIGSEGV caused by VM_FAULT_SIGSEGV
> can AFAIK be handled by a userspace signal handler.
>
> SIGKILL is the one signal that is always fatal; the kernel can also
> send other signals in an always-fatal way, like with force_fatal_sig()
> or force_exit_sig(), but those are not used for VM_FAULT_SIGSEGV.
> (Those functions are mostly for cases where we can't continue because
> something is in an unsafe state, like if a signal return failed and
> the register state might now be messed up.)

I think there's a bit of a disconnect between the meaning of a fatal signal
in userland and the kernel, from the kerne's perspective as per
fatal_signal_pending(), it is, as you say, SIGKILL.

From a user's persepctive, and as per sig_fatal(), it is one that is, by
default, fatal if not handled.

So I think here it's fine to say 'fatal' in the latter sense, and the fact
we immediately mention SIGSEGV clarifies in what sense we mean 'fatal'.

The intent here also is that a user would treat this as a fatal event, a
thread that accesses a guard area is accessing memory that it shouldn't.

However I also see it from your perspective, I mean we say what signal
we're sending so it's not hugely necessary and eliminates a possible
confusion.

Not sure if Alejandro has any objection to this turn of phrase?

From my perspective I don't think it's too problematic to leave it in, but
if it's easy for Alejandro to pull out I have no objection.

If people feel strongly + Alejandro would find it easier, I could just send
a v3 with it removed.

Thanks, Lorenzo