Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201012014837.14305-1-jrtc27@jrtc27.com>
In-Reply-To: <20201012014837.14305-1-jrtc27@jrtc27.com>
From:   Andy Lutomirski <luto@kernel.org>
Date:   Sun, 11 Oct 2020 20:02:21 -0700
Message-ID: <CALCETrV4BFQ_Cdt98NTWnzQ1H4eJfzOpz_UO=Ak+i6jDQAmcrA@mail.gmail.com>
Subject: Re: [PATCH] x86: Fix x32 System V message queue syscalls
To:     Jessica Clarke <jrtc27@jrtc27.com>
Cc:     linux-x86_64@vger.kernel.org, Andy Lutomirski <luto@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        X86 ML <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Brian Gerst <brgerst@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Oct 11, 2020 at 6:48 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> POSIX specifies that the first field of the supplied msgp, namely mtype,
> is a long, not a __kernel_long_t, and it's a user-defined struct due to
> the variable-length mtext field so we can't even bend the spec and make
> it a __kernel_long_t even if we wanted to. Thus we must use the compat
> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> msgrcv respectively.
>
> Due to erroneously including the first 4 bytes of mtext in the mtype
> this would previously also cause non-zero msgtyp arguments for msgrcv to
> search for the wrong messages, and if sharing message queues between x32
> and non-x32 (i386 or x86_64) processes this would previously cause mtext
> to "move" and, depending on the direction and ABI combination, lose the
> first 4 bytes.

This has a few problems.

First, the 512-547 range is a legacy wart and we shouldn't extend it.
I thought I had documented this, but I didn't -- oops.  Patch sent.
The correct way to do this nowadays is to use the same number twice,
once for x64 and once for x32.  If you try to do this and encounter
problems with the build, please either fix them or let me know :)

Second, since this is an ABI issue, can you please include a little
test case that compiles for i386, x86_64 and x32, works correctly on
all three with whatever patch you send, and fails on x32 without the
patch?

Third, how does this interact with various libc implementations?
msgsnd(2) and msgrcv(2) have glibc wrappers.  Have they never been
tested on x32?

Fourth, if there is some deployed code that uses the buggy x32 path
and actually works by accident, do we risk breaking it if we fix the
bug?

--Andy