From: Mark Wielaard <mark@klomp.org>
To: elfutils-devel@sourceware.org
Cc: Mark Wielaard <mark@klomp.org>
Subject: [PATCH 06/10] ar, ranlib: Call fchown before fchmod, explicitly check and ignore errors.
Date: Wed, 17 Jun 2020 00:25:35 +0200 [thread overview]
Message-ID: <20200616222539.29109-6-mark@klomp.org> (raw)
In-Reply-To: <20200616222539.29109-1-mark@klomp.org>
In ar and ranlib we don't mind if the fchown call fails (it normally
would, then the file simply gets own by the current user). We used to
call fchown before fchmod, but that might ignore (or reset) some mode
flags, so call fchown first, then try fchmod (like we already do in
elfcompress). Also explicitly test and then ignore any errors for
chown. We used to do some asm trick, but that confuses some static
analyzers (and it is somewhat unreadable). Also split out the giant
if statements to make them a little bit more understandable.
Signed-off-by: Mark Wielaard <mark@klomp.org>
---
src/ChangeLog | 8 +++++++
src/ar.c | 65 ++++++++++++++++++++++++++++++---------------------
src/ranlib.c | 44 +++++++++++++++++++---------------
3 files changed, 72 insertions(+), 45 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index e78bc358..0129b8bf 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,11 @@
+2020-06-16 Mark Wielaard <mark@klomp.org>
+
+ * ar.c (do_oper_extract): Split large if statement. Call fchown
+ before fchmod and explicitly ignore the return value.
+ (do_oper_delete): Likewise.
+ (do_oper_insert): Likewise.
+ * ranlib.c (handle_file): Likewise.
+
2020-06-16 Mark Wielaard <mark@klomp.org>
* elflint.c (check_elf_header): Explicitly check and ignore
diff --git a/src/ar.c b/src/ar.c
index d70f1f46..7d33d814 100644
--- a/src/ar.c
+++ b/src/ar.c
@@ -787,26 +787,30 @@ cannot rename temporary file to %.*s"),
else
rest_off = SARMAG;
- if ((symtab.symsnamelen != 0
+ if (symtab.symsnamelen != 0
&& ((write_retry (newfd, symtab.symsoff,
symtab.symsofflen)
!= (ssize_t) symtab.symsofflen)
|| (write_retry (newfd, symtab.symsname,
symtab.symsnamelen)
!= (ssize_t) symtab.symsnamelen)))
- /* Even if the original file had content before the
- symbol table, we write it in the correct order. */
- || (index_off != SARMAG
- && copy_content (elf, newfd, SARMAG, index_off - SARMAG))
- || copy_content (elf, newfd, rest_off, st.st_size - rest_off)
- /* Set the mode of the new file to the same values the
- original file has. */
- || fchmod (newfd, st.st_mode & ALLPERMS) != 0
- /* Never complain about fchown failing. */
- || (({asm ("" :: "r" (fchown (newfd, st.st_uid,
- st.st_gid))); }),
- close (newfd) != 0)
- || (newfd = -1, rename (tmpfname, arfname) != 0))
+ goto nonew_unlink;
+ /* Even if the original file had content before the
+ symbol table, we write it in the correct order. */
+ if ((index_off != SARMAG
+ && copy_content (elf, newfd, SARMAG, index_off - SARMAG))
+ || copy_content (elf, newfd, rest_off, st.st_size - rest_off))
+ goto nonew_unlink;
+
+ /* Never complain about fchown failing. */
+ if (fchown (newfd, st.st_uid, st.st_gid) != 0) { ; }
+ /* Set the mode of the new file to the same values the
+ original file has. */
+ if (fchmod (newfd, st.st_mode & ALLPERMS) != 0
+ || close (newfd) != 0)
+ goto nonew_unlink;
+ newfd = -1;
+ if (rename (tmpfname, arfname) != 0)
goto nonew_unlink;
}
}
@@ -1052,12 +1056,15 @@ do_oper_delete (const char *arfname, char **argv, int argc,
}
/* Set the mode of the new file to the same values the original file
- has. */
+ has. Never complain about fchown failing. But do it before
+ setting the mode (which might be reset/ignored if the owner is
+ wrong. */
+ if (fchown (newfd, st.st_uid, st.st_gid) != 0) { ; }
if (fchmod (newfd, st.st_mode & ALLPERMS) != 0
- /* Never complain about fchown failing. */
- || (({asm ("" :: "r" (fchown (newfd, st.st_uid, st.st_gid))); }),
- close (newfd) != 0)
- || (newfd = -1, rename (tmpfname, arfname) != 0))
+ || close (newfd) != 0)
+ goto nonew_unlink;
+ newfd = -1;
+ if (rename (tmpfname, arfname) != 0)
goto nonew_unlink;
errout:
@@ -1534,13 +1541,19 @@ do_oper_insert (int oper, const char *arfname, char **argv, int argc,
/* Set the mode of the new file to the same values the original file
has. */
- if (fd != -1
- && (fchmod (newfd, st.st_mode & ALLPERMS) != 0
- /* Never complain about fchown failing. */
- || (({asm ("" :: "r" (fchown (newfd, st.st_uid, st.st_gid))); }),
- close (newfd) != 0)
- || (newfd = -1, rename (tmpfname, arfname) != 0)))
- goto nonew_unlink;
+ if (fd != -1)
+ {
+ /* Never complain about fchown failing. But do it before
+ setting the modes, or they might be reset/ignored if the
+ owner is wrong. */
+ if (fchown (newfd, st.st_uid, st.st_gid) != 0) { ; }
+ if (fchmod (newfd, st.st_mode & ALLPERMS) != 0
+ || close (newfd) != 0)
+ goto nonew_unlink;
+ newfd = -1;
+ if (rename (tmpfname, arfname) != 0)
+ goto nonew_unlink;
+ }
errout:
for (int cnt = 0; cnt < argc; ++cnt)
diff --git a/src/ranlib.c b/src/ranlib.c
index b9083484..483a1b65 100644
--- a/src/ranlib.c
+++ b/src/ranlib.c
@@ -245,25 +245,31 @@ handle_file (const char *fname)
else
rest_off = SARMAG;
- if ((symtab.symsnamelen != 0
- && ((write_retry (newfd, symtab.symsoff,
- symtab.symsofflen)
- != (ssize_t) symtab.symsofflen)
- || (write_retry (newfd, symtab.symsname,
- symtab.symsnamelen)
- != (ssize_t) symtab.symsnamelen)))
- /* Even if the original file had content before the
- symbol table, we write it in the correct order. */
- || (index_off > SARMAG
- && copy_content (arelf, newfd, SARMAG, index_off - SARMAG))
- || copy_content (arelf, newfd, rest_off, st.st_size - rest_off)
- /* Set the mode of the new file to the same values the
- original file has. */
- || fchmod (newfd, st.st_mode & ALLPERMS) != 0
- /* Never complain about fchown failing. */
- || (({asm ("" :: "r" (fchown (newfd, st.st_uid, st.st_gid))); }),
- close (newfd) != 0)
- || (newfd = -1, rename (tmpfname, fname) != 0))
+ if (symtab.symsnamelen != 0
+ && ((write_retry (newfd, symtab.symsoff,
+ symtab.symsofflen)
+ != (ssize_t) symtab.symsofflen)
+ || (write_retry (newfd, symtab.symsname,
+ symtab.symsnamelen)
+ != (ssize_t) symtab.symsnamelen)))
+ goto nonew_unlink;
+
+ /* Even if the original file had content before the
+ symbol table, we write it in the correct order. */
+ if ((index_off > SARMAG
+ && copy_content (arelf, newfd, SARMAG, index_off - SARMAG))
+ || copy_content (arelf, newfd, rest_off, st.st_size - rest_off))
+ goto nonew_unlink;
+
+ /* Never complain about fchown failing. */
+ if (fchown (newfd, st.st_uid, st.st_gid) != 0) { ; }
+ /* Set the mode of the new file to the same values the
+ original file has. */
+ if (fchmod (newfd, st.st_mode & ALLPERMS) != 0
+ || close (newfd) != 0)
+ goto nonew_unlink;
+ newfd = -1;
+ if (rename (tmpfname, fname) != 0)
goto nonew_unlink;
}
}
--
2.18.4
next prev parent reply other threads:[~2020-06-16 22:26 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-16 22:25 [PATCH 01/10] backends: Make the reloc_nametable zero element an one char array Mark Wielaard
2020-06-16 22:25 ` [PATCH 02/10] libdw: Add missing FALLTHROUGH in execute_cfi Mark Wielaard
2020-06-19 12:56 ` Mark Wielaard
2020-06-16 22:25 ` [PATCH 03/10] elflint: Explicitly check and ignore elf_compress error Mark Wielaard
2020-06-19 13:48 ` Mark Wielaard
2020-06-16 22:25 ` [PATCH 04/10] libdwfl: When we find a compressed image, use that, don't search for others Mark Wielaard
2020-06-19 17:03 ` Mark Wielaard
2020-06-16 22:25 ` [PATCH 05/10] libdwfl: Flag an error if CIE return_address_register is invalid Mark Wielaard
2020-06-19 17:58 ` Mark Wielaard
2020-06-16 22:25 ` Mark Wielaard [this message]
2020-06-19 18:52 ` [PATCH 06/10] ar, ranlib: Call fchown before fchmod, explicitly check and ignore errors Mark Wielaard
2020-06-16 22:25 ` [PATCH 07/10] debuginfod: Handle not being able to fopen interval_path Mark Wielaard
2020-06-24 15:04 ` Mark Wielaard
2020-06-16 22:25 ` [PATCH 08/10] debuginfod: Make sure suffix can place zero terminator when copying filename Mark Wielaard
2020-06-16 22:25 ` [PATCH 09/10] debuginfod: Fix build_id hexadecimal length check Mark Wielaard
2020-06-16 22:25 ` [PATCH 10/10] debuginfod: Make sure handle_data can be allocated and is always freed Mark Wielaard
2020-06-19 11:43 ` [PATCH 01/10] backends: Make the reloc_nametable zero element an one char array Mark Wielaard
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=20200616222539.29109-6-mark@klomp.org \
--to=mark@klomp.org \
--cc=elfutils-devel@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).