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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux