linux-kernel - [PATCH] err.h: add type checking to IS_ERR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 18 Dec 2015 11:55:02 +0100
From:	Andrzej Hajda <a.hajda@...sung.com>
To:	linux-kernel@...r.kernel.org
Cc:	Andrzej Hajda <a.hajda@...sung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Viresh Kumar <viresh.kumar@...aro.org>
Subject: [PATCH] err.h: add type checking to IS_ERR_VALUE macro

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@...sung.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/