Re: [PATCH] Silence compilation warnings about setuid, setgid

[Peter Stephenson <p.stephenson@samsung.com>]

On Wed, 13 Jun 2018 13:41:25 -0500
dana <dana@dana.is> wrote:
> On 13 Jun 2018, at 12:19, Peter Stephenson <p.stephenson@samsung.com>
> wrote:
> >+	(void)setgid(getgid());  
> 
> Casting to void doesn't actually silence this warning with (at least
> some versions of) GCC/glibc. If the function has the
> warn_unused_result attribute, you have to (a) disable the warning
> entirely, (b) actually use the result somehow, or (c) use some hack
> like this:
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/ignore-value.h

Something like the following therefore ought to work generally.
Disabling an explicit cast to void is pretty broken behaviour, so I'm
gradually losing interest if for some reason this doesn't work.

pws

diff --git a/Src/options.c b/Src/options.c
index 590652e..600b649 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -769,15 +769,32 @@ dosetopt(int optno, int value, int force, char *new_opts)
     } else if(optno == PRIVILEGED && !value) {
 	/* unsetting PRIVILEGED causes the shell to make itself unprivileged */
 #ifdef HAVE_SETUID
-	setuid(getuid());
-	setgid(getgid());
-        if (setuid(getuid())) {
-            zwarn("failed to change user ID: %e", errno);
-            return -1;
-	} else if (setgid(getgid())) {
+	int ignore_err;
+	errno = 0;
+	/*
+	 * Set the GID first as if we set the UID to non-privileged it
+	 * might be impossible to restore the GID.
+	 *
+	 * Some OSes (possibly no longer around) have been known to
+	 * fail silently the first time, so we attempt the change twice.
+	 * If it fails we are guaranteed to pick this up the second
+	 * time, so ignore the first time.
+	 *
+	 * Some versions of gcc make it hard to ignore the results the
+	 * first time, hence the following.  (These are probably not
+	 * systems that require the doubled calls.)
+	 */
+	ignore_err = setgid(getgid());
+	(void)ignore_err;
+	ignore_err = setuid(getuid());
+	(void)ignore_err;
+	if (setgid(getgid())) {
             zwarn("failed to change group ID: %e", errno);
             return -1;
-        }
+        } else if (setuid(getuid())) {
+            zwarn("failed to change user ID: %e", errno);
+            return -1;
+	}
 #else
         zwarn("setuid not available");
         return -1;