From: Seth Forshee <seth.forshee@canonical.com>
To: Ivo van Doorn <IvDoorn@gmail.com>,
Gertjan van Wingerde <gwingerde@gmail.com>
Cc: "John W. Linville" <linville@tuxdriver.com>,
linux-wireless@vger.kernel.org, users@rt2x00.serialmonkey.com,
Wolfgang Kufner <wolfgang.kufner@gmail.com>
Subject: Missing skb_pad() return value checks in rt2x00 driver
Date: Mon, 31 Jan 2011 15:45:16 -0600 [thread overview]
Message-ID: <20110131214515.GC6368@thinkpad-t410> (raw)
In looking at the following commit:
commit 739fd9405416e22732e46a9226a8cac379bd57fc
Author: Wolfgang Kufner <wolfgang.kufner@gmail.com>
Date: Mon Dec 13 12:39:12 2010 +0100
rt2x00: Pad beacon to multiple of 32 bits.
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?
Thanks,
Seth
>From 6cebee1731e158df888e5ec19f5c221ac8a1a213 Mon Sep 17 00:00:00 2001
From: Seth Forshee <seth.forshee@canonical.com>
Date: Mon, 31 Jan 2011 15:06:47 -0600
Subject: [PATCH] rt2x00: Check for errors from skb_pad() calls
Commit 739fd94 ("rt2x00: Pad beacon to multiple of 32 bits")
added calls to skb_pad() without checking the return value,
which could cause problems if any of those calls does happen
to fail. Add checks to prevent this from happening and move
the padding to the tops of the relevant functions so we can
bail out before writing to any registers.
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
drivers/net/wireless/rt2x00/rt2800lib.c | 13 +++++++++++--
drivers/net/wireless/rt2x00/rt61pci.c | 13 +++++++++++--
drivers/net/wireless/rt2x00/rt73usb.c | 13 +++++++++++--
3 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index f8ba01c..79d17da 100644
--- 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);
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 8de44dd..83ac31e 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -1965,6 +1965,17 @@ static void rt61pci_write_beacon(struct queue_entry *entry,
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.
*/
@@ -1985,8 +1996,6 @@ static void rt61pci_write_beacon(struct queue_entry *entry,
/*
* Write entire beacon with descriptor 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);
rt2x00pci_register_multiwrite(rt2x00dev, beacon_base,
entry_priv->desc, TXINFO_SIZE);
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 029be3c..5b15609 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -1550,6 +1550,17 @@ static void rt73usb_write_beacon(struct queue_entry *entry,
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.
*/
@@ -1576,8 +1587,6 @@ static void rt73usb_write_beacon(struct queue_entry *entry,
/*
* Write entire beacon with descriptor 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);
rt2x00usb_register_multiwrite(rt2x00dev, beacon_base, entry->skb->data,
entry->skb->len + padding_len);
--
1.7.1
next reply other threads:[~2011-01-31 21:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-31 21:45 Seth Forshee [this message]
2011-02-04 9:57 ` Missing skb_pad() return value checks in rt2x00 driver Stanislaw Gruszka
2011-02-06 16:17 ` Ivo Van Doorn
2011-02-06 19:37 ` Wolfgang Kufner
2011-02-07 4:10 ` Seth Forshee
2011-02-07 19:53 ` Seth Forshee
2011-02-14 10:03 ` Ivo Van Doorn
2011-02-14 15:39 ` Seth Forshee
2011-02-14 16:38 ` Ivo Van Doorn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110131214515.GC6368@thinkpad-t410 \
--to=seth.forshee@canonical.com \
--cc=IvDoorn@gmail.com \
--cc=gwingerde@gmail.com \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=users@rt2x00.serialmonkey.com \
--cc=wolfgang.kufner@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).