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. > --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html