Patchwork AArch64/ILP32: fix wrong sign extension in setgroups

login
register
mail settings
Submitter Andreas Schwab
Date May 28, 2015, 3:02 p.m.
Message ID <mvmoal4kany.fsf@hawking.suse.de>
Download mbox | patch
Permalink /patch/6963/
State New
Headers show

Comments

Andreas Schwab - May 28, 2015, 3:02 p.m.
With ILP32 pointers must be zero-extended when passed to syscalls, but
INLINE_SETXID_SYSCALL is losing the type information that is required by
ARGIFY.

Andreas.

	* nptl/descr.h (struct xid_command): Use __syscall_slong_t for id
	member.
	* sysdeps/nptl/setxid.h (__SETXID_1, __SETXID_2, __SETXID_3): Cast
	to __syscall_long_t instead of long.
	* sysdeps/unix/sysv/linux/aarch64/ilp32/setgroups.c: New file.
---
 nptl/descr.h                                      |  2 +-
 sysdeps/nptl/setxid.h                             |  6 ++---
 sysdeps/unix/sysv/linux/aarch64/ilp32/setgroups.c | 32 +++++++++++++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/aarch64/ilp32/setgroups.c
Roland McGrath - June 5, 2015, 8:40 p.m.
Using __syscall_slong_t seems appropriate.
But I don't quite understand why the new setgroups.c is required.
Andreas Schwab - June 8, 2015, 8:53 a.m.
Roland McGrath <roland@hack.frob.com> writes:

> But I don't quite understand why the new setgroups.c is required.

It's required to avoid a warning about casting different sized types
(pointer vs. __syscall_slong_t).  I'll add a comment.

Andreas.
Roland McGrath - June 9, 2015, 9:01 p.m.
> Roland McGrath <roland@hack.frob.com> writes:
> 
> > But I don't quite understand why the new setgroups.c is required.
> 
> It's required to avoid a warning about casting different sized types
> (pointer vs. __syscall_slong_t).  I'll add a comment.

Perhaps just add the cast to the linux/setgroups.c implementation?
It's pretty much universal that INLINE_SYSCALL does not know anything
useful about the types of the arguments and casts them to a type like
__syscall_slong_t.  We could make this a documented requirement for
using those macros.
Andreas Schwab - June 10, 2015, 7:17 a.m.
Roland McGrath <roland@hack.frob.com> writes:

> Perhaps just add the cast to the linux/setgroups.c implementation?

Yes, that's probably better.

Andreas.

Patch

diff --git a/nptl/descr.h b/nptl/descr.h
index 5bd1282..9e7046f 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -98,7 +98,7 @@  struct pthread_unwind_buf
 struct xid_command
 {
   int syscall_no;
-  long int id[3];
+  __syscall_slong_t id[3];
   volatile int cntr;
   volatile int error; /* -1: no call yet, 0: success seen, >0: error seen.  */
 };
diff --git a/sysdeps/nptl/setxid.h b/sysdeps/nptl/setxid.h
index a3ecb60..e9ce474 100644
--- a/sysdeps/nptl/setxid.h
+++ b/sysdeps/nptl/setxid.h
@@ -19,11 +19,11 @@ 
 #include <sysdep.h>
 
 #define __SETXID_1(cmd, arg1) \
-  cmd.id[0] = (long int) arg1
+  cmd.id[0] = (__syscall_slong_t) arg1
 #define __SETXID_2(cmd, arg1, arg2) \
-  __SETXID_1 (cmd, arg1); cmd.id[1] = (long int) arg2
+  __SETXID_1 (cmd, arg1); cmd.id[1] = (__syscall_slong_t) arg2
 #define __SETXID_3(cmd, arg1, arg2, arg3) \
-  __SETXID_2 (cmd, arg1, arg2); cmd.id[2] = (long int) arg3
+  __SETXID_2 (cmd, arg1, arg2); cmd.id[2] = (__syscall_slong_t) arg3
 
 #ifdef SINGLE_THREAD
 # define INLINE_SETXID_SYSCALL(name, nr, args...) \
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/setgroups.c b/sysdeps/unix/sysv/linux/aarch64/ilp32/setgroups.c
new file mode 100644
index 0000000..472be0c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/setgroups.c
@@ -0,0 +1,32 @@ 
+/* Copyright (C) 1997-2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <grp.h>
+#include <setxid.h>
+#include <sysdep.h>
+
+
+/* Set the group set for the current user to GROUPS (N of them).  For
+   Linux we must convert the array of groups into the format that the
+   kernel expects.  */
+int
+setgroups (size_t n, const gid_t *groups)
+{
+  return INLINE_SETXID_SYSCALL (setgroups, 2, n, (uintptr_t) groups);
+}
+libc_hidden_def (setgroups)