[PATCH] libertas: new mesh control knobs

[David Woodhouse]

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?

> +		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?

-- 
dwmw2