Mark Kettenis <mark.kettenis@xs4all.nl> wrote:

> here.  Although that can only happen if you write your own version of
> ld.so that creates threads before pinning down the syscalls using
> pinsyscalls(2).
> 
> The pinsyscals(2) code does
> 
>         pr->ps_libcpin.pn_start = base;
>         pr->ps_libcpin.pn_end = base + len;
>         pr->ps_libcpin.pn_pins = pins;
>         pr->ps_libcpin.pn_npins = npins;
> 
> but another thread may observe those assignments in a different order
> because there are no (memory) barriers.

I still don't see it.  The testing condition in syscall_mi.h is this:

        if (plibcpin->pn_pins &&
            addr >= plibcpin->pn_start && addr < plibcpin->pn_end)
                pin = plibcpin;

At process start, the 3 libcpin variables are set to 0.

This test is checking if all 3 have become non-0.

If any are still 0, the libc pin is not chosen (goes into "pin"
variable).

Please don't be confused by these "else" statements in the block:

        if (plibcpin->pn_pins &&
            addr >= plibcpin->pn_start && addr < plibcpin->pn_end)
                pin = plibcpin;
        else if (ppin->pn_pins &&
            addr >= ppin->pn_start && addr < ppin->pn_end)
                pin = ppin;
        else if (PROC_PC(p) == pr->ps_sigcoderet) {

This could be rewritten as

        if (plibcpin->pn_pins &&
            addr >= plibcpin->pn_start && addr < plibcpin->pn_end)
                pin = plibcpin;
        if (ppin->pn_pins &&
            addr >= ppin->pn_start && addr < ppin->pn_end)
                pin = ppin;
        if (PROC_PC(p) == pr->ps_sigcoderet) {
                if (code == SYS_sigreturn)
                        return (0);
                error = EPERM;
                goto die;
        }

The "addr" or "PROC_PC(p)" cannot be inside all 3 of these ranges
because these ranges cannot overlap.  So there is no priority scheme
occuring here, the else's are improving the logic by avoiding pointless
tests.

> Fortunately the code that
> checks the libc.so syscall range explicitly checks pn_pins,

This won't happen, unless the address of those other threads is inside
ld.so.  If the other thread is also doing a syscall from inside ld.so,
then it is not doing it within the libc range, so it should be permitted
based upon the ld.so pin table, which is ppin, not libcpin, this is case

         * 2b) dynamic binary: syscalls in ld.so (in the ps_pin region)

And ps_pin is not changed by pinsyscalls(), it is established at execve()
time.

> 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.

If pn_start is zero, then that pintable is not accepted, variable "pin"
remains NULL, and therefore errno = ENOSYS, and the entire process reaches
sigabort().


I don't see the problem at all.