Commit 3eb00275 authored by Ralf Baechle's avatar Ralf Baechle Committed by David S. Miller

NET: Fix possible corruption in bpqether driver

The bpq ether driver is modifying the data art of the skb by first
dropping the KISS byte (a command byte for the radio) then prepending the
length + 4 of the remaining AX.25 packet to be transmitted as a little
endian 16-bit number.  If the high byte of the length has a different
value than the dropped KISS byte users of clones of the skb may observe
this as corruption.  This was observed with by running listen(8) -a which
uses a packet socket which clones transmit packets.  The corruption will
then typically be displayed for as a KISS "TX Delay" command for AX.25
packets in the range of 252..508 bytes or any other KISS command for
yet larger packets.

Fixed by using skb_cow to create a private copy should the skb be cloned.
Using skb_cow also allows us to cleanup the old logic to ensure sufficient
headroom in the skb.

While at it, replace a return of 0 from bpq_xmit with the proper constant
NETDEV_TX_OK which is now being used everywhere else in this function.

Affected: all 2.2, 2.4 and 2.6 kernels.
Signed-off-by: default avatarRalf Baechle <ralf@linux-mips.org>
Reported-by: default avatarJann Traschewski <jann@gmx.de>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent ab08999d
...@@ -249,7 +249,6 @@ drop: ...@@ -249,7 +249,6 @@ drop:
*/ */
static netdev_tx_t bpq_xmit(struct sk_buff *skb, struct net_device *dev) static netdev_tx_t bpq_xmit(struct sk_buff *skb, struct net_device *dev)
{ {
struct sk_buff *newskb;
unsigned char *ptr; unsigned char *ptr;
struct bpqdev *bpq; struct bpqdev *bpq;
int size; int size;
...@@ -263,28 +262,23 @@ static netdev_tx_t bpq_xmit(struct sk_buff *skb, struct net_device *dev) ...@@ -263,28 +262,23 @@ static netdev_tx_t bpq_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK; return NETDEV_TX_OK;
} }
skb_pull(skb, 1); skb_pull(skb, 1); /* Drop KISS byte */
size = skb->len; size = skb->len;
/* /*
* The AX.25 code leaves enough room for the ethernet header, but * We're about to mess with the skb which may still shared with the
* sendto() does not. * generic networking code so unshare and ensure it's got enough
* space for the BPQ headers.
*/ */
if (skb_headroom(skb) < AX25_BPQ_HEADER_LEN) { /* Ough! */ if (skb_cow(skb, AX25_BPQ_HEADER_LEN)) {
if ((newskb = skb_realloc_headroom(skb, AX25_BPQ_HEADER_LEN)) == NULL) { if (net_ratelimit())
printk(KERN_WARNING "bpqether: out of memory\n"); pr_err("bpqether: out of memory\n");
kfree_skb(skb); kfree_skb(skb);
return NETDEV_TX_OK;
}
if (skb->sk != NULL)
skb_set_owner_w(newskb, skb->sk);
kfree_skb(skb); return NETDEV_TX_OK;
skb = newskb;
} }
ptr = skb_push(skb, 2); ptr = skb_push(skb, 2); /* Make space for length */
*ptr++ = (size + 5) % 256; *ptr++ = (size + 5) % 256;
*ptr++ = (size + 5) / 256; *ptr++ = (size + 5) / 256;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment