The libertas driver copies the SSID buffer back to the wireless core and
appends a trailing NULL character for termination. This is
a) unnecessary because the buffer is allocated with kzalloc and is hence
already NULLed when this function is called, and
b) for priv->curbssparams.ssid_len == 32, it writes back one byte too
much which causes memory corruptions.
Fix this by removing the extra write.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Holger Schurig <[email protected]>
Cc: John W. Linville <[email protected]>
Cc: Stephen Hemminger <[email protected]>
Cc: Maithili Hinge <[email protected]>
Cc: Kiran Divekar <[email protected]>
Cc: Michael Hirsch <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/net/wireless/libertas/wext.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/libertas/wext.c b/drivers/net/wireless/libertas/wext.c
index be837a0..01c738b 100644
--- a/drivers/net/wireless/libertas/wext.c
+++ b/drivers/net/wireless/libertas/wext.c
@@ -1953,10 +1953,8 @@ static int lbs_get_essid(struct net_device *dev, struct iw_request_info *info,
if (priv->connect_status == LBS_CONNECTED) {
memcpy(extra, priv->curbssparams.ssid,
priv->curbssparams.ssid_len);
- extra[priv->curbssparams.ssid_len] = '\0';
} else {
memset(extra, 0, 32);
- extra[priv->curbssparams.ssid_len] = '\0';
}
/*
* If none, we may want to get the one that was set
--
1.6.3.3
(Sorry, had a typo in the linux-wireless list address, corrected now).
On Wed, Dec 16, 2009 at 05:12:58AM +0100, Daniel Mack wrote:
>
> The libertas driver copies the SSID buffer back to the wireless core and
> appends a trailing NULL character for termination. This is
>
> a) unnecessary because the buffer is allocated with kzalloc and is hence
> already NULLed when this function is called, and
>
> b) for priv->curbssparams.ssid_len == 32, it writes back one byte too
> much which causes memory corruptions.
>
> Fix this by removing the extra write.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Holger Schurig <[email protected]>
> Cc: John W. Linville <[email protected]>
> Cc: Stephen Hemminger <[email protected]>
> Cc: Maithili Hinge <[email protected]>
> Cc: Kiran Divekar <[email protected]>
> Cc: Michael Hirsch <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/net/wireless/libertas/wext.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/wext.c b/drivers/net/wireless/libertas/wext.c
> index be837a0..01c738b 100644
> --- a/drivers/net/wireless/libertas/wext.c
> +++ b/drivers/net/wireless/libertas/wext.c
> @@ -1953,10 +1953,8 @@ static int lbs_get_essid(struct net_device *dev, struct iw_request_info *info,
> if (priv->connect_status == LBS_CONNECTED) {
> memcpy(extra, priv->curbssparams.ssid,
> priv->curbssparams.ssid_len);
> - extra[priv->curbssparams.ssid_len] = '\0';
> } else {
> memset(extra, 0, 32);
> - extra[priv->curbssparams.ssid_len] = '\0';
> }
> /*
> * If none, we may want to get the one that was set
> --
> 1.6.3.3
>
As Dan didn't answer:
Acked-by: Holger Schurig <[email protected]>
--
http://www.holgerschurig.de
On Wed, 2009-12-16 at 05:12 +0100, Daniel Mack wrote:
> The libertas driver copies the SSID buffer back to the wireless core and
> appends a trailing NULL character for termination. This is
>
> a) unnecessary because the buffer is allocated with kzalloc and is hence
> already NULLed when this function is called, and
>
> b) for priv->curbssparams.ssid_len == 32, it writes back one byte too
> much which causes memory corruptions.
>
> Fix this by removing the extra write.
Acked-by: Dan Williams <[email protected]>
> Signed-off-by: Daniel Mack <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Holger Schurig <[email protected]>
> Cc: John W. Linville <[email protected]>
> Cc: Stephen Hemminger <[email protected]>
> Cc: Maithili Hinge <[email protected]>
> Cc: Kiran Divekar <[email protected]>
> Cc: Michael Hirsch <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/net/wireless/libertas/wext.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/wext.c b/drivers/net/wireless/libertas/wext.c
> index be837a0..01c738b 100644
> --- a/drivers/net/wireless/libertas/wext.c
> +++ b/drivers/net/wireless/libertas/wext.c
> @@ -1953,10 +1953,8 @@ static int lbs_get_essid(struct net_device *dev, struct iw_request_info *info,
> if (priv->connect_status == LBS_CONNECTED) {
> memcpy(extra, priv->curbssparams.ssid,
> priv->curbssparams.ssid_len);
> - extra[priv->curbssparams.ssid_len] = '\0';
> } else {
> memset(extra, 0, 32);
> - extra[priv->curbssparams.ssid_len] = '\0';
> }
> /*
> * If none, we may want to get the one that was set
On Wed, Dec 16, 2009 at 08:57:47AM -0800, Dan Williams wrote:
> On Wed, 2009-12-16 at 05:12 +0100, Daniel Mack wrote:
> > The libertas driver copies the SSID buffer back to the wireless core and
> > appends a trailing NULL character for termination. This is
> >
> > a) unnecessary because the buffer is allocated with kzalloc and is hence
> > already NULLed when this function is called, and
> >
> > b) for priv->curbssparams.ssid_len == 32, it writes back one byte too
> > much which causes memory corruptions.
> >
> > Fix this by removing the extra write.
>
> Acked-by: Dan Williams <[email protected]>
Thanks, everyone. Who will care to pick an queue this one?
Daniel
> > Signed-off-by: Daniel Mack <[email protected]>
> > Cc: Dan Williams <[email protected]>
> > Cc: Holger Schurig <[email protected]>
> > Cc: John W. Linville <[email protected]>
> > Cc: Stephen Hemminger <[email protected]>
> > Cc: Maithili Hinge <[email protected]>
> > Cc: Kiran Divekar <[email protected]>
> > Cc: Michael Hirsch <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > drivers/net/wireless/libertas/wext.c | 2 --
> > 1 files changed, 0 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/wireless/libertas/wext.c b/drivers/net/wireless/libertas/wext.c
> > index be837a0..01c738b 100644
> > --- a/drivers/net/wireless/libertas/wext.c
> > +++ b/drivers/net/wireless/libertas/wext.c
> > @@ -1953,10 +1953,8 @@ static int lbs_get_essid(struct net_device *dev, struct iw_request_info *info,
> > if (priv->connect_status == LBS_CONNECTED) {
> > memcpy(extra, priv->curbssparams.ssid,
> > priv->curbssparams.ssid_len);
> > - extra[priv->curbssparams.ssid_len] = '\0';
> > } else {
> > memset(extra, 0, 32);
> > - extra[priv->curbssparams.ssid_len] = '\0';
> > }
> > /*
> > * If none, we may want to get the one that was set
>
On Thu, Dec 17, 2009 at 07:15:08AM +0800, Daniel Mack wrote:
> On Wed, Dec 16, 2009 at 08:57:47AM -0800, Dan Williams wrote:
> > On Wed, 2009-12-16 at 05:12 +0100, Daniel Mack wrote:
> > > The libertas driver copies the SSID buffer back to the wireless core and
> > > appends a trailing NULL character for termination. This is
> > >
> > > a) unnecessary because the buffer is allocated with kzalloc and is hence
> > > already NULLed when this function is called, and
> > >
> > > b) for priv->curbssparams.ssid_len == 32, it writes back one byte too
> > > much which causes memory corruptions.
> > >
> > > Fix this by removing the extra write.
> >
> > Acked-by: Dan Williams <[email protected]>
>
> Thanks, everyone. Who will care to pick an queue this one?
Is there some reason it wouldn't be me?
John
--
John W. Linville ? ? ? ? ? ? ? ?Someday the world will need a hero, and you
[email protected] ? ? ? ? ? ? ? ? ?might be all we have. ?Be ready.
On Wed, 2009-12-16 at 20:01 -0500, John W. Linville wrote:
> On Thu, Dec 17, 2009 at 07:15:08AM +0800, Daniel Mack wrote:
> > On Wed, Dec 16, 2009 at 08:57:47AM -0800, Dan Williams wrote:
> > > On Wed, 2009-12-16 at 05:12 +0100, Daniel Mack wrote:
> > > > The libertas driver copies the SSID buffer back to the wireless core and
> > > > appends a trailing NULL character for termination. This is
> > > >
> > > > a) unnecessary because the buffer is allocated with kzalloc and is hence
> > > > already NULLed when this function is called, and
> > > >
> > > > b) for priv->curbssparams.ssid_len == 32, it writes back one byte too
> > > > much which causes memory corruptions.
> > > >
> > > > Fix this by removing the extra write.
> > >
> > > Acked-by: Dan Williams <[email protected]>
> >
> > Thanks, everyone. Who will care to pick an queue this one?
>
> Is there some reason it wouldn't be me?
No, it should be you. Please suck it into your 2.6.33 patchset and
we'll cc to stable when it hits linus I guess.
Dan