| Message ID | 1260936778-2096-1-git-send-email-daniel@caiaq.de |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
(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 <daniel@caiaq.de> > Cc: Dan Williams <dcbw@redhat.com> > Cc: Holger Schurig <holgerschurig@gmail.com> > Cc: John W. Linville <linville@tuxdriver.com> > Cc: Stephen Hemminger <shemminger@vyatta.com> > Cc: Maithili Hinge <maithili@marvell.com> > Cc: Kiran Divekar <dkiran@marvell.com> > Cc: Michael Hirsch <m.hirsch@raumfeld.com> > Cc: netdev@vger.kernel.org > Cc: libertas-dev@lists.infradead.org > Cc: linux-wireless@lists.infradead.org > Cc: stable@kernel.org > --- > 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 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
As Dan didn't answer:
Acked-by: Holger Schurig <holgerschurig@gmail.com>
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 <dcbw@redhat.com> > Signed-off-by: Daniel Mack <daniel@caiaq.de> > Cc: Dan Williams <dcbw@redhat.com> > Cc: Holger Schurig <holgerschurig@gmail.com> > Cc: John W. Linville <linville@tuxdriver.com> > Cc: Stephen Hemminger <shemminger@vyatta.com> > Cc: Maithili Hinge <maithili@marvell.com> > Cc: Kiran Divekar <dkiran@marvell.com> > Cc: Michael Hirsch <m.hirsch@raumfeld.com> > Cc: netdev@vger.kernel.org > Cc: libertas-dev@lists.infradead.org > Cc: linux-wireless@lists.infradead.org > Cc: stable@kernel.org > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <dcbw@redhat.com> Thanks, everyone. Who will care to pick an queue this one? Daniel > > Signed-off-by: Daniel Mack <daniel@caiaq.de> > > Cc: Dan Williams <dcbw@redhat.com> > > Cc: Holger Schurig <holgerschurig@gmail.com> > > Cc: John W. Linville <linville@tuxdriver.com> > > Cc: Stephen Hemminger <shemminger@vyatta.com> > > Cc: Maithili Hinge <maithili@marvell.com> > > Cc: Kiran Divekar <dkiran@marvell.com> > > Cc: Michael Hirsch <m.hirsch@raumfeld.com> > > Cc: netdev@vger.kernel.org > > Cc: libertas-dev@lists.infradead.org > > Cc: linux-wireless@lists.infradead.org > > Cc: stable@kernel.org > > --- > > 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 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <dcbw@redhat.com> > > Thanks, everyone. Who will care to pick an queue this one? Is there some reason it wouldn't be me? John
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 <dcbw@redhat.com> > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 <daniel@caiaq.de> Cc: Dan Williams <dcbw@redhat.com> Cc: Holger Schurig <holgerschurig@gmail.com> Cc: John W. Linville <linville@tuxdriver.com> Cc: Stephen Hemminger <shemminger@vyatta.com> Cc: Maithili Hinge <maithili@marvell.com> Cc: Kiran Divekar <dkiran@marvell.com> Cc: Michael Hirsch <m.hirsch@raumfeld.com> Cc: netdev@vger.kernel.org Cc: libertas-dev@lists.infradead.org Cc: linux-wireless@lists.infradead.org Cc: stable@kernel.org --- drivers/net/wireless/libertas/wext.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)