Re: [PATCH] wext: Fix 32 bit iwpriv compatibility issue with 64 bit Kernel

[Johannes Berg <johannes@sipsolutions.net>]

Hi,

I must say, this is a bit of a surprise - where is iwpriv actually
still relevant? What driver could this possibly matter for?

Anyway ...

> +	if (dev->netdev_ops->ndo_do_ioctl) {
> +		if ((info->flags & IW_REQUEST_FLAG_COMPAT) &&
> +				(cmd >= SIOCIWFIRSTPRIV && cmd <=
> SIOCIWLASTPRIV)) {

This has coding style issues, obviously.

Also, handling the non-compat case would allow you to return and reduce
indentation by one in the large code block that handles the compat.

> +			int ret = 0;
> +			struct compat_iw_point *iwp_compat = (struct compat_iw_point *) &iwr->u.data;
> +			struct iw_point *iwp = &iwr->u.data;
> +			__u16 length = iwp_compat->length, flags = iwp_compat->flags;
> +
> +			iwp->pointer = compat_ptr(iwp_compat->pointer);
> +			iwp->length = length;
> +			iwp->flags = flags;
> +
> +			ret = dev->netdev_ops->ndo_do_ioctl(dev, ifr, cmd);
> +
> +			length = iwp->length;
> +			flags = iwp->flags;
> +			iwp_compat->pointer = ptr_to_compat(iwp->pointer);
> +			iwp_compat->length = length;
> +			iwp_compat->flags = flags;

Why don't you just put another ifr/iwr on the stack, and use that to
pass things to the driver? This modify-in-place of 'iwp', which
requires loading all the variables first, seems very awkward to me.


johannes