aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRob Landley <rob@landley.net>2017-02-05 00:51:18 -0600
committerRob Landley <rob@landley.net>2017-02-05 00:51:18 -0600
commit4a4b3d65644ce403b0f22887fc0d38b0202ec8c7 (patch)
tree2baba9ef32aebd7cd40d3a63951eaba250233f09
parent796d873d95e1be63ece2345d0e985a057210c1b9 (diff)
downloadtoybox-4a4b3d65644ce403b0f22887fc0d38b0202ec8c7.tar.gz
Silence a warning.
Once upon a time you could call fchown() and let it fail. Then gcc decided not using its return code was inconcievable, but you could typecast it to (void) to shut it up. Then gcc noticed people doing that and clutched its pearls and took it away, so I added an if() statement that does nothing with the result because we _expect_ this to fail when we're not root. Then clang started complaining about an if (); statement with the semicolon on the same line, but decided it's ok if the ; is on the next line (I.E. significant whitespace in C), so I'm adding an "assignemnt to self" that gets optimized away so it does a more _explicit_ nothing (the same way you suppress gcc's broken "this isn't used uninitialized" warnings). If the compilers weren't going to so much trouble to force the issue I might add code to only call fchown when we're UID 0, but I refuse to be coerced into it. (And if getpid() is still a system call instead of a vdso member then it doesn't actually _save_ us anything, the dentry should be hot and the permission check was just "if (!uid)" before selinux entered into it and we're operating on an fd so the security's the same.)
-rw-r--r--lib/lib.c9
1 files changed, 6 insertions, 3 deletions
diff --git a/lib/lib.c b/lib/lib.c
index c20a06cf..6f7ed303 100644
--- a/lib/lib.c
+++ b/lib/lib.c
@@ -645,9 +645,12 @@ int copy_tempfile(int fdin, char *name, char **tempname)
fstat(fdin, &statbuf);
fchmod(fd, statbuf.st_mode);
- // It's fine if this fails (generally because we're not root), but gcc no
- // longer lets a (void) typecast silence the "unused result" warning, so...
- if (fchown(fd, statbuf.st_uid, statbuf.st_gid));
+ // We chmod before chown, which strips the suid bit. Caller has to explicitly
+ // switch it back on if they want to keep suid.
+
+ // I said IGNORING ERRORS. Both gcc and clang clutch their pearls about this
+ // but it's _supposed_ to fail when we're not root.
+ if (fchown(fd, statbuf.st_uid, statbuf.st_gid)) fd = fd;
return fd;
}