Compat sysinfo syscall (kernel/sys.c) relying on undefined behavior?

Compat sysinfo syscall (kernel/sys.c) relying on undefined behavior?

[Scotty Bauer]

 am getting acquainted with the linux kernel and to do so I've been browsing the source.


In the compat version of sysinfo, kernel/sys.c we see the following:

COMPAT_SYSCALL_DEFINE1(sysinfo, struct compat_sysinfo __user *, info)
{
    struct sysinfo s;

    do_sysinfo(&s);

    /* Check to see if any memory value is too large for 32-bit and scale
     *  down if needed
     */
    if ((s.totalram >> 32) || (s.totalswap >> 32)) {
        int bitcount = 0;

...


s.totalram is a u32, and the standard says:
"If the value of the right operand is negative or is greater than or equal to the width 
of the promoted left operand, the behavior is undefined."

Is there some promotion, compiler flag, something obvious that I am missing, or is this a 
problem?


Best,
Scotty

Re: Compat sysinfo syscall (kernel/sys.c) relying on undefined behavior?

[Clemens Ladisch]

Scotty Bauer wrote:
> In the compat version of sysinfo, kernel/sys.c we see the following:
>
>     /* Check to see if any memory value is too large for 32-bit and scale
>      *  down if needed
>      */
>     if ((s.totalram >> 32) || (s.totalswap >> 32)) {

This code is supposed to check if any of the bits in the upper half of
these 64-bit values are set.

> s.totalram is a u32

Oops.

> the behavior is undefined.

For a constant shift amount, gcc happens to generate correct code, i.e.,
the result is zero.  (If the shift amount were not a constant, x86
processors would use only its lowest five bits, and the result would be
wrong.)

Anyway, it's not a good idea to rely on gcc's implementation of this
undefined behaviour; the code should have used upper_32_bits() instead.
Please write a patch.


Regards,
Clemens