[PATCH] libertas: new mesh control knobs

[Dan Williams]

On Wed, 2007-05-30 at 17:23 +0100, David Woodhouse wrote:
> On Tue, 2007-05-08 at 01:30 +0000, luisca at cozybit.com wrote:
> > +static int wlan_mesh_ioctl(wlan_private * priv, struct ifreq *req, 
> > +		int cmd, int subcmd)
> >  {
> > +	struct iwreq *wrq = (struct iwreq *)req ;
> >  	struct cmd_ds_mesh_access mesh_access;
> > -	int ret;
> > -
> > +	int parameter ;
> > +	char str[128];
> > +	char *ptr = str  ;
> > +	int ret, i;
> > +	
> >  	lbs_deb_enter(LBS_DEB_IOCTL);
> >  
> >  	memset(&mesh_access, 0, sizeof(mesh_access));
> >  
> > -	ret = libertas_prepare_and_send_command(priv, cmd_mesh_access,
> > -				    cmd_act_mesh_get_ttl,
> > +	if ( cmd == WLAN_SETONEINT_GETNONE ) {
> > +		parameter = SUBCMD_DATA(wrq);
> 
> Looking at the SUBCMD_OFFSET/SUBCMD_DATA definitions in wext.h, this
> seems to preprocess to:
> 		parameter = *((int *)(wrq->u.name + 4));
> 
> Can you explain this in terms which don't scare me? Are we abusing the
> field in iwreq which is supposed to be used for the interface name? If
> we use ifrename to rename the interface to a name longer than 4
> characters, what happens? Am I missing something?

This is all f-ed up in most drivers; I'm trying to get a handle on what
different drivers do here.  The iwpriv stuff is a mess; it's not really
libertas' fault.

Dan

> > +		if ( parameter < 0 )
> > +			return -EINVAL ;
> > +		mesh_access.data[0] = parameter ;
> 
> Absent byteswap.
> 
> > +	}
> > +
> > +	else if ( subcmd == cmd_act_mesh_set_link_costs ) {
> > +		if (copy_from_user( str, wrq->u.data.pointer, sizeof(str)))
> > +			return -EFAULT ;
> > +
> > +		for (i = 0 ; i < COSTS_LIST_SIZE  ; i++) {
> > +			mesh_access.data[i] = cpu_to_le32(simple_strtoul(ptr, &ptr, 10)) ;
> 
> 
> You copy 128 bytes from userspace even if userspace didn't offer you
> that much? Should you limit yourself to wrq->u.data.length? 
> 
> And if userspace doesn't 0-terminate the string you get, don't you end
> up running off the end of your buffer when you call strtoul() on it?
>