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: <20210927161955.28494-1-rpalethorpe@suse.com>
In-Reply-To: <20210927161955.28494-1-rpalethorpe@suse.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Mon, 27 Sep 2021 20:22:23 +0200
Message-ID: <CAK8P3a2kuZkoF1m0pTXWeq0gnzUG9oSTQKtjjdiCyo1ptmXPgA@mail.gmail.com>
Subject: Re: [PATCH] x86/entry/ia32: Ensure s32 is sign extended to s64
To:     Richard Palethorpe <rpalethorpe@suse.com>
Cc:     "the arch/x86 maintainers" <x86@kernel.org>,
        Heiko Carstens <hca@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Christian Borntraeger <borntraeger@de.ibm.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>,
        linux-s390 <linux-s390@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux API <linux-api@vger.kernel.org>,
        Arnd Bergmann <arnd@arndb.de>, rpalethorpe@richiejp.com,
        Dominik Brodowski <linux@dominikbrodowski.net>,
        LTP List <ltp@lists.linux.it>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Sep 27, 2021 at 6:21 PM Richard Palethorpe <rpalethorpe@suse.com> wrote:
>
> Presently ia32 registers stored in ptregs are unconditionally cast to
> unsigned int by the ia32 stub. They are then cast to long when passed
> to __se_sys*, but will not be sign extended.
>
> This takes the sign of the syscall argument into account in the ia32
> stub. It still casts to unsigned int to avoid implementation specific
> behavior. However then casts to int or unsigned int as necessary. So
> that the following cast to long sign extends the value.
>
> This fixes the io_pgetevents02 LTP test when compiled with
> -m32. Presently the systemcall io_pgetevents_time64 unexpectedly
> accepts -1 for the maximum number of events. It doesn't appear other
> systemcalls with signed arguments are effected because they all have
> compat variants defined and wired up. A less general solution is to
> wire up the systemcall:
> https://lore.kernel.org/ltp/20210921130127.24131-1-rpalethorpe@suse.com/
>
> Fixes: ebeb8c82ffaf ("syscalls/x86: Use 'struct pt_regs' based syscall calling for IA32_EMULATION and x32")
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>

Looks good to me, thanks for following through with this part, and for
checking the other syscalls!

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

I've added this to my randconfig build tree as well, to see if
it causes any unexpected compile-time issues, though I
don't expect any problems here.

There are a few things that I think we should do as a follow-up:

- do the same thing in the generic syscall wrapper, to ensure the
  other architectures also do the sign-extension.

- Fix the big-endian architectures (ppc64be, mips64be, sparc, s390
  parisc) so they pass the correct signal mask, either using your original
  approach, or by reworking the syscall to detect compat syscalls
  at runtime, killing off the separate entry point

- Go through the compat syscalls to see if any of them can be
  removed once all architectures do sign-extension correctly.

Are you motivated to help out with one or more of these as well?

       Arnd