bug#10021: [PATCH id] Add error-checking on GNU

[Paul Eggert]

No further comment to
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10021#77
so I pushed that patch to groups, install, su, test, whoami.

This should fix all the problems that coreutils has with
getuid etc. returning -1.  (Now all we have to do is fix
all the other packages that assume getuid etc. never fail.
:-)

bug#10021: [PATCH id] Add error-checking on GNU

[Jim Meyering]

Paul Eggert wrote:
 I found yet-another tricky bug in that id.c code,
 and fixed it as follows:

 Subject: [PATCH] id: fix bug when euid != ruid

 * src/id.c (main): Report an error if no args are given and getuid
 fails, because print_full_info needs ruid.  Redo code so that
 getuid and friends are invoked only when needed; this makes the
 code easier to follow, and is how I found the above bug.

Thanks.
I've marked this as done.

bug#10021: [PATCH id] Add error-checking on GNU

[Paul Eggert]

Here are proposed patches for the other coreutils applications
that invoke getuid et al.

port to GNU hosts, where getuid and friends can fail
* src/groups.c (main):
* src/install.c (need_copy):
* src/su.c (log_su):
* src/test.c (unary_operator):
* src/whoami.c (main):
Don't assume that getuid and friends always succeed.
This fixes the same problem that we recently fixed with 'id'.
diff --git a/src/groups.c b/src/groups.c
index abb7bc7..279b831 100644
--- a/src/groups.c
+++ b/src/groups.c
@@ -97,9 +97,23 @@ main (int argc, char **argv)
   if (optind == argc)
 {
   /* No arguments.  Divulge the details of the current process. */
+  uid_t NO_UID = -1;
+  gid_t NO_GID = -1;
+
+  errno = 0;
   ruid = getuid ();
+  if (ruid == NO_UID  errno)
+error (EXIT_FAILURE, errno, _(cannot get real UID));
+
+  errno = 0;
   egid = getegid ();
+  if (egid == NO_GID  errno)
+error (EXIT_FAILURE, errno, _(cannot get effective GID));
+
+  errno = 0;
   rgid = getgid ();
+  if (rgid == NO_GID  errno)
+error (EXIT_FAILURE, errno, _(cannot get real GID));

   if (!print_group_list (NULL, ruid, rgid, egid, true))
 ok = false;
diff --git a/src/install.c b/src/install.c
index dbff9c9..94534f8 100644
--- a/src/install.c
+++ b/src/install.c
@@ -192,9 +192,27 @@ need_copy (const char *src_name, const char *dest_name,
 return true;

   if (src_sb.st_size != dest_sb.st_size
-  || (dest_sb.st_mode  CHMOD_MODE_BITS) != mode
-  || dest_sb.st_uid != (owner_id == (uid_t) -1 ? getuid () : owner_id)
-  || dest_sb.st_gid != (group_id == (gid_t) -1 ? getgid () : group_id))
+  || (dest_sb.st_mode  CHMOD_MODE_BITS) != mode)
+return true;
+
+  if (owner_id == (uid_t) -1)
+{
+  errno = 0;
+  uid_t ruid = getuid ();
+  if ((ruid == (uid_t) -1  errno) || dest_sb.st_uid != ruid)
+return true;
+}
+  else if (dest_sb.st_uid != owner_id)
+return true;
+
+  if (group_id == (uid_t) -1)
+{
+  errno = 0;
+  gid_t rgid = getgid ();
+  if ((rgid == (uid_t) -1  errno) || dest_sb.st_gid != rgid)
+return true;
+}
+  else if (dest_sb.st_gid != group_id)
 return true;

   /* compare SELinux context if preserving */
diff --git a/src/su.c b/src/su.c
index 081ecb2..b1ba2a7 100644
--- a/src/su.c
+++ b/src/su.c
@@ -173,7 +173,10 @@ log_su (struct passwd const *pw, bool successful)
 {
   /* getlogin can fail -- usually due to lack of utmp entry.
  Resort to getpwuid.  */
-  struct passwd *pwd = getpwuid (getuid ());
+  errno = 0;
+  uid_t ruid = getuid ();
+  uid_t NO_UID = -1;
+  struct passwd *pwd = (ruid == NO_UID  errno ? NULL : getpwuid (ruid));
   old_user = (pwd ? pwd-pw_name : );
 }
   tty = ttyname (STDERR_FILENO);
diff --git a/src/test.c b/src/test.c
index 362df65..1b06ca8 100644
--- a/src/test.c
+++ b/src/test.c
@@ -414,13 +414,21 @@ unary_operator (void)

 case 'O':  /* File is owned by you? */
   unary_advance ();
-  return (stat (argv[pos - 1], stat_buf) == 0
-   (geteuid () == stat_buf.st_uid));
+  if (stat (argv[pos - 1], stat_buf) != 0)
+return false;
+  errno = 0;
+  uid_t euid = geteuid ();
+  uid_t NO_UID = -1;
+  return ! (euid == NO_UID  errno)  euid == stat_buf.st_uid;

 case 'G':  /* File is owned by your group? */
   unary_advance ();
-  return (stat (argv[pos - 1], stat_buf) == 0
-   (getegid () == stat_buf.st_gid));
+  if (stat (argv[pos - 1], stat_buf) != 0)
+return false;
+  errno = 0;
+  gid_t egid = getegid ();
+  gid_t NO_GID = -1;
+  return ! (egid == NO_GID  errno)  egid == stat_buf.st_gid;

 case 'f':  /* File is a file? */
   unary_advance ();
diff --git a/src/whoami.c b/src/whoami.c
index e563dcf..1b0c3cd 100644
--- a/src/whoami.c
+++ b/src/whoami.c
@@ -61,6 +61,7 @@ main (int argc, char **argv)
 {
   struct passwd *pw;
   uid_t uid;
+  uid_t NO_UID = -1;

   initialize_main (argc, argv);
   set_program_name (argv[0]);
@@ -81,8 +82,9 @@ main (int argc, char **argv)
   usage (EXIT_FAILURE);
 }

+  errno = 0;
   uid = geteuid ();
-  pw = getpwuid (uid);
+  pw = (uid == NO_UID  errno ? NULL : getpwuid (uid));
   if (pw)
 {
   puts (pw-pw_name);

bug#10021: [PATCH id] Add error-checking on GNU

[Eric Blake]

On 11/15/2011 10:09 AM, Eric Blake wrote:
 Still debatable.  POSIX explicitly states that the condition of errno
 after a successful call to a standardized function is unspecified; that
 is, a successful geteuid() may pollute errno, but it's okay, because the
 user shouldn't be inspecting errno after geteuid().
 
 It might be worth proposing a change to POSIX to require that geteuid()
 and friends leave errno unchanged on success (in order to allow for the
 GNU extension of setting errno on failure, even though POSIX did not
 reserve a specific value for failure); I'll pursue that course.

Done: http://austingroupbugs.net/view.php?id=511

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

bug#10021: [PATCH id] Add error-checking on GNU

[Ludovic Courtès]

Hi Paul,

Paul Eggert egg...@cs.ucla.edu skribis:

 -  if (GETID_MAY_FAIL  euid == -1  !use_real
 +  if (euid  0  !use_real
 !just_group  !just_group_list  !just_context)
  error (EXIT_FAILURE, errno, _(cannot get effective UID));

On GNU/Hurd, no error would ever be raised (since uid_t is unsigned),
which defeats the purpose of the patch initially proposed.

Thanks,
Ludo’.

bug#10021: [PATCH id] Add error-checking on GNU

[Eric Blake]

On 11/15/2011 09:59 AM, Paul Eggert wrote:
 On 11/15/11 05:07, Ludovic Courtès wrote:
 
 On GNU/Hurd, no error would ever be raised (since uid_t is unsigned),
 
 Ouch.  Thanks, now I understand Roland's suggestion.
 How about this patch instead?

else
  {
 +  /* POSIX says getuid etc. cannot fail, but they can fail under
 + GNU/Hurd and a few other systems.  Test for failure by
 + checking errno.  */
 +  uid_t NO_UID = -1;
 +  gid_t NO_GID = -1;
 +
 +  errno = 0;
euid = geteuid ();
 -  if (GETID_MAY_FAIL  euid == -1  !use_real
 +  if (euid == NO_UID  errno  !use_real
 !just_group  !just_group_list  !just_context)
  error (EXIT_FAILURE, errno, _(cannot get effective UID));
 

Still debatable.  POSIX explicitly states that the condition of errno
after a successful call to a standardized function is unspecified; that
is, a successful geteuid() may pollute errno, but it's okay, because the
user shouldn't be inspecting errno after geteuid().

It might be worth proposing a change to POSIX to require that geteuid()
and friends leave errno unchanged on success (in order to allow for the
GNU extension of setting errno on failure, even though POSIX did not
reserve a specific value for failure); I'll pursue that course.

But I like this version better than any previous one about trying to
reject result  0.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

bug#10021: [PATCH id] Add error-checking on GNU

[Jim Meyering]

Paul Eggert wrote:
 On 11/15/11 05:07, Ludovic Courtès wrote:

 On GNU/Hurd, no error would ever be raised (since uid_t is unsigned),

 Ouch.  Thanks, now I understand Roland's suggestion.
 How about this patch instead?

 id: handle (uid_t) -1 more portably
 * src/id.c (GETID_MAY_FAIL): Remove.
 (main): Check for nonzero errno, rather than having a compile-time
 GETID_MAY_FAIL guess.  Suggested by Roland McGrath in
 http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10021#47.
 Also, the old code was incorrect if uid_t was narrower than int.
 (print_full_info): Remove unnecessary cast to -1.
 diff --git a/src/id.c b/src/id.c
 index 047e40b..9fa93f8 100644
 --- a/src/id.c
 +++ b/src/id.c
 @@ -38,13 +38,6 @@
proper_name (Arnold Robbins), \
proper_name (David MacKenzie)

 -/* Whether the functions getuid, geteuid, getgid and getegid may fail.  */
 -#ifdef __GNU__
 -# define GETID_MAY_FAIL 1
 -#else
 -# define GETID_MAY_FAIL 0
 -#endif
 -
  /* If nonzero, output only the SELinux context. -Z */
  static int just_context = 0;

 @@ -208,22 +201,32 @@ main (int argc, char **argv)
  }
else
  {
 +  /* POSIX says getuid etc. cannot fail, but they can fail under
 + GNU/Hurd and a few other systems.  Test for failure by
 + checking errno.  */
 +  uid_t NO_UID = -1;
 +  gid_t NO_GID = -1;
 +
 +  errno = 0;
euid = geteuid ();
 -  if (GETID_MAY_FAIL  euid == -1  !use_real
 +  if (euid == NO_UID  errno  !use_real

I like that.  Thanks!

bug#10021: [PATCH id] Add error-checking on GNU

[Paul Eggert]

On 11/15/11 09:10, Jim Meyering wrote:
 I like that.  Thanks!

You're welcome; I pushed it.

Similar fixes are needed for groups.c,
install.c, su.c, test.c, and whoami.c,
due to the possibility of getuid etc.
failing on GNU/Hurd.

I'll add this to my list of things to do, and
post a patch here, unless someone else gets to it
first.

bug#10021: [PATCH id] Add error-checking on GNU

[Paul Eggert]

I found yet-another tricky bug in that id.c code,
and fixed it as follows:

From 5ca593e87eb1361b4696dd1e002a8ee310a5d1f6 Mon Sep 17 00:00:00 2001
From: Paul Eggert egg...@cs.ucla.edu
Date: Tue, 15 Nov 2011 13:23:24 -0800
Subject: [PATCH] id: fix bug when euid != ruid

* src/id.c (main): Report an error if no args are given and getuid
fails, because print_full_info needs ruid.  Redo code so that
getuid and friends are invoked only when needed; this makes the
code easier to follow, and is how I found the above bug.
---
 src/id.c |   51 ++-
 1 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/src/id.c b/src/id.c
index 8f7ce9e..1047149 100644
--- a/src/id.c
+++ b/src/id.c
@@ -207,27 +207,36 @@ main (int argc, char **argv)
   uid_t NO_UID = -1;
   gid_t NO_GID = -1;
 
-  errno = 0;
-  euid = geteuid ();
-  if (euid == NO_UID  errno  !use_real
-   !just_group  !just_group_list  !just_context)
-error (EXIT_FAILURE, errno, _(cannot get effective UID));
-
-  errno = 0;
-  ruid = getuid ();
-  if (ruid == NO_UID  errno  use_real
-   !just_group  !just_group_list  !just_context)
-error (EXIT_FAILURE, errno, _(cannot get real UID));
-
-  errno = 0;
-  egid = getegid ();
-  if (egid == NO_GID  errno  !use_real  !just_user)
-error (EXIT_FAILURE, errno, _(cannot get effective GID));
-
-  errno = 0;
-  rgid = getgid ();
-  if (rgid == NO_GID  errno  use_real  !just_user)
-error (EXIT_FAILURE, errno, _(cannot get real GID));
+  if (just_user ? !use_real
+  : !just_group  !just_group_list  !just_context)
+{
+  errno = 0;
+  euid = geteuid ();
+  if (euid == NO_UID  errno)
+error (EXIT_FAILURE, errno, _(cannot get effective UID));
+}
+
+  if (just_user ? use_real
+  : !just_group  (just_group_list || !just_context))
+{
+  errno = 0;
+  ruid = getuid ();
+  if (ruid == NO_UID  errno)
+error (EXIT_FAILURE, errno, _(cannot get real UID));
+}
+
+  if (!just_user  (just_group || just_group_list || !just_context))
+{
+  errno = 0;
+  egid = getegid ();
+  if (egid == NO_GID  errno)
+error (EXIT_FAILURE, errno, _(cannot get effective GID));
+
+  errno = 0;
+  rgid = getgid ();
+  if (rgid == NO_GID  errno)
+error (EXIT_FAILURE, errno, _(cannot get real GID));
+}
 }
 
   if (just_user)
-- 
1.7.6.4

bug#10021: [PATCH id] Add error-checking on GNU

[Jim Meyering]

Ludovic Courtès wrote:

 Hi Paul,

 Paul Eggert egg...@cs.ucla.edu skribis:

 On 11/12/11 13:48, Ludovic Courtès wrote:
 +#ifdef __GNU__
 +  if (euid == -1  !use_real
 +   !just_group  !just_group_list  !just_context)
 +error (EXIT_FAILURE, errno, _(cannot get effective UID));
 +#endif

 I suggest removing the #ifdef __GNU__ here and in its other
 three uses in the patch, as functions like as geteuid() can fail on
 a few non-GNU systems too.  See:

 http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fapis%2Fgeteuid.htm

 For this particular application (the 'id' program) I doubt whether
 it's worth our time to configure this stuff at compile-time,
 and that it's fine to do a run-time check on all platforms.

 OTOH, on POSIX-conforming systems (which includes GNU/Linux, so it may
 be the majority of systems in use), -1 may well be a valid UID/GID.

 That’s why I was conservative and decided to ifdef that.  This ifdef
 also served as a documentation that this is a GNU extension to POSIX.

 Perhaps it’d be safer to keep an ifdef and list all the OSes known to
 have this behavior?

Thanks for persevering.
I tested this on a gnu/linux system and found that I could indeed create a
user with a UID of 2^32-1 (though not directly via adduser -- it imposes
a maximum of 2^32-2), and that without something like those #ifdefs,
GNU id would mistakenly fail for such a user:

id: cannot get effective UID: No such file or directory

However, with the patch below, it does this:

$ ./id -u
4294967295

diff --git a/src/id.c b/src/id.c
index 19cf0cf..047e40b 100644
--- a/src/id.c
+++ b/src/id.c
@@ -38,6 +38,13 @@
   proper_name (Arnold Robbins), \
   proper_name (David MacKenzie)

+/* Whether the functions getuid, geteuid, getgid and getegid may fail.  */
+#ifdef __GNU__
+# define GETID_MAY_FAIL 1
+#else
+# define GETID_MAY_FAIL 0
+#endif
+
 /* If nonzero, output only the SELinux context. -Z */
 static int just_context = 0;

@@ -202,21 +209,21 @@ main (int argc, char **argv)
   else
 {
   euid = geteuid ();
-  if (euid == -1  !use_real
+  if (GETID_MAY_FAIL  euid == -1  !use_real
!just_group  !just_group_list  !just_context)
 error (EXIT_FAILURE, errno, _(cannot get effective UID));

   ruid = getuid ();
-  if (ruid == -1  use_real
+  if (GETID_MAY_FAIL  ruid == -1  use_real
!just_group  !just_group_list  !just_context)
 error (EXIT_FAILURE, errno, _(cannot get real UID));

   egid = getegid ();
-  if (egid == -1  !use_real  !just_user)
+  if (GETID_MAY_FAIL  egid == -1  !use_real  !just_user)
 error (EXIT_FAILURE, errno, _(cannot get effective GID));

   rgid = getgid ();
-  if (rgid == -1  use_real  !just_user)
+  if (GETID_MAY_FAIL  rgid == -1  use_real  !just_user)
 error (EXIT_FAILURE, errno, _(cannot get real GID));
 }

bug#10021: [PATCH id] Add error-checking on GNU

[Ludovic Courtès]

Hi Jim,

Jim Meyering j...@meyering.net skribis:

 Ludovic Courtès wrote:

 Hi Paul,

 Paul Eggert egg...@cs.ucla.edu skribis:

 On 11/12/11 13:48, Ludovic Courtès wrote:
 +#ifdef __GNU__
 +  if (euid == -1  !use_real
 +   !just_group  !just_group_list  !just_context)
 +error (EXIT_FAILURE, errno, _(cannot get effective UID));
 +#endif

 I suggest removing the #ifdef __GNU__ here and in its other
 three uses in the patch, as functions like as geteuid() can fail on
 a few non-GNU systems too.  See:

 http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fapis%2Fgeteuid.htm

 For this particular application (the 'id' program) I doubt whether
 it's worth our time to configure this stuff at compile-time,
 and that it's fine to do a run-time check on all platforms.

 OTOH, on POSIX-conforming systems (which includes GNU/Linux, so it may
 be the majority of systems in use), -1 may well be a valid UID/GID.

 That’s why I was conservative and decided to ifdef that.  This ifdef
 also served as a documentation that this is a GNU extension to POSIX.

 Perhaps it’d be safer to keep an ifdef and list all the OSes known to
 have this behavior?

 Thanks for persevering.
 I tested this on a gnu/linux system and found that I could indeed create a
 user with a UID of 2^32-1 (though not directly via adduser -- it imposes
 a maximum of 2^32-2), and that without something like those #ifdefs,
 GNU id would mistakenly fail for such a user:

 id: cannot get effective UID: No such file or directory

 However, with the patch below, it does this:

 $ ./id -u
 4294967295

OK, good to know.  Thank *you* for persevering!

Ludo’.

bug#10021: [PATCH id] Add error-checking on GNU

[Ludovic Courtès]

Hi,

Alan Curry pacman...@kosh.dhis.org skribis:

 Ludovic =?UTF-8?Q?Court=C3=A8s?= writes:
 
 OTOH, on POSIX-conforming systems (which includes GNU/Linux, so it may
 be the majority of systems in use), -1 may well be a valid UID/GID.

 That's a bizarre statement.

   3.428 User ID

   A non-negative integer

It’s always non negative since glibc defines it as unsigned (see
__UID_T_TYPE in typesizes.h.)

I really meant 2^32 - 1.

Thanks,
Ludo’.

bug#10021: [PATCH id] Add error-checking on GNU

[Jim Meyering]

Ludovic Courtès wrote:
...
 However, with the patch below, it does this:

 $ ./id -u
 4294967295

 OK, good to know.  Thank *you* for persevering!

I'll take that as an ACK ;-)
Pushed.

bug#10021: [PATCH id] Add error-checking on GNU

[Paul Eggert]

On 11/14/11 00:52, Jim Meyering wrote:
 I tested this on a gnu/linux system and found that I could indeed create a
 user with a UID of 2^32-1

If we really want to support that, there would be
a lot of other coreutils code that would need fixing,
no?  For example, the command

  chown 4294967295 /tmp/foo

always fails, but presumably on your host which has a
user (call it ubig) with UID 4294967295, the command

  chown ubig /tmp/foo

is a no-op?  Ouch.

I can see where this would have some serious security
implications.  Instead of trying to cater to systems
like that, it's probably better just to say Don't Do That,
i.e., GNU/Linux systems that have a user with ID == (uid_t) -1
are improperly set up and coreutils does not especially
cater to such systems.

That being said, it's OK if 'id' does support such systems,
(if only to diagnose their brokenness :-).  But looking at
the current code, id is making the unportable assumption that
uid_t is at least as wide as int.  And the way that id works
around the GNU porting problem isn't quite right, as there
are systems other than GNU that have the problem.

How about this patch?  It's simpler and should fix the
issues mentioned in the previous paragraph.

id: handle (uid_t) -1 more portably
* src/id.c (GETID_MAY_FAIL): Remove.
(main): Check for negative return values, not for -1.
The old code was incorrect if uid_t was narrower than int,
regardless of whether we were on a GNU or a POSIX platform.
The new code is simpler and doesn't need GETID_MAY_FAIL.
(print_full_info): Remove unnecessary cast to -1.
diff --git a/src/id.c b/src/id.c
index 047e40b..7bd8ebd 100644
--- a/src/id.c
+++ b/src/id.c
@@ -38,13 +38,6 @@
   proper_name (Arnold Robbins), \
   proper_name (David MacKenzie)

-/* Whether the functions getuid, geteuid, getgid and getegid may fail.  */
-#ifdef __GNU__
-# define GETID_MAY_FAIL 1
-#else
-# define GETID_MAY_FAIL 0
-#endif
-
 /* If nonzero, output only the SELinux context. -Z */
 static int just_context = 0;

@@ -208,22 +201,35 @@ main (int argc, char **argv)
 }
   else
 {
+  /* On GNU hosts, getuid etc. can fail and return -1.  On POSIX
+ hosts, such failures are not allowed and (uid_t) -1 may be a
+ valid UID if uid_t is unsigned.  To handle both cases
+ correctly, consider getuid etc. to fail if it returns a
+ negative value.
+
+ POSIX hosts should not give users a UID equal to (uid_t) -1
+ even if uid_t is unsigned, as system calls like chown would
+ behave unexpectedly with such a UID, and in general coreutils
+ therefore does not support such a UID.  However, 'id' makes a
+ special attempt to handle it, if only to help people diagnose
+ the problem.  */
+
   euid = geteuid ();
-  if (GETID_MAY_FAIL  euid == -1  !use_real
+  if (euid  0  !use_real
!just_group  !just_group_list  !just_context)
 error (EXIT_FAILURE, errno, _(cannot get effective UID));

   ruid = getuid ();
-  if (GETID_MAY_FAIL  ruid == -1  use_real
+  if (ruid  0  use_real
!just_group  !just_group_list  !just_context)
 error (EXIT_FAILURE, errno, _(cannot get real UID));

   egid = getegid ();
-  if (GETID_MAY_FAIL  egid == -1  !use_real  !just_user)
+  if (egid  0  !use_real  !just_user)
 error (EXIT_FAILURE, errno, _(cannot get effective GID));

   rgid = getgid ();
-  if (GETID_MAY_FAIL  rgid == -1  use_real  !just_user)
+  if (rgid  0  use_real  !just_user)
 error (EXIT_FAILURE, errno, _(cannot get real GID));
 }

@@ -316,7 +322,7 @@ print_full_info (const char *username)
 gid_t *groups;
 int i;

-int n_groups = xgetgroups (username, (pwd ? pwd-pw_gid : (gid_t) -1),
+int n_groups = xgetgroups (username, (pwd ? pwd-pw_gid : -1),
groups);
 if (n_groups  0)
   {

bug#10021: [PATCH id] Add error-checking on GNU

[Eric Blake]

On 11/14/2011 11:54 AM, Paul Eggert wrote:
else
  {
 +  /* On GNU hosts, getuid etc. can fail and return -1.  On POSIX
 + hosts, such failures are not allowed and (uid_t) -1 may be a
 + valid UID if uid_t is unsigned.

That doesn't read correctly.  You are correct that POSIX states that
getuid() cannot fail in a conforming environment (therefore, the only
way that GNU getuid fails is if you have done something outside the
realms of a conforming environment).  However, POSIX is quite clear that
chown() must honor (uid_t)(-1) as a means of not altering the uid of a
file, therefore, POSIX explicitly rejects (uid_t)(-1) as a valid UID.

POSIX leaves it up to the implementation whether the type uid_t is
signed or unsigned, but also specifies that valid uids are non-negative
integers (so if uid_t is signed, half the possible values will never be
encountered as valid uids).

  To handle both cases
 + correctly, consider getuid etc. to fail if it returns a
 + negative value.

But if uid_t is an unsigned type (which POSIX permits), then it will
never be a negative value.

 +
 + POSIX hosts should not give users a UID equal to (uid_t) -1
 + even if uid_t is unsigned, as system calls like chown would
 + behave unexpectedly with such a UID, and in general coreutils
 + therefore does not support such a UID.  However, 'id' makes a
 + special attempt to handle it, if only to help people diagnose
 + the problem.  */
 +
euid = geteuid ();
 -  if (GETID_MAY_FAIL  euid == -1  !use_real
 +  if (euid  0  !use_real

That is, how can this work?  On systems where uid_t is signed, it makes
sense, but on systems where uid_t is unsigned, this will always be false
(and probably trigger a gcc warning), and still fail to catch the
(uid_t)(-1) case that we are trying to diagnose.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

bug#10021: [PATCH id] Add error-checking on GNU

[Roland McGrath]

I think you can portably detect the failure case with the errno=0 method,
rather than relying on #ifdef.

bug#10021: [PATCH id] Add error-checking on GNU

[Ludovic Courtès]

Hi,

Eric Blake ebl...@redhat.com skribis:

 On 11/14/2011 11:54 AM, Paul Eggert wrote:

[...]

euid = geteuid ();
 -  if (GETID_MAY_FAIL  euid == -1  !use_real
 +  if (euid  0  !use_real

 That is, how can this work?  On systems where uid_t is signed, it makes
 sense, but on systems where uid_t is unsigned, this will always be false

And again, it *is* unsigned in glibc.

Thanks,
Ludo’.

bug#10021: [PATCH id] Add error-checking on GNU

[Paul Eggert]

On 11/14/11 11:12, Eric Blake wrote:
 POSIX explicitly rejects (uid_t)(-1) as a valid UID.

That depends on what one means by valid.  You're right about chown
of course, but it's not clear that POSIX absolutely prohibits getuid
from returning (uid_t) -1.  And even if POSIX did prohibit that,
GNU/Linux does not.

 if uid_t is an unsigned type (which POSIX permits), then it will
 never be a negative value.

Yes, that's correct, and the proposed patch relies on this.

I see that the proposed patch's comments weren't that clear about this,
and I enclose a revised proposal below, which is the same code but
(I hope) clearer comments.

 -  if (GETID_MAY_FAIL  euid == -1  !use_real
 +  if (euid  0  !use_real
 
 That is, how can this work?  On systems where uid_t is signed, it makes
 sense, but on systems where uid_t is unsigned, this will always be false
 (and probably trigger a gcc warning), and still fail to catch the
 (uid_t)(-1) case that we are trying to diagnose.

The idea is that it is not id's problem to diagnose the bug, because
no bug is triggered by id itself.  id should report what it found,
namely, a user with a funny userid (and presumably equally-funny name...).

That is, it's not id's job to anticipate bugs that may occur in other
applications.

We don't need to worry about the GCC warning, because there are
zillions of other places in coreutils that trigger the same warning.
Our approach is to ask builders to compile without that warning
enabled, or to ignore the warning, whichever they prefer.

Here's the revised proposal.

id: handle (uid_t) -1 more portably
* src/id.c (GETID_MAY_FAIL): Remove.
(main): Check for negative return values, not for -1.
The old code was incorrect if uid_t was narrower than int,
regardless of whether we were on a GNU or a POSIX platform.
The new code is simpler and doesn't need GETID_MAY_FAIL.
(print_full_info): Remove unnecessary cast to -1.
diff --git a/src/id.c b/src/id.c
index 047e40b..b3ee437 100644
--- a/src/id.c
+++ b/src/id.c
@@ -38,13 +38,6 @@
   proper_name (Arnold Robbins), \
   proper_name (David MacKenzie)

-/* Whether the functions getuid, geteuid, getgid and getegid may fail.  */
-#ifdef __GNU__
-# define GETID_MAY_FAIL 1
-#else
-# define GETID_MAY_FAIL 0
-#endif
-
 /* If nonzero, output only the SELinux context. -Z */
 static int just_context = 0;

@@ -208,22 +201,36 @@ main (int argc, char **argv)
 }
   else
 {
+  /* On GNU/Hurd hosts, getuid etc. can fail and return -1.
+ However, on GNU/Linux hosts, uid_t is an unsigned value and
+ getuid etc. can return the positive value (uid_t) -1.  To
+ handle both cases correctly, consider getuid etc. to fail if
+ it returns a negative value (a value that is impossible on
+ GNU/Linux hosts).
+
+ GNU/Linux sysadmins should not give users the UID (uid_t) -1
+ even though uid_t is unsigned, as system calls like chown would
+ not have the desired behavior with such a UID, and other
+ coreutils applications therefore do not support such a UID.
+ However, 'id' makes a special attempt to handle this UID, to
+ help people diagnose the problem.  */
+
   euid = geteuid ();
-  if (GETID_MAY_FAIL  euid == -1  !use_real
+  if (euid  0  !use_real
!just_group  !just_group_list  !just_context)
 error (EXIT_FAILURE, errno, _(cannot get effective UID));

   ruid = getuid ();
-  if (GETID_MAY_FAIL  ruid == -1  use_real
+  if (ruid  0  use_real
!just_group  !just_group_list  !just_context)
 error (EXIT_FAILURE, errno, _(cannot get real UID));

   egid = getegid ();
-  if (GETID_MAY_FAIL  egid == -1  !use_real  !just_user)
+  if (egid  0  !use_real  !just_user)
 error (EXIT_FAILURE, errno, _(cannot get effective GID));

   rgid = getgid ();
-  if (GETID_MAY_FAIL  rgid == -1  use_real  !just_user)
+  if (rgid  0  use_real  !just_user)
 error (EXIT_FAILURE, errno, _(cannot get real GID));
 }

@@ -316,7 +323,7 @@ print_full_info (const char *username)
 gid_t *groups;
 int i;

-int n_groups = xgetgroups (username, (pwd ? pwd-pw_gid : (gid_t) -1),
+int n_groups = xgetgroups (username, (pwd ? pwd-pw_gid : -1),
groups);
 if (n_groups  0)
   {

bug#10021: [PATCH id] Add error-checking on GNU

[Jim Meyering]

Ludovic Courtès wrote:
 Thanks for the quick review!

 Jim Meyering j...@meyering.net skribis:

 However, wouldn't that fail unnecessarily for a process without
 one ID even though id is being asked to print some other(s)?
 E.g., it'd fail for a process with no EUID even when id
 is being asked to print only the real UID or GIDs.

 Indeed.

 Also, if you send another version, please indent only with spaces
 and change each diagnostic to start with lower case letter.

 Sure.  (There’s one just above that doesn’t follow the rule.)

Thanks.  There are two like that.  I'll adjust them separately.

 You may want to run make syntax-check to check for things like that.

 OK.

 Here’s an updated patch with a test case.

 I don’t have a copyright assignment on file for Coreutils but I guess
 it’s OK for this change?

It's borderline, but ok once we take Paul's advice and remove the #ifdefs.
Thanks for the test suite addition.

In addition to the #ifdef-removing changes, I've also made these:

diff --git a/tests/id/gnu-zero-uids b/tests/id/gnu-zero-uids
index b4a7d5a..bc9043c 100644
--- a/tests/id/gnu-zero-uids
+++ b/tests/id/gnu-zero-uids
@@ -21,11 +21,9 @@ print_ver_ id

 require_gnu_

-if sush - true; then
-# Run `id' with zero UIDs.  It should exit with a non-zero status.
-sush - id  out  fail=1
-else
-skip_ the \`sush' command does not work
-fi
+sush - true || skip_ the \`sush' command does not work
+
+# Run `id' with zero UIDs.  It should exit with a non-zero status.
+sush - id  out  fail=1

 Exit $fail
diff --git a/tests/init.cfg b/tests/init.cfg
index f5f27dd..9b05b34 100644
--- a/tests/init.cfg
+++ b/tests/init.cfg
@@ -502,9 +502,8 @@ print_ver_()
 # Are we running on GNU/Hurd?
 require_gnu_()
 {
-  if test `uname` != GNU; then
- skip_ 'not running on GNU/Hurd'
-  fi
+  test $(uname) = GNU \
+|| skip_ 'not running on GNU/Hurd'
 }

 sanitize_path_

Here's the complete patch:
Oh, I also had to make the new test script executable,
or else make check would fail.

Since I've made so many changes, I'll wait to hear back
from you before pushing it.

From 70903f8e4ebabd08e2e01403d9eea1e45fe42bb8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= l...@gnu.org
Date: Sat, 12 Nov 2011 01:25:45 +0100
Subject: [PATCH] id: fail when getuid, getgid, etc. fail, e.g., on GNU/Hurd

POSIX-conforming getuid, geteuid, etc. functions cannot fail,
but on GNU/Hurd systems and some others, they may.
* src/id.c (main): Detect and diagnose any such failure.
* tests/id/gnu-zero-uids: New file.
* tests/Makefile.am (TESTS): Add it to the list.
* tests/init.cfg (require_gnu_): New function.
---
 src/id.c   |   13 +
 tests/Makefile.am  |1 +
 tests/id/gnu-zero-uids |   29 +
 tests/init.cfg |7 +++
 4 files changed, 50 insertions(+), 0 deletions(-)
 create mode 100644 tests/id/gnu-zero-uids

diff --git a/src/id.c b/src/id.c
index f80fcd1..0966d15 100644
--- a/src/id.c
+++ b/src/id.c
@@ -202,9 +202,22 @@ main (int argc, char **argv)
   else
 {
   euid = geteuid ();
+  if (euid == -1  !use_real
+   !just_group  !just_group_list  !just_context)
+error (EXIT_FAILURE, errno, _(cannot get effective UID));
+
   ruid = getuid ();
+  if (ruid == -1  use_real
+   !just_group  !just_group_list  !just_context)
+error (EXIT_FAILURE, errno, _(cannot get real UID));
+
   egid = getegid ();
+  if (egid == -1  !use_real  !just_user)
+error (EXIT_FAILURE, errno, _(cannot get effective GID));
+
   rgid = getgid ();
+  if (rgid == -1  use_real  !just_user)
+error (EXIT_FAILURE, errno, _(cannot get real GID));
 }

   if (just_user)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 64366a4..48c33cb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -399,6 +399,7 @@ TESTS = \
   du/slink \
   du/trailing-slash\
   du/two-args  \
+  id/gnu-zero-uids \
   id/no-context\
   install/basic-1  \
   install/create-leading   \
diff --git a/tests/id/gnu-zero-uids b/tests/id/gnu-zero-uids
new file mode 100644
index 000..bc9043c
--- /dev/null
+++ b/tests/id/gnu-zero-uids
@@ -0,0 +1,29 @@
+#!/bin/sh
+# On GNU, `id' must fail for processes with zero UIDs.
+
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or 

bug#10021: [PATCH id] Add error-checking on GNU

[Ludovic Courtès]

Hi Jim,

Jim Meyering j...@meyering.net skribis:

 Oh, I also had to make the new test script executable,
 or else make check would fail.

It works for me even if it’s not executable when I run “make check
TESTS=id/gnu-zero-uids”, apparently because it’s run this way:

  execve(/bin/sh, [/bin/sh, ./shell-or-perl, --test-name, 
id/gnu-zero-uids, --srcdir, ., --shell, /bin/sh, --perl, perl, 
--, ./id/gnu-zero-uids], [/* 96 vars */]) = 0

It might be different when running just “make check”.

(I tested it on GNU/Linux and GNU/Hurd.)

 Since I've made so many changes, I'll wait to hear back
 from you before pushing it.

Looks good to me, thank you!

 +  test $(uname) = GNU \

So you’re assuming Bash, right?

Thanks,
Ludo’.

bug#10021: [PATCH id] Add error-checking on GNU

[Ludovic Courtès]

Hi Paul,

Paul Eggert egg...@cs.ucla.edu skribis:

 On 11/12/11 13:48, Ludovic Courtès wrote:
 +#ifdef __GNU__
 +  if (euid == -1  !use_real
 +   !just_group  !just_group_list  !just_context)
 +error (EXIT_FAILURE, errno, _(cannot get effective UID));
 +#endif

 I suggest removing the #ifdef __GNU__ here and in its other
 three uses in the patch, as functions like as geteuid() can fail on
 a few non-GNU systems too.  See:

 http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fapis%2Fgeteuid.htm

 For this particular application (the 'id' program) I doubt whether
 it's worth our time to configure this stuff at compile-time,
 and that it's fine to do a run-time check on all platforms.

OTOH, on POSIX-conforming systems (which includes GNU/Linux, so it may
be the majority of systems in use), -1 may well be a valid UID/GID.

That’s why I was conservative and decided to ifdef that.  This ifdef
also served as a documentation that this is a GNU extension to POSIX.

Perhaps it’d be safer to keep an ifdef and list all the OSes known to
have this behavior?

Thanks,
Ludo’.

bug#10021: [PATCH id] Add error-checking on GNU

[Alan Curry]

Ludovic =?UTF-8?Q?Court=C3=A8s?= writes:
 
 OTOH, on POSIX-conforming systems (which includes GNU/Linux, so it may
 be the majority of systems in use), -1 may well be a valid UID/GID.

That's a bizarre statement.

  3.428 User ID

  A non-negative integer that is used to identify a system user. When the
  identity of a user is associated with a process, a user ID value is
  referred to as a real user ID, an effective user ID, or a saved
  set-user-ID.

chown(2) uses (uid_t)-1 and (gid_t)-1 as the don't change special values.
So does setreuid(2)/setregid(2).

setuid(-1) isn't documented as special. Trying it out, it seems to be treated
as equivalent to setuid(1). Not what I expected, but it doesn't really
support your -1 is a valid uid theory.

-- 
Alan Curry

bug#10021: [PATCH id] Add error-checking on GNU

[Jim Meyering]

Ludovic Courtès wrote:
 Jim Meyering j...@meyering.net skribis:

 Oh, I also had to make the new test script executable,
 or else make check would fail.

 It works for me even if it’s not executable when I run “make check
 TESTS=id/gnu-zero-uids”, apparently because it’s run this way:

   execve(/bin/sh, [/bin/sh, ./shell-or-perl, --test-name,
 id/gnu-zero-uids, --srcdir, ., --shell, /bin/sh, --perl,
 perl, --, ./id/gnu-zero-uids], [/* 96 vars */]) = 0

The test itself runs either way.  However there's a make check
sub-test (vc_exe_in_TESTS) that ensures all such scripts are
consistently chmod a+x.  It was failing.

 It might be different when running just “make check”.

 (I tested it on GNU/Linux and GNU/Hurd.)

 Since I've made so many changes, I'll wait to hear back
 from you before pushing it.

 Looks good to me, thank you!

 +  test $(uname) = GNU \

 So you’re assuming Bash, right?

No.  The test starts with the usual invocation of init.sh,
which ensures we're using a shell that handles $(...).

bug#10021: [PATCH id] Add error-checking on GNU

[Ludovic Courtès]

Hi Jim,

Thanks for the quick review!

Jim Meyering j...@meyering.net skribis:

 However, wouldn't that fail unnecessarily for a process without
 one ID even though id is being asked to print some other(s)?
 E.g., it'd fail for a process with no EUID even when id
 is being asked to print only the real UID or GIDs.

Indeed.

 Also, if you send another version, please indent only with spaces
 and change each diagnostic to start with lower case letter.

Sure.  (There’s one just above that doesn’t follow the rule.)

 You may want to run make syntax-check to check for things like that.

OK.

Here’s an updated patch with a test case.

I don’t have a copyright assignment on file for Coreutils but I guess
it’s OK for this change?

Thanks,
Ludo’.

From b1e9324ea83a2df604256d7d41d25440e21dd2d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= l...@gnu.org
Date: Sat, 12 Nov 2011 01:25:45 +0100
Subject: [PATCH] id: error out for zero UIDs/GIDs on GNU/Hurd

* src/id.c (main)[__GNU__]: Check for EUID, RUID, EGID, or RGID = -1;
  raise an error when appropriate.

* tests/Makefile.am (TESTS): Add `id/gnu-zero-uids'.
* tests/id/gnu-zero-uids: New file.
* tests/init.cfg (require_gnu_): New function.
---
 src/id.c   |   21 +
 tests/Makefile.am  |1 +
 tests/id/gnu-zero-uids |   31 +++
 tests/init.cfg |8 
 4 files changed, 61 insertions(+), 0 deletions(-)
 create mode 100644 tests/id/gnu-zero-uids

diff --git a/src/id.c b/src/id.c
index f80fcd1..9325282 100644
--- a/src/id.c
+++ b/src/id.c
@@ -202,9 +202,30 @@ main (int argc, char **argv)
   else
 {
   euid = geteuid ();
+#ifdef __GNU__
+  if (euid == -1  !use_real
+   !just_group  !just_group_list  !just_context)
+error (EXIT_FAILURE, errno, _(cannot get effective UID));
+#endif
+
   ruid = getuid ();
+#ifdef __GNU__
+  if (ruid == -1  use_real
+   !just_group  !just_group_list  !just_context)
+error (EXIT_FAILURE, errno, _(cannot get real UID));
+#endif
+
   egid = getegid ();
+#ifdef __GNU__
+  if (egid == -1  !use_real  !just_user)
+error (EXIT_FAILURE, errno, _(cannot get effective GID));
+#endif
+
   rgid = getgid ();
+#ifdef __GNU__
+  if (rgid == -1  use_real  !just_user)
+error (EXIT_FAILURE, errno, _(cannot get real GID));
+#endif
 }
 
   if (just_user)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5021c18..80f95b1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -399,6 +399,7 @@ TESTS =		\
   du/slink	\
   du/trailing-slash\
   du/two-args	\
+  id/gnu-zero-uids\
   id/no-context	\
   install/basic-1\
   install/create-leading			\
diff --git a/tests/id/gnu-zero-uids b/tests/id/gnu-zero-uids
new file mode 100644
index 000..b4a7d5a
--- /dev/null
+++ b/tests/id/gnu-zero-uids
@@ -0,0 +1,31 @@
+#!/bin/sh
+# On GNU, `id' must fail for processes with zero UIDs.
+
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program 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 General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+
+. ${srcdir=.}/init.sh; path_prepend_ ../src
+print_ver_ id
+
+require_gnu_
+
+if sush - true; then
+# Run `id' with zero UIDs.  It should exit with a non-zero status.
+sush - id  out  fail=1
+else
+skip_ the \`sush' command does not work
+fi
+
+Exit $fail
diff --git a/tests/init.cfg b/tests/init.cfg
index 915f38a..f5f27dd 100644
--- a/tests/init.cfg
+++ b/tests/init.cfg
@@ -499,4 +499,12 @@ print_ver_()
   fi
 }
 
+# Are we running on GNU/Hurd?
+require_gnu_()
+{
+  if test `uname` != GNU; then
+ skip_ 'not running on GNU/Hurd'
+  fi
+}
+
 sanitize_path_
-- 
1.7.6



pgpgmV9TWGefw.pgp
Description: PGP signature

bug#10021: [PATCH id] Add error-checking on GNU

[Paul Eggert]

On 11/12/11 13:48, Ludovic Courtès wrote:
 +#ifdef __GNU__
 +  if (euid == -1  !use_real
 +   !just_group  !just_group_list  !just_context)
 +error (EXIT_FAILURE, errno, _(cannot get effective UID));
 +#endif

I suggest removing the #ifdef __GNU__ here and in its other
three uses in the patch, as functions like as geteuid() can fail on
a few non-GNU systems too.  See:

http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fapis%2Fgeteuid.htm

For this particular application (the 'id' program) I doubt whether
it's worth our time to configure this stuff at compile-time,
and that it's fine to do a run-time check on all platforms.

bug#10021: [PATCH id] Add error-checking on GNU

[Ludovic Courtès]

Hello,

On GNU, processes can have zero or more UIDs/GIDs.  In the case of a
process with zero UIDs, for instance, ‘getuid’ returns -1 and sets
ERRNO [0] (as an extension to POSIX [1].)

Currently ‘id’ would print (unsigned int) -1 as the UID in that case,
whereas it should rather print an error.  The attached patch does that.

(Note that the Hurd comes with another utility, called ‘ids’, which
prints all the (E)[UG]IDs of the process and gracefully handles the
zero-UID/GID case [2].)

Thanks,
Ludo’.

[0] 
http://git.savannah.gnu.org/cgit/hurd/glibc.git/tree/sysdeps/mach/hurd/getuid.c
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/getuid.html
[2] http://git.savannah.gnu.org/cgit/hurd/hurd.git/tree/utils/ids.c

diff --git a/src/id.c b/src/id.c
index f80fcd1..824a471 100644
--- a/src/id.c
+++ b/src/id.c
@@ -202,9 +202,28 @@ main (int argc, char **argv)
   else
 {
   euid = geteuid ();
+#ifdef __GNU__
+  if (euid == -1)
+	error (EXIT_FAILURE, errno, _(Cannot get effective UID));
+#endif
+
   ruid = getuid ();
+#ifdef __GNU__
+  if (ruid == -1)
+	error (EXIT_FAILURE, errno, _(Cannot get real UID));
+#endif
+
   egid = getegid ();
+#ifdef __GNU__
+  if (egid == -1)
+	error (EXIT_FAILURE, errno, _(Cannot get effective GID));
+#endif
+
   rgid = getgid ();
+#ifdef __GNU__
+  if (rgid == -1)
+	error (EXIT_FAILURE, errno, _(Cannot get real GID));
+#endif
 }
 
   if (just_user)

bug#10021: [PATCH id] Add error-checking on GNU

[Jim Meyering]

Ludovic Courtès wrote:
 On GNU, processes can have zero or more UIDs/GIDs.  In the case of a
 process with zero UIDs, for instance, ‘getuid’ returns -1 and sets
 ERRNO [0] (as an extension to POSIX [1].)

 Currently ‘id’ would print (unsigned int) -1 as the UID in that case,
 whereas it should rather print an error.  The attached patch does that.

 (Note that the Hurd comes with another utility, called ‘ids’, which
 prints all the (E)[UG]IDs of the process and gracefully handles the
 zero-UID/GID case [2].)

 Thanks,
 Ludo’.

 [0]
 http://git.savannah.gnu.org/cgit/hurd/glibc.git/tree/sysdeps/mach/hurd/getuid.c
 [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/getuid.html
 [2] http://git.savannah.gnu.org/cgit/hurd/hurd.git/tree/utils/ids.c

Hi Ludo,

Thanks for the patch.
However, wouldn't that fail unnecessarily for a process without
one ID even though id is being asked to print some other(s)?
E.g., it'd fail for a process with no EUID even when id
is being asked to print only the real UID or GIDs.

Also, if you send another version, please indent only with spaces
and change each diagnostic to start with lower case letter.
You may want to run make syntax-check to check for things like that.

 diff --git a/src/id.c b/src/id.c
 index f80fcd1..824a471 100644
 --- a/src/id.c
 +++ b/src/id.c
 @@ -202,9 +202,28 @@ main (int argc, char **argv)
else
  {
euid = geteuid ();
 +#ifdef __GNU__
 +  if (euid == -1)
 + error (EXIT_FAILURE, errno, _(Cannot get effective UID));
 +#endif
 +
ruid = getuid ();
 +#ifdef __GNU__
 +  if (ruid == -1)
 + error (EXIT_FAILURE, errno, _(Cannot get real UID));
 +#endif
 +
egid = getegid ();
 +#ifdef __GNU__
 +  if (egid == -1)
 + error (EXIT_FAILURE, errno, _(Cannot get effective GID));
 +#endif
 +
rgid = getgid ();
 +#ifdef __GNU__
 +  if (rgid == -1)
 + error (EXIT_FAILURE, errno, _(Cannot get real GID));
 +#endif
  }

if (just_user)