From: Chris Metcalf <cmetcalf@tilera.com>
To: libc-ports@sourceware.org
Subject: [PATCH] tilegx: provide a setxid.h to handle negative uids
Date: Thu, 17 May 2012 20:54:00 -0000 [thread overview]
Message-ID: <201205172053.q4HKrjdU035038@gx-1.internal.tilera.com> (raw)
We observed a bug where setreuid(-1, -1) was becoming
setreuid(4294967295, 4294967295) instead, and therefore failing,
instead of doing nothing and returning success.
The problem is that normally, tilegx64 converts uint32_t to 64 bits
by sign extending. This is counter-intuitive, but does allow for some
simplifications of the comparison instructions in the generated code.
So for a normal setreuid() call, the compiler would see the uint32_t
(aka uid_t) type for the function, and sign-extend for the SYSCALL
macro. However, with the pthread setxid mechanism, we store the
arguments in an array of int64_t. When we put the values into the
array, the compiler zero-extends them, and then they would be
passed to the kernel unchanged since they were already 64-bit types.
The result is the value 4294967295L, which is different from -1U.
To make this work properly on tilegx, we have to add a cast to
(int32_t) on the path so that whatever 32-bit values are put in the
array of longs get properly sign-extended.
---
This change is obviously accompanied by a change in the core
nptl/sysdeps/pthread/setxid.h file; we put an #ifndef __SETXID_1
around the chunk of code that #defines __SETXID_1 through __SETXID_3.
If the idea, and this change, seem acceptable, I'll send the patch
for the core file as well.
ChangeLog.tile | 2 +
sysdeps/unix/sysv/linux/tile/nptl/setxid.h | 43 ++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 0 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/tile/nptl/setxid.h
diff --git a/ChangeLog.tile b/ChangeLog.tile
index 0befa86..baebcce 100644
--- a/ChangeLog.tile
+++ b/ChangeLog.tile
@@ -1,5 +1,7 @@
2012-05-17 Chris Metcalf <cmetcalf@tilera.com>
+ * sysdeps/unix/sysv/linux/tile/nptl/setxid.h: New file.
+
* sysdeps/tile/fegetenv.c: Version fegetenv() like fesetenv().
* sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/nptl/libm.abilist:
Add fegetenv.
diff --git a/sysdeps/unix/sysv/linux/tile/nptl/setxid.h b/sysdeps/unix/sysv/linux/tile/nptl/setxid.h
new file mode 100644
index 0000000..8a4902b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tile/nptl/setxid.h
@@ -0,0 +1,43 @@
+/* Override nptl/sysdeps/pthread/setxid.h for tilegx64.
+ Copyright (C) 2012 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 <bits/wordsize.h>
+
+/* On tilegx, we sign-extend 32-bit unsigned values to set the high 32
+ bits in a 64-bit register. Thus, a -1U value and a -1UL value are
+ represented the same way in a register. This allows some compiler
+ optimizations where we don't have to explicitly clear the high bits
+ and can use native 64-bit instructions for 32-bit values. However,
+ this means that if we are trying to pass a -1U value into the "long
+ int" fields of the xid_command.id[] fields, we need to explicitly
+ sign-extend the values before placing them in the fields, since we
+ eventually make the syscall using INTERNAL_SYSCALL_NCS(), at which
+ point the register values need to be in the correct ABI format. */
+
+#if __WORDSIZE == 64
+# define __SETXID_1(cmd, arg1) \
+ cmd.id[0] = (sizeof (arg1) == 4) ? (int) arg1 : (long int) arg1
+# define __SETXID_2(cmd, arg1, arg2) \
+ __SETXID_1 (cmd, arg1); \
+ cmd.id[1] = (sizeof (arg2) == 4) ? (int) arg2 : (long int) arg2
+# define __SETXID_3(cmd, arg1, arg2, arg3) \
+ __SETXID_2 (cmd, arg1, arg2); \
+ cmd.id[2] = (sizeof (arg3) == 4) ? (int) arg3 : (long int) arg3
+#endif
+
+#include_next <setxid.h>
--
1.7.1
next reply other threads:[~2012-05-17 20:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-17 20:54 Chris Metcalf [this message]
2012-05-17 23:21 ` Joseph S. Myers
2012-05-18 17:05 ` Chris Metcalf
2012-05-22 13:28 ` Chris Metcalf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201205172053.q4HKrjdU035038@gx-1.internal.tilera.com \
--to=cmetcalf@tilera.com \
--cc=libc-ports@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).