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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 Dec 2013 10:35:54 -0800
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Serban Constantinescu <serban.constantinescu@....com>
Cc:	arve@...roid.com, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org, john.stultz@...aro.org,
	ccross@...roid.com, Dave.Butcher@....com, irogers@...gle.com,
	romlem@...roid.com
Subject: Re: [PATCH v1 9/9] staging: android: binder: Add binder compat layer

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/