On Tue, Mar 15, 2022 at 12:56 AM David Laight David.Laight@aculab.com wrote:
From: T.J. Mercier
Sent: 14 March 2022 23:45
On Thu, Mar 10, 2022 at 11:33 AM Todd Kjos tkjos@google.com wrote:
On Wed, Mar 9, 2022 at 8:52 AM T.J. Mercier tjmercier@google.com wrote:
The kernel interface should use types that the kernel defines instead of pid_t and uid_t, whose definiton is owned by libc. This fixes the header so that it can be included without first including sys/types.h.
Signed-off-by: T.J. Mercier tjmercier@google.com
include/uapi/linux/android/binder.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h index 169fd5069a1a..aa28454dbca3 100644 --- a/include/uapi/linux/android/binder.h +++ b/include/uapi/linux/android/binder.h @@ -289,8 +289,8 @@ struct binder_transaction_data {
/* General information about the transaction. */ __u32 flags;
pid_t sender_pid;uid_t sender_euid;
__kernel_pid_t sender_pid;__kernel_uid_t sender_euid;Are we guaranteed that this does not affect the UAPI at all? Userspace code using this definition will have to run with kernels using the old definition and visa-versa.
A standards compliant userspace should be expecting a signed integer type here. So the only way I can think userspace would be affected is if:
- pid_t is a long AND
- sizeof(long) > sizeof(int) AND
- Consumers of the pid_t definition actually attempt to mutate the
result to make use of extra bits in the variable (which are not there)
Or the userspace headers have a 16bit pid_t.
Since the kernel uses an int for PIDs, wouldn't a 16 bit pid_t already be potentially broken (overflow) on systems where int is not 16 bits? On systems where int is 16 bits, there is no change here except to achieve uniform use of __kernel_pid_t in the kernel headers and fix the include problem.
I can't help feeling that uapi headers should only use explicit fixed sized types. There is no point indirecting the type names - the sizes still can't be changes.
I think it's still unlikely to be an actual problem. For example there are other occasions where a switch like this was made: https://github.com/torvalds/linux/commit/694a58e29ef27c4c26f103a9decfd053f94... https://github.com/torvalds/linux/commit/269b8fd5d058f2c0da01a42b20315ffc264...
And also since Binder's only known user is Android through Bionic which already expects the type of pid_t to be __kernel_pid_t.
David
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)