* [PATCH AUTOSEL 5.4 08/27] hamradio: 6pack: fix array-index-out-of-bounds in decode_std_command()
[not found] <20220628022413.596341-1-sashal@kernel.org>
@ 2022-06-28 2:23 ` Sasha Levin
2022-06-28 2:23 ` [PATCH AUTOSEL 5.4 09/27] drivers/net/ethernet/neterion/vxge: Fix a use-after-free bug in vxge-main.c Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2022-06-28 2:23 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Xu Jia, Hulk Robot, David S . Miller, Sasha Levin, ajk, edumazet,
kuba, pabeni, linux-hams, netdev
From: Xu Jia <xujia39@huawei.com>
[ Upstream commit 2b04495e21cdb9b45c28c6aeb2da560184de20a3 ]
Hulk Robot reports incorrect sp->rx_count_cooked value in decode_std_command().
This should be caused by the subtracting from sp->rx_count_cooked before.
It seems that sp->rx_count_cooked value is changed to 0, which bypassed the
previous judgment.
The situation is shown below:
(Thread 1) | (Thread 2)
decode_std_command() | resync_tnc()
... |
if (rest == 2) |
sp->rx_count_cooked -= 2; |
else if (rest == 3) | ...
| sp->rx_count_cooked = 0;
sp->rx_count_cooked -= 1; |
for (i = 0; i < sp->rx_count_cooked; i++) // report error
checksum += sp->cooked_buf[i];
sp->rx_count_cooked is a shared variable but is not protected by a lock.
The same applies to sp->rx_count. This patch adds a lock to fix the bug.
The fail log is shown below:
=======================================================================
UBSAN: array-index-out-of-bounds in drivers/net/hamradio/6pack.c:925:31
index 400 is out of range for type 'unsigned char [400]'
CPU: 3 PID: 7433 Comm: kworker/u10:1 Not tainted 5.18.0-rc5-00163-g4b97bac0756a #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
Workqueue: events_unbound flush_to_ldisc
Call Trace:
<TASK>
dump_stack_lvl+0xcd/0x134
ubsan_epilogue+0xb/0x50
__ubsan_handle_out_of_bounds.cold+0x62/0x6c
sixpack_receive_buf+0xfda/0x1330
tty_ldisc_receive_buf+0x13e/0x180
tty_port_default_receive_buf+0x6d/0xa0
flush_to_ldisc+0x213/0x3f0
process_one_work+0x98f/0x1620
worker_thread+0x665/0x1080
kthread+0x2e9/0x3a0
ret_from_fork+0x1f/0x30
...
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Xu Jia <xujia39@huawei.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/hamradio/6pack.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 83dc1c2c3b84..d92df9bafbbd 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -99,6 +99,7 @@ struct sixpack {
unsigned int rx_count;
unsigned int rx_count_cooked;
+ spinlock_t rxlock;
int mtu; /* Our mtu (to spot changes!) */
int buffsize; /* Max buffers sizes */
@@ -570,6 +571,7 @@ static int sixpack_open(struct tty_struct *tty)
sp->dev = dev;
spin_lock_init(&sp->lock);
+ spin_lock_init(&sp->rxlock);
refcount_set(&sp->refcnt, 1);
init_completion(&sp->dead);
@@ -925,6 +927,7 @@ static void decode_std_command(struct sixpack *sp, unsigned char cmd)
sp->led_state = 0x60;
/* fill trailing bytes with zeroes */
sp->tty->ops->write(sp->tty, &sp->led_state, 1);
+ spin_lock_bh(&sp->rxlock);
rest = sp->rx_count;
if (rest != 0)
for (i = rest; i <= 3; i++)
@@ -942,6 +945,7 @@ static void decode_std_command(struct sixpack *sp, unsigned char cmd)
sp_bump(sp, 0);
}
sp->rx_count_cooked = 0;
+ spin_unlock_bh(&sp->rxlock);
}
break;
case SIXP_TX_URUN: printk(KERN_DEBUG "6pack: TX underrun\n");
@@ -971,8 +975,11 @@ sixpack_decode(struct sixpack *sp, const unsigned char *pre_rbuff, int count)
decode_prio_command(sp, inbyte);
else if ((inbyte & SIXP_STD_CMD_MASK) != 0)
decode_std_command(sp, inbyte);
- else if ((sp->status & SIXP_RX_DCD_MASK) == SIXP_RX_DCD_MASK)
+ else if ((sp->status & SIXP_RX_DCD_MASK) == SIXP_RX_DCD_MASK) {
+ spin_lock_bh(&sp->rxlock);
decode_data(sp, inbyte);
+ spin_unlock_bh(&sp->rxlock);
+ }
}
}
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* [PATCH AUTOSEL 5.4 09/27] drivers/net/ethernet/neterion/vxge: Fix a use-after-free bug in vxge-main.c
[not found] <20220628022413.596341-1-sashal@kernel.org>
2022-06-28 2:23 ` [PATCH AUTOSEL 5.4 08/27] hamradio: 6pack: fix array-index-out-of-bounds in decode_std_command() Sasha Levin
@ 2022-06-28 2:23 ` Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2022-06-28 2:23 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Wentao_Liang, David S . Miller, Sasha Levin, jdmason, edumazet,
kuba, pabeni, liuhangbin, arnd, christophe.jaillet, chi.minghao,
paskripkin, netdev
From: Wentao_Liang <Wentao_Liang_g@163.com>
[ Upstream commit 8fc74d18639a2402ca52b177e990428e26ea881f ]
The pointer vdev points to a memory region adjacent to a net_device
structure ndev, which is a field of hldev. At line 4740, the invocation
to vxge_device_unregister unregisters device hldev, and it also releases
the memory region pointed by vdev->bar0. At line 4743, the freed memory
region is referenced (i.e., iounmap(vdev->bar0)), resulting in a
use-after-free vulnerability. We can fix the bug by calling iounmap
before vxge_device_unregister.
4721. static void vxge_remove(struct pci_dev *pdev)
4722. {
4723. struct __vxge_hw_device *hldev;
4724. struct vxgedev *vdev;
…
4731. vdev = netdev_priv(hldev->ndev);
…
4740. vxge_device_unregister(hldev);
4741. /* Do not call pci_disable_sriov here, as it
will break child devices */
4742. vxge_hw_device_terminate(hldev);
4743. iounmap(vdev->bar0);
…
4749 vxge_debug_init(vdev->level_trace, "%s:%d
Device unregistered",
4750 __func__, __LINE__);
4751 vxge_debug_entryexit(vdev->level_trace, "%s:%d
Exiting...", __func__,
4752 __LINE__);
4753. }
This is the screenshot when the vulnerability is triggered by using
KASAN. We can see that there is a use-after-free reported by KASAN.
/***************************start**************************/
root@kernel:~# echo 1 > /sys/bus/pci/devices/0000:00:03.0/remove
[ 178.296316] vxge_remove
[ 182.057081]
==================================================================
[ 182.057548] BUG: KASAN: use-after-free in vxge_remove+0xe0/0x15c
[ 182.057760] Read of size 8 at addr ffff888006c76598 by task bash/119
[ 182.057983]
[ 182.058747] CPU: 0 PID: 119 Comm: bash Not tainted 5.18.0 #5
[ 182.058919] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
[ 182.059463] Call Trace:
[ 182.059726] <TASK>
[ 182.060017] dump_stack_lvl+0x34/0x44
[ 182.060316] print_report.cold+0xb2/0x6b7
[ 182.060401] ? kfree+0x89/0x290
[ 182.060478] ? vxge_remove+0xe0/0x15c
[ 182.060545] kasan_report+0xa9/0x120
[ 182.060629] ? vxge_remove+0xe0/0x15c
[ 182.060706] vxge_remove+0xe0/0x15c
[ 182.060793] pci_device_remove+0x5d/0xe0
[ 182.060968] device_release_driver_internal+0xf1/0x180
[ 182.061063] pci_stop_bus_device+0xae/0xe0
[ 182.061150] pci_stop_and_remove_bus_device_locked+0x11/0x20
[ 182.061236] remove_store+0xc6/0xe0
[ 182.061297] ? subordinate_bus_number_show+0xc0/0xc0
[ 182.061359] ? __mutex_lock_slowpath+0x10/0x10
[ 182.061438] ? sysfs_kf_write+0x6d/0xa0
[ 182.061525] kernfs_fop_write_iter+0x1b0/0x260
[ 182.061610] ? sysfs_kf_bin_read+0xf0/0xf0
[ 182.061695] new_sync_write+0x209/0x310
[ 182.061789] ? new_sync_read+0x310/0x310
[ 182.061865] ? cgroup_rstat_updated+0x5c/0x170
[ 182.061937] ? preempt_count_sub+0xf/0xb0
[ 182.061995] ? pick_next_entity+0x13a/0x220
[ 182.062063] ? __inode_security_revalidate+0x44/0x80
[ 182.062155] ? security_file_permission+0x46/0x2a0
[ 182.062230] vfs_write+0x33f/0x3e0
[ 182.062303] ksys_write+0xb4/0x150
[ 182.062369] ? __ia32_sys_read+0x40/0x40
[ 182.062451] do_syscall_64+0x3b/0x90
[ 182.062531] entry_SYSCALL_64_after_hwframe+0x46/0xb0
[ 182.062894] RIP: 0033:0x7f3f37d17274
[ 182.063558] Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f
80 00 00 00 00 48 8d 05 89 54 0d 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f
05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 49 89 d4 55 48 89 f5 53
[ 182.063797] RSP: 002b:00007ffd5ba9e178 EFLAGS: 00000246
ORIG_RAX: 0000000000000001
[ 182.064117] RAX: ffffffffffffffda RBX: 0000000000000002
RCX: 00007f3f37d17274
[ 182.064219] RDX: 0000000000000002 RSI: 000055bbec327180
RDI: 0000000000000001
[ 182.064315] RBP: 000055bbec327180 R08: 000000000000000a
R09: 00007f3f37de7cf0
[ 182.064414] R10: 000000000000000a R11: 0000000000000246
R12: 00007f3f37de8760
[ 182.064513] R13: 0000000000000002 R14: 00007f3f37de3760
R15: 0000000000000002
[ 182.064691] </TASK>
[ 182.064916]
[ 182.065224] The buggy address belongs to the physical page:
[ 182.065804] page:00000000ef31e4f4 refcount:0 mapcount:0
mapping:0000000000000000 index:0x0 pfn:0x6c76
[ 182.067419] flags: 0x100000000000000(node=0|zone=1)
[ 182.068997] raw: 0100000000000000 0000000000000000
ffffea00001b1d88 0000000000000000
[ 182.069118] raw: 0000000000000000 0000000000000000
00000000ffffffff 0000000000000000
[ 182.069294] page dumped because: kasan: bad access detected
[ 182.069331]
[ 182.069360] Memory state around the buggy address:
[ 182.070006] ffff888006c76480: ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff
[ 182.070136] ffff888006c76500: ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff
[ 182.070230] >ffff888006c76580: ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff
[ 182.070305] ^
[ 182.070456] ffff888006c76600: ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff
[ 182.070505] ffff888006c76680: ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff
[ 182.070606]
==================================================================
[ 182.071374] Disabling lock debugging due to kernel taint
/*****************************end*****************************/
After fixing the bug as done in the patch, we can find KASAN do not report
the bug and the device(00:03.0) has been successfully removed.
/*****************************start***************************/
root@kernel:~# echo 1 > /sys/bus/pci/devices/0000:00:03.0/remove
root@kernel:~#
/******************************end****************************/
Signed-off-by: Wentao_Liang <Wentao_Liang_g@163.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/neterion/vxge/vxge-main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
index 607e2ff272dc..b734347ca0d7 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
@@ -4775,10 +4775,10 @@ static void vxge_remove(struct pci_dev *pdev)
for (i = 0; i < vdev->no_of_vpath; i++)
vxge_free_mac_add_list(&vdev->vpaths[i]);
+ iounmap(vdev->bar0);
vxge_device_unregister(hldev);
/* Do not call pci_disable_sriov here, as it will break child devices */
vxge_hw_device_terminate(hldev);
- iounmap(vdev->bar0);
pci_release_region(pdev, 0);
pci_disable_device(pdev);
driver_config->config_dev_cnt--;
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread