Re: ia32 signed long treated as x64 unsigned int by __ia32_sys*

[Arnd Bergmann <arnd@arndb.de>]

On Wed, Sep 22, 2021 at 10:46 AM Richard Palethorpe <rpalethorpe@suse.de> wrote:
> Richard Palethorpe <rpalethorpe@suse.de> writes:

> >
> > Then the output is:
> >
> > [   11.252268] io_pgetevents(f7f19000, 4294967295, 1, ...)
> > [   11.252401] comparing 4294967295 <= 1
> > io_pgetevents02.c:114: TPASS: invalid min_nr: io_pgetevents() failed as expected: EINVAL (22)
> > [   11.252610] io_pgetevents(f7f19000, 1, 4294967295, ...)
> > [   11.252748] comparing 1 <= 4294967295
> > io_pgetevents02.c:103: TFAIL: invalid max_nr: io_pgetevents() passed unexpectedly
>
> and below is the macro expansion for the automatically generated 32bit to
> 64bit io_pgetevents. I believe it is casting u32 to s64, which appears
> to mean there is no sign extension. I don't know if this is the expected
> behaviour?

Thank you for digging through this, I meant to already reply once more yesterday
but didn't get around to that.

>     __typeof(__builtin_choose_expr(
>         (__builtin_types_compatible_p(typeof((long)0), typeof(0LL)) ||
>          __builtin_types_compatible_p(typeof((long)0), typeof(0ULL))),
>         0LL, 0L)) min_nr,
>     __typeof(__builtin_choose_expr(
>         (__builtin_types_compatible_p(typeof((long)0), typeof(0LL)) ||
>          __builtin_types_compatible_p(typeof((long)0), typeof(0ULL))),
>         0LL, 0L)) nr,

The part that I remembered is in arch/s390/include/asm/syscall_wrapper.h,
which uses this version instead:

#define __SC_COMPAT_CAST(t, a)                                          \
({                                                                      \
        long __ReS = a;                                                 \
                                                                        \
        BUILD_BUG_ON((sizeof(t) > 4) && !__TYPE_IS_L(t) &&              \
                     !__TYPE_IS_UL(t) && !__TYPE_IS_PTR(t) &&           \
                     !__TYPE_IS_LL(t));                                 \
        if (__TYPE_IS_L(t))                                             \
                __ReS = (s32)a;                                         \
        if (__TYPE_IS_UL(t))                                            \
                __ReS = (u32)a;                                         \
        if (__TYPE_IS_PTR(t))                                           \
                __ReS = a & 0x7fffffff;                                 \
        if (__TYPE_IS_LL(t))                                            \
                return -ENOSYS;                                         \
        (t)__ReS;                                                       \
})

This also takes care of s390-specific pointer conversion, which is the
reason for needing an architecture-specific wrapper, but I suppose the
handling of signed arguments as done in s390 should also be done
everywhere else.

I also noticed that only x86 and s390 even have separate entry
points for normal syscalls when called in compat mode, while
the others all just zero the upper halves of the registers in the
low-level entry code and then call the native entry point.

        Arnd