Hi Linus,
please pull a fix for a long-standing, hard-to-catch issue in the EDAC
weird struct allocation code, for 5.17.
Thx.
---
The following changes since commit 754e0b0e35608ed5206d6a67a791563c631cec07:
Linux 5.17-rc4 (2022-02-13 12:13:30 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git tags/edac_urgent_for_v5.17_rc5
for you to fetch changes up to f8efca92ae509c25e0a4bd5d0a86decea4f0c41e:
EDAC: Fix calculation of returned address and next offset in edac_align_ptr() (2022-02-15 15:54:46 +0100)
----------------------------------------------------------------
- Fix a long-standing struct alignment bug in the EDAC struct allocation code
----------------------------------------------------------------
Eliav Farber (1):
EDAC: Fix calculation of returned address and next offset in edac_align_ptr()
drivers/edac/edac_mc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg
On Sun, Feb 20, 2022 at 3:50 AM Borislav Petkov <[email protected]> wrote:
>
> please pull a fix for a long-standing, hard-to-catch issue in the EDAC
> weird struct allocation code, for 5.17.
Heh. Yeah, that was subtly buggy.
Side note: the comment says that it will align to at least as much as
the compiler would do, but then it does something different, eg
if (size > sizeof(long))
align = sizeof(long long);
...
and it might make sense to use "__alignof__()" instead of "sizeof()"
just to make the code match the comment.
For example, on 32-bit x86, "sizeof(long long)" is 8, but
"__alignof__(long long)" is only 4.
And then we have m68k..
Or maybe the comment should be fixed instead, and talk about "natural
alignment" rather than "compiler alignment".
Linus
The pull request you sent on Sun, 20 Feb 2022 12:50:15 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git tags/edac_urgent_for_v5.17_rc5
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/6e8e752f705c2713005a3182c8444ef7b54f10aa
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
On Sun, Feb 20, 2022 at 12:12:41PM -0800, Linus Torvalds wrote:
> Or maybe the comment should be fixed instead, and talk about "natural
> alignment" rather than "compiler alignment".
Yah, where do I start... so, about this, I think I can simplify it by
simply unconditionally aligning to 8. My gut feeling is telling me
8-bytes alignment should simply work on everything. Because if it does,
all that crap becomes a lot simpler. But maybe I'm being too simplistic
here and there might be a corner-case where 8-bytes alignment just
doesn't work...
Then, that edac_align_ptr() thing is an abomination. It probably has
made sense at some point to allocate the whole structure, including the
embedded pointers in one go but I can't recall of ever seeing something
like that done somewhere else around the kernel. But maybe you'll know
of another example and why that would have made sense in the past.
If not, I'm thinking of gradually converting all drivers to do normal
structs allocation like the rest of the tree does and then getting rid
of that thing.
And I keep hoping someone else would volunteer but no one has so far...
Thx.
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg
On Sun, Feb 20, 2022 at 12:25 PM Borislav Petkov <[email protected]> wrote:
>
> Yah, where do I start... so, about this, I think I can simplify it by
> simply unconditionally aligning to 8.
Sounds good.
Then you could just do something like
void *ptr = (void *)ALIGN_UP((unsigned long)*p, 8);
*p = ptr + size*n_memb;
return ptr;
and that would be a lot simpler.
> My gut feeling is telling me
> 8-bytes alignment should simply work on everything. Because if it does,
> all that crap becomes a lot simpler. But maybe I'm being too simplistic
> here and there might be a corner-case where 8-bytes alignment just
> doesn't work...
Well, if 8-byte alignment doesn't work, then the existing code (with
the fix) doesn't work either, so..
Linus