Re: Missing skb_pad() return value checks in rt2x00 driver
From: Ivo Van Doorn <hidden>
Date: 2011-02-06 16:17:56
Hi,
I noticed that the skb_pad() calls it introduces are not being checked for failure. I don't know whether or not it's possible for these calls to fail in this context; would it make sense to incorporate the patch below?
The suggested change sounds good, however I'm a bit worried about the relocation of the padding.
quoted hunk ↗ jump to hunk
--- a/drivers/net/wireless/rt2x00/rt2800lib.c +++ b/drivers/net/wireless/rt2x00/rt2800lib.c@@ -776,6 +776,17 @@ void rt2800_write_beacon(struct queue_entry *entry, struct txentry_desc *txdesc)u32 reg; /* + * Pad out the beacon to a 32-bit boundary + */ + padding_len = roundup(entry->skb->len, 4) - entry->skb->len; + if (padding_len && skb_pad(entry->skb, padding_len)) { + dev_err(rt2x00dev->dev, "Failure padding beacon, aborting\n"); + /* skb freed by skb_pad() on failure */ + entry->skb = NULL; + return; + } + + /* * Disable beaconing while we are reloading the beacon data, * otherwise we might be sending out invalid data. */@@ -809,8 +820,6 @@ void rt2800_write_beacon(struct queue_entry *entry, struct txentry_desc *txdesc)/* * Write entire beacon with TXWI and padding to register. */ - padding_len = roundup(entry->skb->len, 4) - entry->skb->len; - skb_pad(entry->skb, padding_len); beacon_base = HW_BEACON_OFFSET(entry->entry_idx); rt2800_register_multiwrite(rt2x00dev, beacon_base, entry->skb->data, entry->skb->len + padding_len);
Between here and where you added the padding are a couple of function calls which use the skb->len field. So this patch would change the value what they expect. Have you checked the possible impact? Ivo