This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] aarch64: Fix ipc_perm definition for ILP32
- From: Yury Norov <ynorov at caviumnetworks dot com>
- To: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- Cc: sellcey at cavium dot com, libc-alpha <libc-alpha at sourceware dot org>, nd at arm dot com
- Date: Wed, 23 Aug 2017 13:10:23 +0300
- Subject: Re: [PATCH] aarch64: Fix ipc_perm definition for ILP32
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Yuri dot Norov at cavium dot com;
- References: <1503423981.28672.31.camel@cavium.com> <599D50F6.6060009@arm.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On Wed, Aug 23, 2017 at 10:55:02AM +0100, Szabolcs Nagy wrote:
> On 22/08/17 18:46, Steve Ellcey wrote:
> > Here is another aarch64 ILP32 patch. The mode field in ipc_perm in ILP32
> > should be a 16 bit field, not a 32 bit one. This was out-of-sync with what the
> > kernel had. This was causing sysvipc/test-sysvsem to fail in ILP32 mode.
> > This change is only needed for ILP32 so it doesn't need to go in until we add
> > that ABI but I am sending out for review and comments.
> >
> > 2017-08-22 Yury Norov <ynorov@caviumnetworks.com>
> > Steve Ellcey <sellcey@cavium.com>
> >
> > * sysdeps/unix/sysv/linux/aarch64/bits/ipc.h (ipc_perm):
> > Ifdef and pad the mode field for ILP32.
> >
> >
> > diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/ipc.h b/sysdeps/unix/sysv/linu
> > x/aarch64/bits/ipc.h
> > index cd1f06e..cd05b74 100644
> > --- a/sysdeps/unix/sysv/linux/aarch64/bits/ipc.h
> > +++ b/sysdeps/unix/sysv/linux/aarch64/bits/ipc.h
> > @@ -46,7 +46,12 @@ struct ipc_perm
> > __gid_t gid; /* Owner's group ID. */
> > __uid_t cuid; /* Creator's user ID. */
> > __gid_t cgid; /* Creator's group ID. */
> > +#ifdef __LP64
> > unsigned int mode; /* Read/write permission. */
> > +#else
> > + unsigned short int mode; /* Read/write permission. */
> > + unsigned short int __pad0;
> > +#endif
>
> when did this happen?
>
> as far as i can tell staging/ilp32-4.12 branch in
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
> still has unsigned int __kernel_mode_t in uapi, so this is
> an abi change compared to that branch.
>
> i guess it's for 32bit compat, but i'd like to see the
> kernel patch for this.
It is there from the very beginning of ILP32 patches. The patch changes
the ABI only for arm64/ilp32. LP64 remains untouched, and it works correct
with and without ilp32 patches and this fix.
To reproduce:
struct semid_ds seminfo;
static char name[] = "Hello";
key_t key = ftok (name, 'G');
semid = semget(key, 1, IPC_CREAT | IPC_EXCL | 0644);
semctl (semid, 0, IPC_STAT, &seminfo);
printf("%o\n", seminfo.sem_perm.mode); // bits 16-31 contain garbage
On kernel side no fixes needed because arm64/ilp32 ipc requests are
wired to compat layer, and the layout for structure is like this:
arch/arm64/include/asm/compat.h:
246 struct compat_ipc64_perm {
247 compat_key_t key;
248 __compat_uid32_t uid;
249 __compat_gid32_t gid;
250 __compat_uid32_t cuid;
251 __compat_gid32_t cgid;
252 unsigned short mode;
253 unsigned short __pad1;
254 unsigned short seq;
255 unsigned short __pad2;
256 compat_ulong_t unused1;
257 compat_ulong_t unused2;
258 };
So I would prefer to treat it not as the change of ABI, bit the
fix of ABI incompatibility on GLIBC side.
Yury