| 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
| ||
|
Date: Mon, 9 Aug 2021 14:25:35 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Mete Polat <metepolat2000@...il.com>
Cc: Michel Lespinasse <michel@...pinasse.org>,
Davidlohr Bueso <dbueso@...e.de>,
Andrew Morton <akpm@...ux-foundation.org>,
Lukas Bulwahn <lukas.bulwahn@...il.com>,
Peter Zijlstra <peterz@...radead.org>,
Jesper Nilsson <jesper@....nu>, Arnd Bergmann <arnd@...db.de>,
David Woodhouse <dwmw2@...radead.org>,
Ingo Molnar <mingo@...nel.org>,
Randy Dunlap <rdunlap@...radead.org>,
kernel-janitors@...r.kernel.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-m68k <linux-m68k@...ts.linux-m68k.org>
Subject: Re: [PATCH v2] rbtree: remove unneeded explicit alignment in struct rb_node
Hi Mete,
On Fri, Aug 6, 2021 at 5:22 PM Mete Polat <metepolat2000@...il.com> wrote:
> Commit e977145aeaad ("[RBTREE] Add explicit alignment to sizeof(long) for
> struct rb_node.") adds an explicit alignment to the struct rb_node due to
> some speciality of the CRIS architecture.
>
> The support for the CRIS architecture was removed with commit c690eddc2f3b
> ("CRIS: Drop support for the CRIS port")
>
> So, remove this now unneeded explicit alignment in struct rb_node as well.
>
> This basically reverts commit e977145aeaad ("[RBTREE] Add explicit
> alignment to sizeof(long) for struct rb_node.").
>
> The rbtree node color is stored in the LSB of '__rb_parent_color'.
> Only mask the first bit in '__rb_parent()', otherwise it modifies the
> node's parent address on m68k.
>
> Reported-by: Randy Dunlap <rdunlap@...radead.org>
> Reported-by: Mete Polat <metepolat2000@...il.com>
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@...il.com>
> Signed-off-by: Mete Polat <metepolat2000@...il.com>
> ---
> I have tested it on x86, but not on m68k. Can you ack that Geert?
Unfortunately not. Just booting on ARAnyM gives:
Unable to handle kernel access at virtual address 84abc227
Oops: 00000000
Modules linked in:
PC: [<b45b0005>] 0xb45b0005
SR: 2700 SP: fc24ede1 a2: 01322b40
d0: b45aff7c d1: 00002708 d2: 00000000 d3: 0033a6a4
d4: 00000005 d5: 95847357 a0: b45b0005 a1: 000f4240
Process sh (pid: 402, task=672891a8)
Frame format=7 eff addr=b45b0005 ssw=0546 faddr=b45b0005
wb 1 stat/addr/data: 0000 00000000 00000000
wb 2 stat/addr/data: 0000 00000000 00000000
wb 3 stat/addr/data: 0000 b45b0005 00000000
push data: 00000000 00000000 00000000 00000000
Stack from 0131debc:
0004ddf4 0122e0a4 0122e0a4 00477c26 00000000 00000000 00000005 95847357
00002708 0004558e 0033ab30 00000002 01003180 0047273c 0131df88 0033a6a4
00477c3a 00477c24 0004dfee 00000005 95847357 00002708 0000000f 00000001
9915a000 0000000d 0004d430 01322b40 00000001 00002604 00000000 00006e50
00000001 00000000 0004560c 0000000d 00000000 00000000 efc3aa78 efc3aa78
c0158c0c 8009ff68 0047273c efc3aa64 c01537a8 0131df8c 000456b6 0047273c
Call Trace: [<0004ddf4>] __hrtimer_run_queues.constprop.0+0xba/0x144
[<00002708>] calibrate_delay+0xd6/0x18a
[<0004558e>] __irq_wake_thread+0x0/0x40
[<0033ab30>] printk+0x0/0x18
[<0033a6a4>] warn_slowpath_fmt+0x0/0x62
[<0004dfee>] hrtimer_run_queues+0xb4/0xc0
[<00002708>] calibrate_delay+0xd6/0x18a
[<0004d430>] update_process_times+0x38/0x8c
[<00002604>] wait_for_initramfs+0x30/0x58
[<00006e50>] mfp_timer_c_handler+0x2c/0x3c
[<0004560c>] __handle_irq_event_percpu+0x3e/0xcc
[<000456b6>] handle_irq_event_percpu+0x1c/0x52
[<0004570a>] handle_irq_event+0x1e/0x30
[<00047aa2>] handle_simple_irq+0x4a/0x4e
[<00045062>] handle_irq_desc+0x1e/0x28
[<00002cac>] do_IRQ+0x20/0x32
[<00002bcc>] user_irqvec_fixup+0xc/0x14
[<0010c00b>] mb_cache_create+0x47/0xdc
Code: dde0 dde0 dde0 dde0 dde0 dde0 dde0 dde0 Bad PC value.
Disabling lock debugging due to kernel taint
Kernel panic - not syncing: Aiee, killing interrupt handler!
---[ end Kernel panic - not syncing: Aiee, killing interrupt handler! ]---
> --- a/include/linux/rbtree.h
> +++ b/include/linux/rbtree.h
> @@ -25,8 +25,7 @@ struct rb_node {
> unsigned long __rb_parent_color;
> struct rb_node *rb_right;
> struct rb_node *rb_left;
> -} __attribute__((aligned(sizeof(long))));
> - /* The alignment might seem pointless, but allegedly CRIS needs it */
> +};
>
> struct rb_root {
> struct rb_node *rb_node;
> diff --git a/include/linux/rbtree_augmented.h b/include/linux/rbtree_augmented.h
> index d1c53e9d8c75..94b6a0f4499e 100644
> --- a/include/linux/rbtree_augmented.h
> +++ b/include/linux/rbtree_augmented.h
> @@ -145,7 +145,7 @@ RB_DECLARE_CALLBACKS(RBSTATIC, RBNAME, \
> #define RB_RED 0
> #define RB_BLACK 1
>
> -#define __rb_parent(pc) ((struct rb_node *)(pc & ~3))
> +#define __rb_parent(pc) ((struct rb_node *)(pc & ~1))
Are you sure you can do this, i.e. that nothing else relies on the
alignment to (at least) 4 bytes?
There is another definition of rb_parent(). If I make a similar change there:
--- a/include/linux/rbtree.h
+++ b/include/linux/rbtree.h
@@ -31,7 +31,7 @@ struct rb_root {
struct rb_node *rb_node;
};
-#define rb_parent(r) ((struct rb_node *)((r)->__rb_parent_color & ~3))
+#define rb_parent(r) ((struct rb_node *)((r)->__rb_parent_color & ~1))
#define RB_ROOT (struct rb_root) { NULL, }
#define rb_entry(ptr, type, member) container_of(ptr, type, member)
the system boots on ARAnyM, but I prefer to be a bit cautious...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Powered by blists - more mailing lists