netdev - Re: [PATCH RESEND v2] net: dsa: sja1105: avoid out of bounds access in sja1105_init_l2

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221208172232.fldnlue35km76ldt@skbuf>
Date:   Thu, 8 Dec 2022 19:22:32 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     "Radu Nicolae Pirea (OSS)" <radu-nicolae.pirea@....nxp.com>
Cc:     andrew@...n.ch, f.fainelli@...il.com, davem@...emloft.net,
        gregkh@...uxfoundation.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH RESEND v2] net: dsa: sja1105: avoid out of bounds access
 in sja1105_init_l2_policing()

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: stable@...r.kernel.org # 5.15+
> Signed-off-by: Radu Nicolae Pirea (OSS) <radu-nicolae.pirea@....nxp.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@...il.com>