From: Roland Dreier <rdreier@cisco.com>
To: Andrew Morton <akpm@osdl.org>
Cc: David Miller <davem@davemloft.net>,
linux-kernel@vger.kernel.org, openib-general@openib.org,
tom@opengridcomputing.com
Subject: Re: [PATCH] Avoid truncating to 'long' in ALIGN() macro
Date: Sun, 26 Nov 2006 11:10:23 -0800 [thread overview]
Message-ID: <ada64d23ty8.fsf@cisco.com> (raw)
In-Reply-To: <20061125164118.de53d1cf.akpm@osdl.org> (Andrew Morton's message of "Sat, 25 Nov 2006 16:41:18 -0800")
Commit 4c8bd7ee ("Do not truncate to 'int' in ALIGN() macro.") was
merged to fix the case of code like the following:
unsigned long addr;
unsigned int alignment;
addr = ALIGN(addr, alignment);
The original ALIGN macro calculated a mask as ~(alignment - 1), and
when alignment is just an int, this creates an int mask. If alignment
is also unsigned, then this mask will not be sign extended when
promoted to a long, which leads to the code above chopping off the top
half of addr when long is 64 bits.
However, the changed ALIGN macro, which computes the mask as
~(alignment - 1UL) actually breaks code like the following when long
is 32 bits:
u64 addr;
int alignment;
addr = ALIGN(addr, alignment);
The reason this breaks is pretty much the same as the original bug
that the change was supposed to fix: ~(alignment - 1UL) creates a mask
that is an unsigned long, which is not sign extended when promoted to
u64 (if long is 32 bits).
As suggested by Dave Miller and Al Viro, I fixed this by having the
ALIGN macro make sure the alignment is promoted to the same type as
the value being aligned before doing the negation.
This second construct is actually used in the amso1100 driver, so that
driver does not work on 32-bit architectures right now. Unfortunately
almost everyone using it runs 64-bit kernels, so this regression was
not noticed until now.
Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
Patch updated as suggested by Dave Miller and Al Viro...
can we merge this for 2.6.20?
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 24b6111..80955b3 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -31,7 +31,7 @@ #define ULLONG_MAX (~0ULL)
#define STACK_MAGIC 0xdeadbeef
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-#define ALIGN(x,a) (((x)+(a)-1UL)&~((a)-1UL))
+#define ALIGN(x,a) ((typeof(x)) (((x) + (a) - 1) & ~((typeof(x)) (a) - 1)))
#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
#define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
next prev parent reply other threads:[~2006-11-26 19:10 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-11-25 5:40 [PATCH] Avoid truncating to 'long' in ALIGN() macro Roland Dreier
2006-11-25 6:07 ` David Miller
2006-11-25 22:56 ` Roland Dreier
2006-11-25 23:05 ` David Miller
2006-11-25 23:09 ` Roland Dreier
2006-11-26 0:41 ` Andrew Morton
2006-11-26 19:09 ` Roland Dreier
2006-11-26 19:10 ` Roland Dreier [this message]
2006-11-26 19:17 ` Andrew Morton
2006-11-26 20:06 ` Roland Dreier
2006-11-26 20:11 ` Linus Torvalds
2006-11-26 20:26 ` Roland Dreier
2006-11-26 21:06 ` Jörn Engel
2006-11-26 22:20 ` Linus Torvalds
2006-11-27 4:41 ` Al Viro
2006-11-26 1:10 ` Al Viro
2006-11-26 1:17 ` Roland Dreier
2006-11-26 1:25 ` Al Viro
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=ada64d23ty8.fsf@cisco.com \
--to=rdreier@cisco.com \
--cc=akpm@osdl.org \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=openib-general@openib.org \
--cc=tom@opengridcomputing.com \
/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