linux-kernel - [PATCH] Make sure abs() works as expected with both ints and longs

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-prev] [day] [month] [year] [list]

Message-ID: <20100728152849.GA3728@quadria>
Date:	Wed, 28 Jul 2010 17:28:49 +0200
From:	Michal Januszewski <michalj@...il.com>
To:	Andreas Schwab <schwab@...hat.com>
Cc:	linux-kernel@...r.kernel.org
Subject: [PATCH] Make sure abs() works as expected with both ints and longs

The abs() macro is commonly used to calculate the modulus of a difference
of two numbers.  With the introduction of "long" inside the definition
of abs() in the commit a49c59c042c63b432307c1bbf7dac5a104c786e6, some of
the abs() calls in the kernel started returning unexpected values (for
instance, see abs() usage in drivers/video/modedb.c).

The problem is apparent if the argument of abs() is a difference of two
32-bit integers, at least one of which is unsigned.  The result is then
assumed to be an unsigned integer, which gets cast to a positive long. The
return value of abs() is then this large positive integer, instead of the
expected small positive integer representing the modulus of the argument.

Example:

  u32 a = 0, b = 1;
  u32 c = abs(a - b);

  'c' will end up with a value of 0xffffffff instead of the expected 0x1.

To fix this problem, modify the abs() macro so that it detects the size
of the argument, and if it's not larger than 32 bits, uses an int instead
of a long.

Signed-off-by: Michal Januszewski <michalj@...il.com>
---
OK, please disregard my previous patch.  How about this new one?  
A solution with sizeof() was mentioned as a possibility in the original 
discussion of a49c59c0, but it looks like it was never actually
presented and considered.

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8317ec4..bacefff 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -157,10 +157,14 @@ extern int _cond_resched(void);

 #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)

-#define abs(x) ({				\
-		long __x = (x);			\
-		(__x < 0) ? -__x : __x;		\
-	})
+#define abs(x) (			\
+	(sizeof(x) <= 4) ? ({		\
+		int __x = (x);		\
+		(__x < 0) ? -__x : __x;	\
+	}) : ({				\
+		long __x = (x);		\
+		(__x < 0) ? -__x : __x;	\
+	}))

 #ifdef CONFIG_PROVE_LOCKING
 void might_fault(void);

--
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/