Skip to content

Commit

Permalink
ravb: Fix use-after-free on ifconfig eth0 down
Browse files Browse the repository at this point in the history
Commit a47b70e ("ravb: unmap descriptors when freeing rings") has
introduced the issue seen in [1] reproduced on H3ULCB board.

Fix this by relocating the RX skb ringbuffer free operation, so that
swiotlb page unmapping can be done first. Freeing of aligned TX buffers
is not relevant to the issue seen in [1]. Still, reposition TX free
calls as well, to have all kfree() operations performed consistently
_after_ dma_unmap_*()/dma_free_*().

[1] Console screenshot with the problem reproduced:

salvator-x login: root
root@salvator-x:~# ifconfig eth0 up
Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: \
       attached PHY driver [Micrel KSZ9031 Gigabit PHY]   \
       (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=235)
IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
root@salvator-x:~#
root@salvator-x:~# ifconfig eth0 down

==================================================================
BUG: KASAN: use-after-free in swiotlb_tbl_unmap_single+0xc4/0x35c
Write of size 1538 at addr ffff8006d884f780 by task ifconfig/1649

CPU: 0 PID: 1649 Comm: ifconfig Not tainted 4.12.0-rc4-00004-g112eb07287d1 openbmc#32
Hardware name: Renesas H3ULCB board based on r8a7795 (DT)
Call trace:
[<ffff20000808f11c>] dump_backtrace+0x0/0x3a4
[<ffff20000808f4d4>] show_stack+0x14/0x1c
[<ffff20000865970c>] dump_stack+0xf8/0x150
[<ffff20000831f8b0>] print_address_description+0x7c/0x330
[<ffff200008320010>] kasan_report+0x2e0/0x2f4
[<ffff20000831eac0>] check_memory_region+0x20/0x14c
[<ffff20000831f054>] memcpy+0x48/0x68
[<ffff20000869ed50>] swiotlb_tbl_unmap_single+0xc4/0x35c
[<ffff20000869fcf4>] unmap_single+0x90/0xa4
[<ffff20000869fd14>] swiotlb_unmap_page+0xc/0x14
[<ffff2000080a2974>] __swiotlb_unmap_page+0xcc/0xe4
[<ffff2000088acdb8>] ravb_ring_free+0x514/0x870
[<ffff2000088b25dc>] ravb_close+0x288/0x36c
[<ffff200008aaf8c4>] __dev_close_many+0x14c/0x174
[<ffff200008aaf9b4>] __dev_close+0xc8/0x144
[<ffff200008ac2100>] __dev_change_flags+0xd8/0x194
[<ffff200008ac221c>] dev_change_flags+0x60/0xb0
[<ffff200008ba2dec>] devinet_ioctl+0x484/0x9d4
[<ffff200008ba7b78>] inet_ioctl+0x190/0x194
[<ffff200008a78c44>] sock_do_ioctl+0x78/0xa8
[<ffff200008a7a128>] sock_ioctl+0x110/0x3c4
[<ffff200008365a70>] vfs_ioctl+0x90/0xa0
[<ffff200008365dbc>] do_vfs_ioctl+0x148/0xc38
[<ffff2000083668f0>] SyS_ioctl+0x44/0x74
[<ffff200008083770>] el0_svc_naked+0x24/0x28

The buggy address belongs to the page:
page:ffff7e001b6213c0 count:0 mapcount:0 mapping:          (null) index:0x0
flags: 0x4000000000000000()
raw: 4000000000000000 0000000000000000 0000000000000000 00000000ffffffff
raw: 0000000000000000 ffff7e001b6213e0 0000000000000000 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff8006d884f680: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff8006d884f700: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>ffff8006d884f780: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
                   ^
 ffff8006d884f800: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff8006d884f880: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
Disabling lock debugging due to kernel taint
root@salvator-x:~#

Fixes: a47b70e ("ravb: unmap descriptors when freeing rings")
Signed-off-by: Eugeniu Rosca <[email protected]>
Acked-by: Sergei Shtylyov <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
Eugeniu Rosca authored and davem330 committed Jun 6, 2017
1 parent e3ebdb2 commit 79514ef
Showing 1 changed file with 12 additions and 12 deletions.
24 changes: 12 additions & 12 deletions drivers/net/ethernet/renesas/ravb_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -230,18 +230,6 @@ static void ravb_ring_free(struct net_device *ndev, int q)
int ring_size;
int i;

/* Free RX skb ringbuffer */
if (priv->rx_skb[q]) {
for (i = 0; i < priv->num_rx_ring[q]; i++)
dev_kfree_skb(priv->rx_skb[q][i]);
}
kfree(priv->rx_skb[q]);
priv->rx_skb[q] = NULL;

/* Free aligned TX buffers */
kfree(priv->tx_align[q]);
priv->tx_align[q] = NULL;

if (priv->rx_ring[q]) {
for (i = 0; i < priv->num_rx_ring[q]; i++) {
struct ravb_ex_rx_desc *desc = &priv->rx_ring[q][i];
Expand Down Expand Up @@ -270,6 +258,18 @@ static void ravb_ring_free(struct net_device *ndev, int q)
priv->tx_ring[q] = NULL;
}

/* Free RX skb ringbuffer */
if (priv->rx_skb[q]) {
for (i = 0; i < priv->num_rx_ring[q]; i++)
dev_kfree_skb(priv->rx_skb[q][i]);
}
kfree(priv->rx_skb[q]);
priv->rx_skb[q] = NULL;

/* Free aligned TX buffers */
kfree(priv->tx_align[q]);
priv->tx_align[q] = NULL;

/* Free TX skb ringbuffer.
* SKBs are freed by ravb_tx_free() call above.
*/
Expand Down

0 comments on commit 79514ef

Please sign in to comment.