* [PATCH AUTOSEL 5.15 16/41] hamradio: 6pack: fix array-index-out-of-bounds in decode_std_command()
[not found] <20220628022100.595243-1-sashal@kernel.org>
@ 2022-06-28 2:20 ` Sasha Levin
2022-06-28 2:20 ` [PATCH AUTOSEL 5.15 17/41] drivers/net/ethernet/neterion/vxge: Fix a use-after-free bug in vxge-main.c Sasha Levin
2022-06-28 2:20 ` [PATCH AUTOSEL 5.15 37/41] hinic: Replace memcpy() with direct assignment Sasha Levin
2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2022-06-28 2:20 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 36a9fbb70402..ac2e79f0a928 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 */
@@ -565,6 +566,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);
@@ -913,6 +915,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++)
@@ -930,6 +933,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");
@@ -959,8 +963,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] 3+ messages in thread* [PATCH AUTOSEL 5.15 17/41] drivers/net/ethernet/neterion/vxge: Fix a use-after-free bug in vxge-main.c
[not found] <20220628022100.595243-1-sashal@kernel.org>
2022-06-28 2:20 ` [PATCH AUTOSEL 5.15 16/41] hamradio: 6pack: fix array-index-out-of-bounds in decode_std_command() Sasha Levin
@ 2022-06-28 2:20 ` Sasha Levin
2022-06-28 2:20 ` [PATCH AUTOSEL 5.15 37/41] hinic: Replace memcpy() with direct assignment Sasha Levin
2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2022-06-28 2:20 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Wentao_Liang, David S . Miller, Sasha Levin, jdmason, edumazet,
kuba, pabeni, arnd, jgg, paskripkin, liuhangbin, chi.minghao,
christophe.jaillet, 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 df4a3f3da83a..6bbda3676c1a 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
@@ -4760,10 +4760,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] 3+ messages in thread* [PATCH AUTOSEL 5.15 37/41] hinic: Replace memcpy() with direct assignment
[not found] <20220628022100.595243-1-sashal@kernel.org>
2022-06-28 2:20 ` [PATCH AUTOSEL 5.15 16/41] hamradio: 6pack: fix array-index-out-of-bounds in decode_std_command() Sasha Levin
2022-06-28 2:20 ` [PATCH AUTOSEL 5.15 17/41] drivers/net/ethernet/neterion/vxge: Fix a use-after-free bug in vxge-main.c Sasha Levin
@ 2022-06-28 2:20 ` Sasha Levin
2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2022-06-28 2:20 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Kees Cook, Nick Desaulniers, Tom Rix, llvm, Gustavo A . R . Silva,
Nathan Chancellor, Jakub Kicinski, Sasha Levin, davem, edumazet,
pabeni, leon, jiri, olteanv, simon.horman, netdev
From: Kees Cook <keescook@chromium.org>
[ Upstream commit 1e70212e031528918066a631c9fdccda93a1ffaa ]
Under CONFIG_FORTIFY_SOURCE=y and CONFIG_UBSAN_BOUNDS=y, Clang is bugged
here for calculating the size of the destination buffer (0x10 instead of
0x14). This copy is a fixed size (sizeof(struct fw_section_info_st)), with
the source and dest being struct fw_section_info_st, so the memcpy should
be safe, assuming the index is within bounds, which is UBSAN_BOUNDS's
responsibility to figure out.
Avoid the whole thing and just do a direct assignment. This results in
no change to the executable code.
[This is a duplicate of commit 2c0ab32b73cf ("hinic: Replace memcpy()
with direct assignment") which was applied to net-next.]
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Tom Rix <trix@redhat.com>
Cc: llvm@lists.linux.dev
Link: https://github.com/ClangBuiltLinux/linux/issues/1592
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org> # build
Link: https://lore.kernel.org/r/20220616052312.292861-1-keescook@chromium.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/huawei/hinic/hinic_devlink.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
index 6e11ee339f12..92d4e0039565 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
@@ -43,9 +43,7 @@ static bool check_image_valid(struct hinic_devlink_priv *priv, const u8 *buf,
for (i = 0; i < fw_image->fw_info.fw_section_cnt; i++) {
len += fw_image->fw_section_info[i].fw_section_len;
- memcpy(&host_image->image_section_info[i],
- &fw_image->fw_section_info[i],
- sizeof(struct fw_section_info_st));
+ host_image->image_section_info[i] = fw_image->fw_section_info[i];
}
if (len != fw_image->fw_len ||
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread