[PATCH] rt2x00: fix memory corruption caused by eeprom buffer overflow

[PATCH] rt2x00: fix memory corruption caused by eeprom buffer overflow

[Pavel Roskin]

eeprom_93cx6_multiread() expects the last argument to be the buffer
length in words, but kzalloc() expects the length in bytes.  This
results in dangerous kernel memory corruption.

Since there are already occurrences of "EEPROM_SIZE * sizeof(u16)" in
the driver, I'm assuming that EEPROM_SIZE is in words, so the driver
needs to allocate more memory.

Signed-off-by: Pavel Roskin <proski@gnu.org>
---

 drivers/net/wireless/d80211/rt2x00/rt2400pci.c |    2 +-
 drivers/net/wireless/d80211/rt2x00/rt2500pci.c |    2 +-
 drivers/net/wireless/d80211/rt2x00/rt2500usb.c |    2 +-
 drivers/net/wireless/d80211/rt2x00/rt61pci.c   |    2 +-
 drivers/net/wireless/d80211/rt2x00/rt73usb.c   |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/d80211/rt2x00/rt2400pci.c b/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
index 2e3a514..ef47554 100644
--- a/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
@@ -2370,7 +2370,7 @@ static int rt2400pci_alloc_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Allocate the eeprom memory, check the eeprom width
 	 * and copy the entire eeprom into this allocated memory.
 	 */
-	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL);
+	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE * sizeof(u16), GFP_KERNEL);
 	if (!rt2x00dev->eeprom)
 		return -ENOMEM;
 
diff --git a/drivers/net/wireless/d80211/rt2x00/rt2500pci.c b/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
index 305cff6..1085978 100644
--- a/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
@@ -2526,7 +2526,7 @@ static int rt2500pci_alloc_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Allocate the eeprom memory, check the eeprom width
 	 * and copy the entire eeprom into this allocated memory.
 	 */
-	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL);
+	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE * sizeof(u16), GFP_KERNEL);
 	if (!rt2x00dev->eeprom)
 		return -ENOMEM;
 
diff --git a/drivers/net/wireless/d80211/rt2x00/rt2500usb.c b/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
index 0976c98..0ace302 100644
--- a/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
@@ -2386,7 +2386,7 @@ static int rt2500usb_alloc_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Allocate the eeprom memory, check the eeprom width
 	 * and copy the entire eeprom into this allocated memory.
 	 */
-	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL);
+	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE * sizeof(u16), GFP_KERNEL);
 	if (!rt2x00dev->eeprom)
 		return -ENOMEM;
 
diff --git a/drivers/net/wireless/d80211/rt2x00/rt61pci.c b/drivers/net/wireless/d80211/rt2x00/rt61pci.c
index 547c660..46c552f 100644
--- a/drivers/net/wireless/d80211/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/d80211/rt2x00/rt61pci.c
@@ -3015,7 +3015,7 @@ static int rt61pci_alloc_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Allocate the eeprom memory, check the eeprom width
 	 * and copy the entire eeprom into this allocated memory.
 	 */
-	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL);
+	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE * sizeof(u16), GFP_KERNEL);
 	if (!rt2x00dev->eeprom)
 		return -ENOMEM;
 
diff --git a/drivers/net/wireless/d80211/rt2x00/rt73usb.c b/drivers/net/wireless/d80211/rt2x00/rt73usb.c
index be66e2f..6fa5a7e 100644
--- a/drivers/net/wireless/d80211/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/d80211/rt2x00/rt73usb.c
@@ -2705,7 +2705,7 @@ static int rt73usb_alloc_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Allocate the eeprom memory, check the eeprom width
 	 * and copy the entire eeprom into this allocated memory.
 	 */
-	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL);
+	rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL * sizeof(u16));
 	if (!rt2x00dev->eeprom)
 		return -ENOMEM;
 

Re: [PATCH] rt2x00: fix memory corruption caused by eeprom buffer overflow

[Ivo Van Doorn]

> eeprom_93cx6_multiread() expects the last argument to be the buffer
> length in words, but kzalloc() expects the length in bytes.  This
> results in dangerous kernel memory corruption.
>
> Since there are already occurrences of "EEPROM_SIZE * sizeof(u16)" in
> the driver, I'm assuming that EEPROM_SIZE is in words, so the driver
> needs to allocate more memory.
>
> Signed-off-by: Pavel Roskin <proski@gnu.org>

ACK, this fix has been in CVS already, but I hadn't send the patch yet.

Ivo

Re: [PATCH] rt2x00: fix memory corruption caused by eeprom buffer overflow

[Ivo Van Doorn]

hi

> diff --git a/drivers/net/wireless/d80211/rt2x00/rt73usb.c b/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> index be66e2f..6fa5a7e 100644
> --- a/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> +++ b/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> @@ -2705,7 +2705,7 @@ static int rt73usb_alloc_eeprom(struct rt2x00_dev *rt2x00dev)
>          * Allocate the eeprom memory, check the eeprom width
>          * and copy the entire eeprom into this allocated memory.
>          */
> -       rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL);
> +       rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL * sizeof(u16));
>         if (!rt2x00dev->eeprom)
>                 return -ENOMEM;

you might want to change this to EEPROM_SIZE * sizeof(u16). ;)

Ivo

Re: [PATCH] rt2x00: fix memory corruption caused by eeprom buffer overflow

[Pavel Roskin]

On Mon, 2007-02-19 at 12:07 +0100, Ivo Van Doorn wrote:
> hi
> 
> > diff --git a/drivers/net/wireless/d80211/rt2x00/rt73usb.c b/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> > index be66e2f..6fa5a7e 100644
> > --- a/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> > +++ b/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> > @@ -2705,7 +2705,7 @@ static int rt73usb_alloc_eeprom(struct rt2x00_dev *rt2x00dev)
> >          * Allocate the eeprom memory, check the eeprom width
> >          * and copy the entire eeprom into this allocated memory.
> >          */
> > -       rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL);
> > +       rt2x00dev->eeprom = kzalloc(EEPROM_SIZE, GFP_KERNEL * sizeof(u16));
> >         if (!rt2x00dev->eeprom)
> >                 return -ENOMEM;
> 
> you might want to change this to EEPROM_SIZE * sizeof(u16). ;)

Shame on me.  I wrote a patch that worked on rt61, and then I discarded
it to make a "clean" fix for all drivers.

But anyway, I'm seeing now that CVS on sourceforge.net is not abandoned,
so I think the best approach would be to integrate it into the kernel.
It would be great if you remove dead code from CVS, all those
"experimental" branches that haven't been touched for months if not
years.  It was hard for me to find the actual sources.  I would have
spent last Sunday evening doing something more interesting than fixing
fixed bugs :)

Finally, I'll appreciate if the driver is fixed to compile against
wireless-dev kernels.  You can take an approach similar to iwlwifi and
even rely on the same d80211 package.

-- 
Regards,
Pavel Roskin

Re: [PATCH] rt2x00: fix memory corruption caused by eeprom buffer overflow

[Ivo Van Doorn]

Hi,

> Shame on me.  I wrote a patch that worked on rt61, and then I discarded
> it to make a "clean" fix for all drivers.
>
> But anyway, I'm seeing now that CVS on sourceforge.net is not abandoned,
> so I think the best approach would be to integrate it into the kernel.
> It would be great if you remove dead code from CVS, all those
> "experimental" branches that haven't been touched for months if not
> years.  It was hard for me to find the actual sources.  I would have
> spent last Sunday evening doing something more interesting than fixing
> fixed bugs :)

True true, everything useful is in the "source" module of the CVS tree.
Along with the legacy drivers for each chipset.
I thought the other older tree was already removed... :(

> Finally, I'll appreciate if the driver is fixed to compile against
> wireless-dev kernels.  You can take an approach similar to iwlwifi and
> even rely on the same d80211 package.

Well the d80211 stack inside rt2x00 is slightly modified to contain
compatibility
fixes for regular kernels. That way regular users can test rt2x00,
which is extremely helpful.
Other than that the stack also contains small patches that are in
testing and I am sending
to the wireless list when it is stable. At this moment I have for
example the sequence
counter patch that is currently under discussion on this list, as well
as the currently
antenna patch that will split antenna_sel into a RX and TX antenna selection.

Ivo