[PATCH] P2P: Handle possible long P2P Device interface name

Otcheretianski, Andrei andrei.otcheretianski at intel.com
Sun Apr 7 01:05:53 PDT 2019


> On Wed, Apr 03, 2019 at 06:17:11PM +0300, Andrei Otcheretianski wrote:
> > The way that the P2P Device interface name was constructed, might
> > result with an interface name that exceeds the maximal allowed
> > interface name length (IFNAMSZ).
> >
> > Fix this by properly limiting the created interface name length.
> 
> How is this supposed to work and guarantee that the truncated interface name
> would be unique?

It is not guaranteed to be unique. That's why we print a warning. In this case and in case that the driver can't handle this,
wpa_drv_if_add() will probably return an error.
Anyway it's a p2p device, so most probably there will be no such issue.
> 
> > diff --git a/wpa_supplicant/p2p_supplicant.c
> > b/wpa_supplicant/p2p_supplicant.c @@ -3794,14 +3794,21 @@ int
> > wpas_p2p_add_p2pdev_interface(struct wpa_supplicant *wpa_s,  {
> >  	struct wpa_interface iface;
> >  	struct wpa_supplicant *p2pdev_wpa_s;
> > -	char ifname[100];
> > -	char force_name[100];
> > +	char ifname[IFNAMSIZ];
> > +	char force_name[IFNAMSIZ];
> 
> IFNAMSIZ as the array length would mean that the array can hold only
> IFNAMSIZ-1 character long name, so this would be truncating to shorter than
> IFNAMSIZ characters..

I think IFNAMSIZ includes the NULL terminator. Though in quite a lot of places in the code it's indeed used with +1 for some reason.
Here is a nl80211 policy as an example:
"[NL80211_ATTR_IFNAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ-1 },"

> 
> >  	ret = os_snprintf(ifname, sizeof(ifname), P2P_MGMT_DEVICE_PREFIX
> "%s",
> >  			  wpa_s->ifname);
> > -	if (os_snprintf_error(sizeof(ifname), ret))
> > +
> > +	if (ret >= IFNAMSIZ) {
> > +		wpa_printf(MSG_WARNING,
> > +			   "P2P: P2P Device interface name truncated=%s",
> > +			   ifname);
> > +	} else if (ret < 0) {
> >  		return -1;
> > +	}
> 
> So what if snprintf return IFNAMSIZ? Wouldn't that leave ifname[] without nul
> termination here? And that could result in reading beyond the end of the buffer
> when using this string, e.g., in that wpa_printf() print.

snprintf() never leaves strings without NULL termination. So it's ok.

Andrei

> 
> --
> Jouni Malinen                                            PGP id EFC895FA



More information about the Hostap mailing list