Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
From: christophe leroy
Date: Mon Feb 26 2018 - 15:00:20 EST
Le 26/02/2018 Ã 18:50, Mathieu Malaterre a ÃcritÂ:
On Mon, Feb 26, 2018 at 8:44 AM, Mathieu Malaterre <malat@xxxxxxxxxx> wrote:
On Mon, Feb 26, 2018 at 7:50 AM, Christophe LEROY
<christophe.leroy@xxxxxx> wrote:
Le 26/02/2018 Ã 07:34, Christophe LEROY a Ãcrit :
Le 25/02/2018 Ã 18:22, Mathieu Malaterre a Ãcrit :
Rewrite check size - 1 <= Y as size < Y since `size` is unsigned value.
Fix warning (treated as error in W=1):
CC arch/powerpc/kernel/signal_32.o
In file included from ./include/linux/uaccess.h:14:0,
from ./include/asm-generic/termios-base.h:8,
from ./arch/powerpc/include/asm/termios.h:20,
from ./include/uapi/linux/termios.h:6,
from ./include/linux/tty.h:7,
from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function
âuser_termio_to_kernel_termiosâ:
./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned
expression >= 0 is always true [-Werror=type-limits]
(((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
^
./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro
â__access_okâ
__access_ok((__force unsigned long)(addr), (size), get_fs()))
^~~~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro
âaccess_okâ
if (access_ok(VERIFY_READ, __gu_addr, (size))) \
^~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro
â__get_user_checkâ
__get_user_check((x), (ptr), sizeof(*(ptr)))
^~~~~~~~~~~~~~~~
./include/asm-generic/termios-base.h:36:6: note: in expansion of macro
âget_userâ
if (get_user(termios->c_line, &termio->c_line) < 0)
^~~~~~~~
[...]
cc1: all warnings being treated as errors
Signed-off-by: Mathieu Malaterre <malat@xxxxxxxxxx>
---
arch/powerpc/include/asm/uaccess.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
index 51bfeb8777f0..fadc406bd39d 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -49,7 +49,7 @@
#define __access_ok(addr, size, segment) \
(((addr) <= (segment).seg) && \
- (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
+ (((size) == 0) || ((size) < ((segment).seg - (addr)))))
IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????
The whole series was pretty mediocre, but this one was actually pretty
destructive. Thanks for catching this.
Note that I already try to submit a fix for this warning 3 years ago
(https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the
following comment:
Tested again today with gcc 6.3.0 and gcc is still producing the
original warning (treated as error).
That's right, it seems that recent versions of gcc are not happy anymore
with that change.
Maybe Segher has a suggestion for that one ?
Again, I don't think Linux enables this warning. What did you do to
produce this? In any case, it's a bad warning that doesn't take macros
into account, and the answer is not to make the code less clear by hiding
the fact that zero is a special case.
Right. I'll try to see how to make W=1 run without error with an
alternate solution.
So the other alternative is to update a bunch of ppc32 defconfig(s)
with: CONFIG_PPC_DISABLE_WERROR=y.
Would that be preferable ?
No I don't think it is the solution. PPC is built with WERROR in order
to catch warnings on real problems.
You could disable it selectively when you want to run 'make W=1' and see
all possible warnings, then select by yourself which warnings are worth
fixing up.
Christophe
Christophe
Christophe
#endif
---
L'absence de virus dans ce courrier Ãlectronique a Ãtà vÃrifiÃe par le logiciel antivirus Avast.
https://www.avast.com/antivirus