| From foo@baz Mon 17 Aug 2020 11:30:21 AM CEST |
| From: Xie He <xie.he.0141@gmail.com> |
| Date: Wed, 5 Aug 2020 18:50:40 -0700 |
| Subject: drivers/net/wan/lapbether: Added needed_headroom and a skb->len check |
| |
| From: Xie He <xie.he.0141@gmail.com> |
| |
| [ Upstream commit c7ca03c216acb14466a713fedf1b9f2c24994ef2 ] |
| |
| 1. Added a skb->len check |
| |
| This driver expects upper layers to include a pseudo header of 1 byte |
| when passing down a skb for transmission. This driver will read this |
| 1-byte header. This patch added a skb->len check before reading the |
| header to make sure the header exists. |
| |
| 2. Changed to use needed_headroom instead of hard_header_len to request |
| necessary headroom to be allocated |
| |
| In net/packet/af_packet.c, the function packet_snd first reserves a |
| headroom of length (dev->hard_header_len + dev->needed_headroom). |
| Then if the socket is a SOCK_DGRAM socket, it calls dev_hard_header, |
| which calls dev->header_ops->create, to create the link layer header. |
| If the socket is a SOCK_RAW socket, it "un-reserves" a headroom of |
| length (dev->hard_header_len), and assumes the user to provide the |
| appropriate link layer header. |
| |
| So according to the logic of af_packet.c, dev->hard_header_len should |
| be the length of the header that would be created by |
| dev->header_ops->create. |
| |
| However, this driver doesn't provide dev->header_ops, so logically |
| dev->hard_header_len should be 0. |
| |
| So we should use dev->needed_headroom instead of dev->hard_header_len |
| to request necessary headroom to be allocated. |
| |
| This change fixes kernel panic when this driver is used with AF_PACKET |
| SOCK_RAW sockets. |
| |
| Call stack when panic: |
| |
| [ 168.399197] skbuff: skb_under_panic: text:ffffffff819d95fb len:20 |
| put:14 head:ffff8882704c0a00 data:ffff8882704c09fd tail:0x11 end:0xc0 |
| dev:veth0 |
| ... |
| [ 168.399255] Call Trace: |
| [ 168.399259] skb_push.cold+0x14/0x24 |
| [ 168.399262] eth_header+0x2b/0xc0 |
| [ 168.399267] lapbeth_data_transmit+0x9a/0xb0 [lapbether] |
| [ 168.399275] lapb_data_transmit+0x22/0x2c [lapb] |
| [ 168.399277] lapb_transmit_buffer+0x71/0xb0 [lapb] |
| [ 168.399279] lapb_kick+0xe3/0x1c0 [lapb] |
| [ 168.399281] lapb_data_request+0x76/0xc0 [lapb] |
| [ 168.399283] lapbeth_xmit+0x56/0x90 [lapbether] |
| [ 168.399286] dev_hard_start_xmit+0x91/0x1f0 |
| [ 168.399289] ? irq_init_percpu_irqstack+0xc0/0x100 |
| [ 168.399291] __dev_queue_xmit+0x721/0x8e0 |
| [ 168.399295] ? packet_parse_headers.isra.0+0xd2/0x110 |
| [ 168.399297] dev_queue_xmit+0x10/0x20 |
| [ 168.399298] packet_sendmsg+0xbf0/0x19b0 |
| ...... |
| |
| Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> |
| Cc: Martin Schiller <ms@dev.tdt.de> |
| Cc: Brian Norris <briannorris@chromium.org> |
| Signed-off-by: Xie He <xie.he.0141@gmail.com> |
| Acked-by: Willem de Bruijn <willemb@google.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/net/wan/lapbether.c | 10 +++++++++- |
| 1 file changed, 9 insertions(+), 1 deletion(-) |
| |
| --- a/drivers/net/wan/lapbether.c |
| +++ b/drivers/net/wan/lapbether.c |
| @@ -157,6 +157,12 @@ static netdev_tx_t lapbeth_xmit(struct s |
| if (!netif_running(dev)) |
| goto drop; |
| |
| + /* There should be a pseudo header of 1 byte added by upper layers. |
| + * Check to make sure it is there before reading it. |
| + */ |
| + if (skb->len < 1) |
| + goto drop; |
| + |
| switch (skb->data[0]) { |
| case X25_IFACE_DATA: |
| break; |
| @@ -305,6 +311,7 @@ static void lapbeth_setup(struct net_dev |
| dev->netdev_ops = &lapbeth_netdev_ops; |
| dev->needs_free_netdev = true; |
| dev->type = ARPHRD_X25; |
| + dev->hard_header_len = 0; |
| dev->mtu = 1000; |
| dev->addr_len = 0; |
| } |
| @@ -331,7 +338,8 @@ static int lapbeth_new_device(struct net |
| * then this driver prepends a length field of 2 bytes, |
| * then the underlying Ethernet device prepends its own header. |
| */ |
| - ndev->hard_header_len = -1 + 3 + 2 + dev->hard_header_len; |
| + ndev->needed_headroom = -1 + 3 + 2 + dev->hard_header_len |
| + + dev->needed_headroom; |
| |
| lapbeth = netdev_priv(ndev); |
| lapbeth->axdev = ndev; |