Re: [PATCH v1 9/9] staging: android: binder: Add binder compat layer

[Greg KH]

On Wed, Dec 04, 2013 at 06:09:41PM +0000, Serban Constantinescu wrote:
> +#define size_helper(x) ({						    \
> +	size_t __size;							    \
> +	if (!is_compat_task())						    \
> +		__size = sizeof(x);					    \
> +	else if (sizeof(x) == sizeof(struct flat_binder_object))	    \
> +		__size = sizeof(struct compat_flat_binder_object);	    \
> +	else if (sizeof(x) == sizeof(struct binder_transaction_data))	    \
> +		__size = sizeof(struct compat_binder_transaction_data);	    \
> +	else if (sizeof(x) == sizeof(size_t))				    \
> +		__size = sizeof(compat_size_t);				    \
> +	else								    \
> +		 BUG();							    \
> +	__size;								    \
> +	})

Ick.

First off, no driver should ever be able to crash the kernel, which you
just did.

Second, almost none of those "if" lines will ever be hit, why did you
include it all?

And finally, is this all really needed?  Why not just fix the structures
to be "correct", and then fix userspace to use the correct structures as
well, thereby not needing a compat layer at all?

You have the chance to fix the api properly, why not take it and do it,
making all of this unnecessary.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/