Re: Build regressions/improvements in v5.5-rc2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

On 16/12/19 6:49 pm, Geert Uytterhoeven wrote:
Hi Greg,

On Mon, Dec 16, 2019 at 9:38 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
Below is the list of build error/warning regressions/improvements in
v5.5-rc2[1] compared to v5.4[2].

[1] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/d1eef1c619749b2a57e514a3fa67d9a516ffa919/ (all 232 configs)
[2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/219d54332a09e8d8741c1e1982f5eae56099de85/ (all 232 configs)

   + /kisskb/src/arch/m68k/include/asm/string.h: warning: '__builtin_memcpy' forming offset [3, 4] is out of the bounds [0, 2] of object '__gu_val' with type 'short unsigned int' [-Warray-bounds]:  => 72:25

The upgrade from gcc 4.6.3 to 81.0 seems to have revealed a potential
issue in get_user() in arch/m68k/include/asm/uaccess_no.h.

         #define get_user(x, ptr)                                        \
         ({                                                              \
             int __gu_err = 0;                                           \
             typeof(x) __gu_val = 0;                                     \
             ^^^^^^^^^
             This is the type of the destination

             switch (sizeof(*(ptr))) {                                   \
             case 1:                                                     \
                 __get_user_asm(__gu_err, __gu_val, ptr, b, "=d");       \
                 break;                                                  \
             case 2:                                                     \
                 __get_user_asm(__gu_err, __gu_val, ptr, w, "=r");       \
                 break;                                                  \
             case 4:                                                     \
                 __get_user_asm(__gu_err, __gu_val, ptr, l, "=r");       \
                 break;                                                  \
             case 8:                                                     \
                 memcpy((void *) &__gu_val, ptr, sizeof (*(ptr)));       \
                 break;                                                  \
             default:                                                    \
                 __gu_val = 0;                                           \
                 __gu_err = __get_user_bad();                            \
                 break;                                                  \
             }                                                           \
             (x) = (typeof(*(ptr))) __gu_val;                            \
             __gu_err;                                                   \
         })

ext2_ioctl() calls this like

         unsigned short rsv_window_size;
         if (get_user(rsv_window_size, (int __user *)arg)) { ... }

So a 32-bit value is being copied to an unsigned short value, leading
to the warning (for the memcpy() in the non-taken "case 8" branch).

Fortunately the compiler emits a register move for this, so no real harm
is done:

         | fs/ext2/ioctl.c:123:          if (get_user(rsv_window_size,
(int __user *)arg))
                 move.l 48(%sp),%a0      | arg,
         #APP
         | 123 "fs/ext2/ioctl.c" 1
                 movel (%a0),%d2 | *arg.32_43, __gu_val

The corresponding code in arch/m68k/include/asm/uaccess_mm.h
uses a temporary __gu_val of the right sized type based on the source
type to avoid this.

Yeah, your right. I will put a change together to fix.

Regards
Greg




[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux