[PATCH] err.h: add type checking to IS_ERR_VALUE macro

* [PATCH] err.h: add type checking to IS_ERR_VALUE macro
@ 2015-12-18 10:55 Andrzej Hajda
  2015-12-18 11:08 ` kbuild test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Andrzej Hajda @ 2015-12-18 10:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
	Andrew Morton, Viresh Kumar

IS_ERR_VALUE used with some common types can work not as expected.
The problem affects all unsigned types shorter than ulong and
signed types longer than ulong.
The patch add compile time checker which reports bug in case wrong type
is passed to the macro. Type checking is performed by testing if value
of -MAX_ERRNO differes if casted to argument type and to ulong.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi,

Current implementation of IS_ERR_VALUE does not check type of its arguments.
It can result in subtle bugs in the code, especially when compiled for 64bit
architectures.
The patch tries to solve it by adding error checking. Another solution is
to change semantic of the macro, for example:

if (typeof(x) is signed)
	return x < 0;
else
	return x >= ((typeof(x))-MAX_ERRNO;

For me the latter is more natural, but changes semantic. I can prepare
patch if it is preferable.
I suppose "typeof(x) is signed" can be implemented by:
(typeof(x))(-1) < 0
and 'if' clause can be replaced by __builtin_choose_expr.

Finally simplified SmPL patch to find all occurences of suspected code:

virtual context

@@
typedef bool, u8, u16, u32, s64;
{unsigned char, unsigned short, unsigned int, long long, bool, u8, u16, u32, s64} e;
position p;
@@
* IS_ERR_VALUE(e@p)

It detected about 20 suspects.

Regards
Andrzej
---
 include/linux/err.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/err.h b/include/linux/err.h
index 56762ab..5cdf849 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -1,6 +1,7 @@
 #ifndef _LINUX_ERR_H
 #define _LINUX_ERR_H
 
+#include <linux/bug.h>
 #include <linux/compiler.h>
 #include <linux/types.h>
 
@@ -18,7 +19,11 @@
 
 #ifndef __ASSEMBLY__
 
-#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
+#define IS_ERR_VALUE(x) \
+({ \
+    BUILD_BUG_ON_MSG((typeof(x))(-MAX_ERRNO) != (unsigned long)-MAX_ERRNO, "Invalid IS_ERR_VALUE argument type");\
+    unlikely((x) >= (unsigned long)-MAX_ERRNO);\
+})
 
 static inline void * __must_check ERR_PTR(long error)
 {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread