2023-05-31 10:49:39

by Qingfang Deng

[permalink] [raw]
Subject: [PATCH net] neighbour: fix unaligned access to pneigh_entry

From: Qingfang DENG <[email protected]>

After the blamed commit, the member key is longer 4-byte aligned. On
platforms that do not support unaligned access, e.g., MIPS32R2 with
unaligned_action set to 2, this will trigger a crash when accessing
an IPv6 pneigh_entry, as the key is cast to an in6_addr pointer.

Align the member to 4 bytes on said platforms.

Fixes: 62dd93181aaa ("[IPV6] NDISC: Set per-entry is_router flag in Proxy NA.")
Signed-off-by: Qingfang DENG <[email protected]>
---
include/net/neighbour.h | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 3fa5774bddac..c0195ed9d111 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -180,7 +180,11 @@ struct pneigh_entry {
netdevice_tracker dev_tracker;
u32 flags;
u8 protocol;
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
u8 key[];
+#else
+ u8 key[] __aligned(4);
+#endif
};

/*
--
2.34.1



2023-05-31 18:52:44

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net] neighbour: fix unaligned access to pneigh_entry

On Wed, 31 May 2023 18:42:33 +0800 Qingfang DENG wrote:
> +#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> u8 key[];
> +#else
> + u8 key[] __aligned(4);
> +#endif

I'd appreciate a second opinion, but to me it's very unlikely we'd save
any memory even with efficient aligned access here. No reasonably key
will fit into 3 bytes, right? So we can as well avoid the ifdef and
make the key[] always aligned. Or preferably, if it doesn't cause
compilation issues, make the type of the key u32?

2023-05-31 18:53:36

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH net] neighbour: fix unaligned access to pneigh_entry

On Wed, May 31, 2023 at 8:16 PM Jakub Kicinski <[email protected]> wrote:
>
> On Wed, 31 May 2023 18:42:33 +0800 Qingfang DENG wrote:
> > +#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> > u8 key[];
> > +#else
> > + u8 key[] __aligned(4);
> > +#endif
>
> I'd appreciate a second opinion, but to me it's very unlikely we'd save
> any memory even with efficient aligned access here. No reasonably key
> will fit into 3 bytes, right? So we can as well avoid the ifdef and
> make the key[] always aligned. Or preferably, if it doesn't cause
> compilation issues, make the type of the key u32?

Same feeling, we could avoid the CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ifdef.

2023-06-01 02:02:45

by Qingfang Deng

[permalink] [raw]
Subject: [PATCH net v2] neighbour: fix unaligned access to pneigh_entry

From: Qingfang DENG <[email protected]>

After the blamed commit, the member key is longer 4-byte aligned. On
platforms that do not support unaligned access, e.g., MIPS32R2 with
unaligned_action set to 1, this will trigger a crash when accessing
an IPv6 pneigh_entry, as the key is cast to an in6_addr pointer.

Change the type of the key to u32 to make it aligned.

Fixes: 62dd93181aaa ("[IPV6] NDISC: Set per-entry is_router flag in Proxy NA.")
Signed-off-by: Qingfang DENG <[email protected]>
---
v2: remove the ifdef, and use u32 type

include/net/neighbour.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 3fa5774bddac..f6a8ecc6b1fa 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -180,7 +180,7 @@ struct pneigh_entry {
netdevice_tracker dev_tracker;
u32 flags;
u8 protocol;
- u8 key[];
+ u32 key[];
};

/*
--
2.34.1


2023-06-02 05:05:47

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net v2] neighbour: fix unaligned access to pneigh_entry

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

On Thu, 1 Jun 2023 09:54:32 +0800 you wrote:
> From: Qingfang DENG <[email protected]>
>
> After the blamed commit, the member key is longer 4-byte aligned. On
> platforms that do not support unaligned access, e.g., MIPS32R2 with
> unaligned_action set to 1, this will trigger a crash when accessing
> an IPv6 pneigh_entry, as the key is cast to an in6_addr pointer.
>
> [...]

Here is the summary with links:
- [net,v2] neighbour: fix unaligned access to pneigh_entry
https://git.kernel.org/netdev/net/c/ed779fe4c9b5

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html