Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: SYS_pinsyscalls question
To:
"Theo de Raadt" <deraadt@openbsd.org>
Cc:
mjguzik@gmail.com, marc.espie.openbsd@gmail.com, tech@openbsd.org
Date:
Mon, 03 Mar 2025 11:50:30 +0100

Download raw body.

Thread
> From: "Theo de Raadt" <deraadt@openbsd.org>
> Date: Sun, 02 Mar 2025 16:35:51 -0700
> 
> > I think the worst case is when the thread sees the correct pn_end,
> > pn_pins and pn_npins, but pn_start is still zero.  That could
> > potentially permit a syscall that shouldn't be allowed.
> 
> No.
> 
>         if (plibcpin->pn_pins &&
>             addr >= plibcpin->pn_start && addr < plibcpin->pn_end)
>                 pin = plibcpin;
> 
> "addr" cannot be zero, because we don't allow mapping the NULL page
> in userland.

That doesn't really matter for the scenario I have in mind.

What I'm talking about is the situation where a thread sees:

  pn_start = 0
  pn_end != 0
  pn_npins != 0

and addr < pn_end but also addr < intended value of pn_start.  In that
scenario we'd pick the ps_libcpin table instead of the ps_pin table.
And the ps_libcpin table might allow the syscall if:

1. The table entry is -1.  This should be harmless since only "safe"
   system calls should have their entry set to -1.

2. The table entry has an address and the binary has been crafted such
   that the system call is at the "unrelocated" address that's stored
   in the table.  Seems convoluted to me, but not impossible.

> I wonder if we can change >= to >

I don't see how that would help.  And it would be wrong if we managed
to shrink the syscall area to the absolute minimum where the first
syscall instruction happens to live at pn_start.  This is possible on
architectures where the syscall number is encoded in the syscall
instruction itself.

Anyway, I think it wouldn't be a bad idea to actually have the
pinsyscalls(2) implementation check that we're single-threaded and
document it as such.  I think it is good enough to return an error in
that case.  I really don't see a use case where calling pinsyscalls(2)
in a multi-threaded process would make any sense.