In vmxnet3_rq_alloc_rx_buf(), when dma_map_single() fails, rbi->skb is
freed immediately. Similarly, in another branch, when dma_map_page() fails,
rbi->page is also freed. In the two cases, vmxnet3_rq_alloc_rx_buf()
returns an error to its callers vmxnet3_rq_init() -> vmxnet3_rq_init_all()
-> vmxnet3_activate_dev(). Then vmxnet3_activate_dev() calls
vmxnet3_rq_cleanup_all() in error handling code, and rbi->skb or rbi->page
are freed again in vmxnet3_rq_cleanup_all(), causing use-after-free bugs.
To fix these possible bugs, rbi->skb and rbi->page should not be freed in
mxnet3_rq_alloc_rx_buf() when dma_map_single() fails.
The error log in our fault-injection testing is shown as follows:
[ 14.319016] BUG: KASAN: use-after-free in consume_skb+0x2f/0x150
...
[ 14.321586] Call Trace:
...
[ 14.325357] consume_skb+0x2f/0x150
[ 14.325671] vmxnet3_rq_cleanup_all+0x33a/0x4e0 [vmxnet3]
[ 14.326150] vmxnet3_activate_dev+0xb9d/0x2ca0 [vmxnet3]
[ 14.326616] vmxnet3_open+0x387/0x470 [vmxnet3]
...
[ 14.361675] Allocated by task 351:
...
[ 14.362688] __netdev_alloc_skb+0x1b3/0x6f0
[ 14.362960] vmxnet3_rq_alloc_rx_buf+0x1b0/0x8d0 [vmxnet3]
[ 14.363317] vmxnet3_activate_dev+0x3e3/0x2ca0 [vmxnet3]
[ 14.363661] vmxnet3_open+0x387/0x470 [vmxnet3]
...
[ 14.367309]
[ 14.367412] Freed by task 351:
...
[ 14.368932] __dev_kfree_skb_any+0xd2/0xe0
[ 14.369193] vmxnet3_rq_alloc_rx_buf+0x71e/0x8d0 [vmxnet3]
[ 14.369544] vmxnet3_activate_dev+0x3e3/0x2ca0 [vmxnet3]
[ 14.369883] vmxnet3_open+0x387/0x470 [vmxnet3]
[ 14.370174] __dev_open+0x28a/0x420
[ 14.370399] __dev_change_flags+0x192/0x590
[ 14.370667] dev_change_flags+0x7a/0x180
[ 14.370919] do_setlink+0xb28/0x3570
[ 14.371150] rtnl_newlink+0x1160/0x1740
[ 14.371399] rtnetlink_rcv_msg+0x5bf/0xa50
[ 14.371661] netlink_rcv_skb+0x1cd/0x3e0
[ 14.371913] netlink_unicast+0x5dc/0x840
[ 14.372169] netlink_sendmsg+0x856/0xc40
[ 14.372420] ____sys_sendmsg+0x8a7/0x8d0
[ 14.372673] __sys_sendmsg+0x1c2/0x270
[ 14.372914] do_syscall_64+0x41/0x90
[ 14.373145] entry_SYSCALL_64_after_hwframe+0x44/0xae
...
Reported-by: TOTE Robot <[email protected]>
Signed-off-by: Zixuan Fu <[email protected]>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index d9d90baac72a..f17e9871ba27 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -588,7 +588,6 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
DMA_FROM_DEVICE);
if (dma_mapping_error(&adapter->pdev->dev,
rbi->dma_addr)) {
- dev_kfree_skb_any(rbi->skb);
rq->stats.rx_buf_alloc_failure++;
break;
}
@@ -612,7 +611,6 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
DMA_FROM_DEVICE);
if (dma_mapping_error(&adapter->pdev->dev,
rbi->dma_addr)) {
- put_page(rbi->page);
rq->stats.rx_buf_alloc_failure++;
break;
}
--
2.25.1
On Fri, 6 May 2022 20:31:18 +0800 Zixuan Fu wrote:
> In vmxnet3_rq_alloc_rx_buf(), when dma_map_single() fails, rbi->skb is
> freed immediately. Similarly, in another branch, when dma_map_page() fails,
> rbi->page is also freed. In the two cases, vmxnet3_rq_alloc_rx_buf()
> returns an error to its callers vmxnet3_rq_init() -> vmxnet3_rq_init_all()
> -> vmxnet3_activate_dev(). Then vmxnet3_activate_dev() calls
> vmxnet3_rq_cleanup_all() in error handling code, and rbi->skb or rbi->page
> are freed again in vmxnet3_rq_cleanup_all(), causing use-after-free bugs.
>
> To fix these possible bugs, rbi->skb and rbi->page should not be freed in
> mxnet3_rq_alloc_rx_buf() when dma_map_single() fails.
You should not leave unmapped data on the ring. Freeing is fine, just
make sure the pointer is cleared.
> The error log in our fault-injection testing is shown as follows:
>
> [ 14.319016] BUG: KASAN: use-after-free in consume_skb+0x2f/0x150
> ...
> [ 14.321586] Call Trace:
> ...
> [ 14.325357] consume_skb+0x2f/0x150
> [ 14.325671] vmxnet3_rq_cleanup_all+0x33a/0x4e0 [vmxnet3]
> [ 14.326150] vmxnet3_activate_dev+0xb9d/0x2ca0 [vmxnet3]
> [ 14.326616] vmxnet3_open+0x387/0x470 [vmxnet3]
> ...
> [ 14.361675] Allocated by task 351:
> ...
> [ 14.362688] __netdev_alloc_skb+0x1b3/0x6f0
> [ 14.362960] vmxnet3_rq_alloc_rx_buf+0x1b0/0x8d0 [vmxnet3]
> [ 14.363317] vmxnet3_activate_dev+0x3e3/0x2ca0 [vmxnet3]
> [ 14.363661] vmxnet3_open+0x387/0x470 [vmxnet3]
> ...
> [ 14.367309]
> [ 14.367412] Freed by task 351:
> ...
> [ 14.368932] __dev_kfree_skb_any+0xd2/0xe0
> [ 14.369193] vmxnet3_rq_alloc_rx_buf+0x71e/0x8d0 [vmxnet3]
> [ 14.369544] vmxnet3_activate_dev+0x3e3/0x2ca0 [vmxnet3]
> [ 14.369883] vmxnet3_open+0x387/0x470 [vmxnet3]
> [ 14.370174] __dev_open+0x28a/0x420
> [ 14.370399] __dev_change_flags+0x192/0x590
> [ 14.370667] dev_change_flags+0x7a/0x180
> [ 14.370919] do_setlink+0xb28/0x3570
> [ 14.371150] rtnl_newlink+0x1160/0x1740
> [ 14.371399] rtnetlink_rcv_msg+0x5bf/0xa50
> [ 14.371661] netlink_rcv_skb+0x1cd/0x3e0
> [ 14.371913] netlink_unicast+0x5dc/0x840
> [ 14.372169] netlink_sendmsg+0x856/0xc40
> [ 14.372420] ____sys_sendmsg+0x8a7/0x8d0
> [ 14.372673] __sys_sendmsg+0x1c2/0x270
> [ 14.372914] do_syscall_64+0x41/0x90
> [ 14.373145] entry_SYSCALL_64_after_hwframe+0x44/0xae
> ...
>
> Reported-by: TOTE Robot <[email protected]>
> Signed-off-by: Zixuan Fu <[email protected]>
> ---
> drivers/net/vmxnet3/vmxnet3_drv.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
> index d9d90baac72a..f17e9871ba27 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -588,7 +588,6 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
> DMA_FROM_DEVICE);
> if (dma_mapping_error(&adapter->pdev->dev,
> rbi->dma_addr)) {
> - dev_kfree_skb_any(rbi->skb);
> rq->stats.rx_buf_alloc_failure++;
> break;
> }
> @@ -612,7 +611,6 @@ vmxnet3_rq_alloc_rx_buf(struct vmxnet3_rx_queue *rq, u32 ring_idx,
> DMA_FROM_DEVICE);
> if (dma_mapping_error(&adapter->pdev->dev,
> rbi->dma_addr)) {
> - put_page(rbi->page);
> rq->stats.rx_buf_alloc_failure++;
> break;
> }