The SJA1105 family has 45 L2 policing table entries
(SJA1105_MAX_L2_POLICING_COUNT) and SJA1110 has 110
(SJA1110_MAX_L2_POLICING_COUNT). Keeping the table structure but
accounting for the difference in port count (5 in SJA1105 vs 10 in
SJA1110) does not fully explain the difference. Rather, the SJA1110 also
has L2 ingress policers for multicast traffic. If a packet is classified
as multicast, it will be processed by the policer index 99 + SRCPORT.
The sja1105_init_l2_policing() function initializes all L2 policers such
that they don't interfere with normal packet reception by default. To have
a common code between SJA1105 and SJA1110, the index of the multicast
policer for the port is calculated because it's an index that is out of
bounds for SJA1105 but in bounds for SJA1110, and a bounds check is
performed.
The code fails to do the proper thing when determining what to do with the
multicast policer of port 0 on SJA1105 (ds->num_ports = 5). The "mcast"
index will be equal to 45, which is also equal to
table->ops->max_entry_count (SJA1105_MAX_L2_POLICING_COUNT). So it passes
through the check. But at the same time, SJA1105 doesn't have multicast
policers. So the code programs the SHARINDX field of an out-of-bounds
element in the L2 Policing table of the static config.
The comparison between index 45 and 45 entries should have determined the
code to not access this policer index on SJA1105, since its memory wasn't
even allocated.
With enough bad luck, the out-of-bounds write could even overwrite other
valid kernel data, but in this case, the issue was detected using KASAN.
Kernel log:
sja1105 spi5.0: Probed switch chip: SJA1105Q
==================================================================
BUG: KASAN: slab-out-of-bounds in sja1105_setup+0x1cbc/0x2340
Write of size 8 at addr ffffff880bd57708 by task kworker/u8:0/8
...
Workqueue: events_unbound deferred_probe_work_func
Call trace:
...
sja1105_setup+0x1cbc/0x2340
dsa_register_switch+0x1284/0x18d0
sja1105_probe+0x748/0x840
...
Allocated by task 8:
...
sja1105_setup+0x1bcc/0x2340
dsa_register_switch+0x1284/0x18d0
sja1105_probe+0x748/0x840
...
Fixes: 38fbe91f2287 ("net: dsa: sja1105: configure the multicast policers, if present")
CC: [email protected] # 5.15+
Signed-off-by: Radu Nicolae Pirea (OSS) <[email protected]>
---
drivers/net/dsa/sja1105/sja1105_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Hi,
This is the v2 of "net: dsa: sja1105: fix slab-out-of-bounds in
sja1105_setup" and it is a resend because the first time it was send only
to stable [email protected].
Cheers.
Radu P.
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 412666111b0c..b70dcf32a26d 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1038,7 +1038,7 @@ static int sja1105_init_l2_policing(struct sja1105_private *priv)
policing[bcast].sharindx = port;
/* Only SJA1110 has multicast policers */
- if (mcast <= table->ops->max_entry_count)
+ if (mcast < table->ops->max_entry_count)
policing[mcast].sharindx = port;
}
--
2.34.1
On Wed, Dec 07, 2022 at 03:23:47PM +0200, Radu Nicolae Pirea (OSS) wrote:
> The SJA1105 family has 45 L2 policing table entries
> (SJA1105_MAX_L2_POLICING_COUNT) and SJA1110 has 110
> (SJA1110_MAX_L2_POLICING_COUNT). Keeping the table structure but
> accounting for the difference in port count (5 in SJA1105 vs 10 in
> SJA1110) does not fully explain the difference. Rather, the SJA1110 also
> has L2 ingress policers for multicast traffic. If a packet is classified
> as multicast, it will be processed by the policer index 99 + SRCPORT.
>
> The sja1105_init_l2_policing() function initializes all L2 policers such
> that they don't interfere with normal packet reception by default. To have
> a common code between SJA1105 and SJA1110, the index of the multicast
> policer for the port is calculated because it's an index that is out of
> bounds for SJA1105 but in bounds for SJA1110, and a bounds check is
> performed.
>
> The code fails to do the proper thing when determining what to do with the
> multicast policer of port 0 on SJA1105 (ds->num_ports = 5). The "mcast"
> index will be equal to 45, which is also equal to
> table->ops->max_entry_count (SJA1105_MAX_L2_POLICING_COUNT). So it passes
> through the check. But at the same time, SJA1105 doesn't have multicast
> policers. So the code programs the SHARINDX field of an out-of-bounds
> element in the L2 Policing table of the static config.
>
> The comparison between index 45 and 45 entries should have determined the
> code to not access this policer index on SJA1105, since its memory wasn't
> even allocated.
>
> With enough bad luck, the out-of-bounds write could even overwrite other
> valid kernel data, but in this case, the issue was detected using KASAN.
>
> Kernel log:
>
> sja1105 spi5.0: Probed switch chip: SJA1105Q
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in sja1105_setup+0x1cbc/0x2340
> Write of size 8 at addr ffffff880bd57708 by task kworker/u8:0/8
> ...
> Workqueue: events_unbound deferred_probe_work_func
> Call trace:
> ...
> sja1105_setup+0x1cbc/0x2340
> dsa_register_switch+0x1284/0x18d0
> sja1105_probe+0x748/0x840
> ...
> Allocated by task 8:
> ...
> sja1105_setup+0x1bcc/0x2340
> dsa_register_switch+0x1284/0x18d0
> sja1105_probe+0x748/0x840
> ...
>
> Fixes: 38fbe91f2287 ("net: dsa: sja1105: configure the multicast policers, if present")
> CC: [email protected] # 5.15+
> Signed-off-by: Radu Nicolae Pirea (OSS) <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:
On Wed, 7 Dec 2022 15:23:47 +0200 you wrote:
> The SJA1105 family has 45 L2 policing table entries
> (SJA1105_MAX_L2_POLICING_COUNT) and SJA1110 has 110
> (SJA1110_MAX_L2_POLICING_COUNT). Keeping the table structure but
> accounting for the difference in port count (5 in SJA1105 vs 10 in
> SJA1110) does not fully explain the difference. Rather, the SJA1110 also
> has L2 ingress policers for multicast traffic. If a packet is classified
> as multicast, it will be processed by the policer index 99 + SRCPORT.
>
> [...]
Here is the summary with links:
- [RESEND,v2] net: dsa: sja1105: avoid out of bounds access in sja1105_init_l2_policing()
https://git.kernel.org/netdev/net/c/f8bac7f9fdb0
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html