From: Andrzej Hajda <a.hajda@samsung.com>
To: linux-kernel@vger.kernel.org
Cc: Andrzej Hajda <a.hajda@samsung.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Andrew Morton <akpm@linux-foundation.org>,
Viresh Kumar <viresh.kumar@linaro.org>
Subject: [PATCH] err.h: add type checking to IS_ERR_VALUE macro
Date: Fri, 18 Dec 2015 11:55:02 +0100 [thread overview]
Message-ID: <1450436102-6360-1-git-send-email-a.hajda@samsung.com> (raw)
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
next reply other threads:[~2015-12-18 10:55 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-18 10:55 Andrzej Hajda [this message]
2015-12-18 11:08 ` kbuild test robot
2015-12-18 11:22 ` kbuild test robot
2015-12-18 14:02 ` Andrzej Hajda
2015-12-18 11:28 ` kbuild test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1450436102-6360-1-git-send-email-a.hajda@samsung.com \
--to=a.hajda@samsung.com \
--cc=akpm@linux-foundation.org \
--cc=b.zolnierkie@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome