* Re: Build regressions/improvements in v5.5-rc2
[not found] <20191216083658.21386-1-geert@linux-m68k.org>
@ 2019-12-16 8:49 ` Geert Uytterhoeven
2019-12-17 0:56 ` Greg Ungerer
0 siblings, 1 reply; 2+ messages in thread
From: Geert Uytterhoeven @ 2019-12-16 8:49 UTC (permalink / raw)
To: Greg Ungerer; +Cc: Linux Kernel Mailing List, linux-m68k
Hi Greg,
On Mon, Dec 16, 2019 at 9:38 AM Geert Uytterhoeven <geert@linux-m68k.org> 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.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Build regressions/improvements in v5.5-rc2
2019-12-16 8:49 ` Build regressions/improvements in v5.5-rc2 Geert Uytterhoeven
@ 2019-12-17 0:56 ` Greg Ungerer
0 siblings, 0 replies; 2+ messages in thread
From: Greg Ungerer @ 2019-12-17 0:56 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Linux Kernel Mailing List, linux-m68k
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@linux-m68k.org> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-17 0:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20191216083658.21386-1-geert@linux-m68k.org>
2019-12-16 8:49 ` Build regressions/improvements in v5.5-rc2 Geert Uytterhoeven
2019-12-17 0:56 ` Greg Ungerer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).