Patchwork [RFC] hostapd: Fix pointer assignment for new iface alloc

login
register
mail settings
Submitter Mohammed Shafi Shajakhan
Date May 16, 2013, 2:44 p.m.
Message ID <1368715448-8665-1-git-send-email-mohammed@qca.qualcomm.com>
Download mbox | patch
Permalink /patch/244346/
State Rejected
Headers show

Comments

Mohammed Shafi Shajakhan - May 16, 2013, 2:44 p.m.
From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>

interface count has to be increment, otherwise the previous interfaces
per-interface data structure maintained inside 'hapd_interfaces'
is over-written.

Signed-hostap: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
---
 src/ap/hostapd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Jouni Malinen - May 16, 2013, 4:45 p.m.
On Thu, May 16, 2013 at 08:14:08PM +0530, Mohammed Shafi Shajakhan wrote:
> interface count has to be increment, otherwise the previous interfaces
> per-interface data structure maintained inside 'hapd_interfaces'
> is over-written.

> diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c
> @@ -1185,6 +1185,7 @@ hostapd_iface_alloc(struct hapd_interfaces *interfaces)
>  	if (iface == NULL)
>  		return NULL;
>  	interfaces->iface = iface;
> +	interfaces->count++;
>  	hapd_iface = interfaces->iface[interfaces->count] =
>  		os_zalloc(sizeof(*hapd_iface));

Huh? That would make this interfaces->iface[interfaces->count]
assignment a buffer overflow. The previous code looks fine as-is.
Mohammed Shafi Shajakhan - May 17, 2013, 5:15 a.m.
On Thu, May 16, 2013 at 07:45:30PM +0300, Jouni Malinen wrote:
> On Thu, May 16, 2013 at 08:14:08PM +0530, Mohammed Shafi Shajakhan wrote:
> > interface count has to be increment, otherwise the previous interfaces
> > per-interface data structure maintained inside 'hapd_interfaces'
> > is over-written.
> 
> > diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c
> > @@ -1185,6 +1185,7 @@ hostapd_iface_alloc(struct hapd_interfaces *interfaces)
> >  	if (iface == NULL)
> >  		return NULL;
> >  	interfaces->iface = iface;
> > +	interfaces->count++;
> >  	hapd_iface = interfaces->iface[interfaces->count] =
> >  		os_zalloc(sizeof(*hapd_iface));
> 
> Huh? That would make this interfaces->iface[interfaces->count]
> assignment a buffer overflow. The previous code looks fine as-is.

sorry, misread the code. thanks for the review.

shafi

> 
> -- 
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap

Patch

diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c
index a0ac38c..e04d498 100644
--- a/src/ap/hostapd.c
+++ b/src/ap/hostapd.c
@@ -1185,6 +1185,7 @@  hostapd_iface_alloc(struct hapd_interfaces *interfaces)
 	if (iface == NULL)
 		return NULL;
 	interfaces->iface = iface;
+	interfaces->count++;
 	hapd_iface = interfaces->iface[interfaces->count] =
 		os_zalloc(sizeof(*hapd_iface));
 	if (hapd_iface == NULL) {
@@ -1192,7 +1193,6 @@  hostapd_iface_alloc(struct hapd_interfaces *interfaces)
 			   "the interface", __func__);
 		return NULL;
 	}
-	interfaces->count++;
 	hapd_iface->interfaces = interfaces;
 
 	return hapd_iface;