From: Radu Pirea radu-nicolae.pirea@nxp.com
Fix slab-out-of-bounds in sja1105_setup.
Kernel log:
[ 98.394543] sja1105 spi5.0: Probed switch chip: SJA1105Q [ 98.425782] ================================================================== [ 98.425819] BUG: KASAN: slab-out-of-bounds in sja1105_setup+0x1cbc/0x2340 [ 98.425880] Write of size 8 at addr ffffff880bd57708 by task kworker/u8:0/8
[ 98.425921] CPU: 0 PID: 8 Comm: kworker/u8:0 Tainted: G O 5.15.73-rt52-00001-g9f4226d49b44 #6 [ 98.425955] Hardware name: NXP S32G2XXX-EVB (DT) [ 98.425975] Workqueue: events_unbound deferred_probe_work_func [ 98.426039] Call trace: [ 98.426049] dump_backtrace+0x0/0x2b4 [ 98.426099] show_stack+0x18/0x24 [ 98.426140] dump_stack_lvl+0x68/0x84 [ 98.426179] print_address_description.constprop.0+0x78/0x29c [ 98.426221] kasan_report+0x1d4/0x240 [ 98.426261] __asan_store8+0x98/0xd0 [ 98.426299] sja1105_setup+0x1cbc/0x2340 [ 98.426331] dsa_register_switch+0x1284/0x18d0 [ 98.426381] sja1105_probe+0x748/0x840 [ 98.426411] spi_probe+0xb0/0x110 [ 98.426458] really_probe.part.0+0xf8/0x48c [ 98.426503] __driver_probe_device+0xd4/0x180 [ 98.426546] driver_probe_device+0xf8/0x1e0 [ 98.426588] __device_attach_driver+0xe8/0x1a0 [ 98.426631] bus_for_each_drv+0xf4/0x15c [ 98.426670] __device_attach+0x120/0x270 [ 98.426711] device_initial_probe+0x14/0x20 [ 98.426753] bus_probe_device+0xec/0x100 [ 98.426793] deferred_probe_work_func+0xe8/0x130 [ 98.426835] process_one_work+0x3cc/0x664 [ 98.426872] worker_thread+0xa0/0x72c [ 98.426904] kthread+0x21c/0x230 [ 98.426946] ret_from_fork+0x10/0x20
[ 98.426988] Allocated by task 8: [ 98.427004] kasan_save_stack+0x28/0x60 [ 98.427040] __kasan_kmalloc+0x8c/0xb0 [ 98.427072] __kmalloc+0xdc/0x1a0 [ 98.427100] kmalloc_array.constprop.0+0x20/0x34 [ 98.427131] sja1105_setup+0x1bcc/0x2340 [ 98.427160] dsa_register_switch+0x1284/0x18d0 [ 98.427203] sja1105_probe+0x748/0x840 [ 98.427232] spi_probe+0xb0/0x110 [ 98.427274] really_probe.part.0+0xf8/0x48c [ 98.427316] __driver_probe_device+0xd4/0x180 [ 98.427357] driver_probe_device+0xf8/0x1e0 [ 98.427398] __device_attach_driver+0xe8/0x1a0 [ 98.427441] bus_for_each_drv+0xf4/0x15c [ 98.427478] __device_attach+0x120/0x270 [ 98.427516] device_initial_probe+0x14/0x20 [ 98.427557] bus_probe_device+0xec/0x100 [ 98.427596] deferred_probe_work_func+0xe8/0x130 [ 98.427636] process_one_work+0x3cc/0x664 [ 98.427668] worker_thread+0xa0/0x72c [ 98.427698] kthread+0x21c/0x230 [ 98.427737] ret_from_fork+0x10/0x20
[ 98.427775] The buggy address belongs to the object at ffffff880bd57000 which belongs to the cache kmalloc-2k of size 2048 [ 98.427801] The buggy address is located 1800 bytes inside of 2048-byte region [ffffff880bd57000, ffffff880bd57800) [ 98.427833] The buggy address belongs to the page: [ 98.427848] page:0000000065dd1b0f refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x88bd57 [ 98.427881] flags: 0x8000000000000200(slab|zone=2) [ 98.427935] raw: 8000000000000200 fffffffe1c296ad8 fffffffe1c296b80 ffffff8800000400 [ 98.427966] raw: 0000000000000000 ffffff880bd57000 0000000100000001 [ 98.427982] page dumped because: kasan: bad access detected
[ 98.428003] Memory state around the buggy address: [ 98.428021] ffffff880bd57600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 98.428046] ffffff880bd57680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 98.428072] >ffffff880bd57700: 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 98.428088] ^ [ 98.428106] ffffff880bd57780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 98.428131] ffffff880bd57800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 98.428148] ==================================================================
Signed-off-by: Radu Pirea radu-nicolae.pirea@nxp.com --- drivers/net/dsa/sja1105/sja1105_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Can be applied on top of 5.15.81 stable branch.
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; }
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
Rule: 'Cc: stable@vger.kernel.org' or 'commit <sha1> upstream.' Subject: [PATCH] net: dsa: sja1105: fix slab-out-of-bounds in sja1105_setup Link: https://lore.kernel.org/stable/20221206151136.802344-1-radu-nicolae.pirea%40...
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
On Tue, Dec 06, 2022 at 05:11:36PM +0200, Radu Nicolae Pirea (OSS) wrote:
From: Radu Pirea radu-nicolae.pirea@nxp.com
Fix slab-out-of-bounds in sja1105_setup.
Signed-off-by: Radu Pirea radu-nicolae.pirea@nxp.com
drivers/net/dsa/sja1105/sja1105_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Can be applied on top of 5.15.81 stable branch.
Here the backporting process has to work differently.
Since the fixed code is also present in the master branch of the tree responsible with bug fixes for this code area (https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git), you have to base your patch on that tree, and send the patch to be applied by the netdev maintainers, not by stable directly. You do that by using git send-email --subject-prefix "PATCH v2 net". You also don't CC stable and Greg, just the networking people from ./scripts/get_maintainer.pl. After a while, the patch will land automatically in stable, through the "net" tree.
You must also add this above your Signed-off-by tag:
Fixes: 38fbe91f2287 ("net: dsa: sja1105: configure the multicast policers, if present")
On Tue, Dec 06, 2022 at 05:11:36PM +0200, Radu Nicolae Pirea (OSS) wrote:
From: Radu Pirea radu-nicolae.pirea@nxp.com
Fix slab-out-of-bounds in sja1105_setup.
Kernel log:
<snip>
This log doesn't say much, sorry. Please read the kernel documentation for how to write a good changelog text and how to submit a patch to the stable trees (hint, this isn't how...)
thanks,
greg k-h
On Tue, Dec 06, 2022 at 05:06:00PM +0100, Greg KH wrote:
On Tue, Dec 06, 2022 at 05:11:36PM +0200, Radu Nicolae Pirea (OSS) wrote:
From: Radu Pirea radu-nicolae.pirea@nxp.com
Fix slab-out-of-bounds in sja1105_setup.
Kernel log:
<snip>
This log doesn't say much, sorry. Please read the kernel documentation for how to write a good changelog text and how to submit a patch to the stable trees (hint, this isn't how...)
Agree with Greg.
The commit description should say that 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_setup() function initializes all L2 policers such that they don't interfere with normal packet reception by default. To have common code between SJA1105 and SJA1110, the index of the multicast policer for the port is calculated, and because it's an index that is out of bounds for SJA1105 but in bounds for SJA1110, 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.
Or something like that. The point is that you should use the commit description to prove to yourself (and also to readers) that the change is correct.
linux-stable-mirror@lists.linaro.org
-
Greg KH -
kernel test robot -
Radu Nicolae Pirea (OSS) -
Vladimir Oltean