Guillem Jover wrote:
> This is misleading on Linux, but it's not a safe portable assumption
> to make on POSIX in general as that behaviour is not specified and as
> such is implementation specific
Ok, here's a patch with better behavior. On Linux and similar platforms,
it just opens the file for reading. If fsync fails with errno == EBADF,
it falls back to opening for writing, and if that fails with errno ==
ETXTBSY, it skips the fsync under the assumption that the file has already
been fsynced.
One problem: on the affected platforms (i.e., not Debian), a prankster
could go around executing .dpkg-new files to prevent the fsyncs,
trigger a crash, and then we have filesystem corruption again.
Unpleasant.
To fix that, we should fsync() each file with multiple hard links only
once and then rename all links, so the logic to skip fsync would be no
longer needed.
Untested.
---
debian/changelog | 11 ++++++++
src/archives.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 75 insertions(+), 10 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index 20c296d5..e5de69d4 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -186,6 +186,17 @@ dpkg (1.16.1~jrn) local; urgency=low
* Add new --raw-extract option to dpkg-deb combining --control and
--extract. Closes: #552123
+ [ Jonathan Nieder ]
+ * Defer installing hard links with their final name until the file has
+ been fsynced.
+ * Open extracted files for reading, not writing, in order to fsync them.
+ Otherwise the open can error out when preparing to rename a binary into
+ place that has a hard link already in use. Regression introduced in
+ 1.15.6.1. Closes: #635683
+ * If fsync fails with EBADF, as it is documented to on older platforms
+ when fd is not opened for writing, reopen the file for writing and try
+ again. If that fails with ETXTBSY, just skip the fsync.
+
[ Updated dpkg translations ]
* German (Sven Joachim). Closes: #620312
* Swedish (Peter Krefting).
diff --git a/src/archives.c b/src/archives.c
index f060e706..c221f388 100644
--- a/src/archives.c
+++ b/src/archives.c
@@ -851,7 +851,8 @@ tarobject(void *ctx, struct tar_entry *ti)
* in .dpkg-new.
*/
- if (ti->type == tar_filetype_file || ti->type == tar_filetype_symlink) {
+ if (ti->type == tar_filetype_file || ti->type == tar_filetype_symlink ||
+ ti->type == tar_filetype_hardlink) {
nifd->namenode->flags |= fnnf_deferred_rename;
debug(dbg_eachfiledetail, "tarobject done and installation deferred");
@@ -912,6 +913,67 @@ tar_writeback_barrier(struct fileinlist *files, struct
pkginfo *pkg)
}
#endif
+static void
+deferred_extract_fsync(const char *path)
+{
+ /*
+ * The documentation for some Unixen (e.g., AIX) says fsync
+ * requires a file descriptor open for writing and will
+ * return EBADF otherwise.
+ *
+ * Unfortunately, opening a file for writing fails if that
+ * file happens to be a binary whose text is mapped. That
+ * can happen in two cases:
+ *
+ * A) We are installing a hard link to a binary that was
+ * already installed into place and run. In that case,
+ * we've already done the fsync, so it's safe to skip.
+ *
+ * B) Some obnoxious user decided to run the .dpkg-new
+ * file directly.
+ *
+ * For now let's assume we are in case A and skip the fsync
+ * when errno == ETXTBSY. XXX: It would be better to prevent (A)
+ * higher in the call stack and fall back to sync() here.
+ *
+ * On Linux, we just open the file for reading, fsync does not
+ * return EBADF, and the behavior is as simple as
+ *
+ * fd = open(...);
+ * if (fd < 0)
+ * ohshite(...);
+ * if (fsync(...))
+ * ohshite(...);
+ * if (close(...))
+ * ohshite(...);
+ */
+
+ int fd;
+
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ ohshite(_("unable to open '%.255s'"), path);
+ if (fsync(fd) == 0)
+ goto done;
+ else if (errno != EBADF)
+ ohshite(_("unable to sync file '%.255s'"), path);
+
+ /* Try again with an fd open for writing. */
+ close(fd);
+ fd = open(path, O_WRONLY);
+ if (fd < 0) {
+ if (errno == ETXTBSY)
+ goto done;
+ ohshite(_("unable to open '%.255s'"), path);
+ }
+ if (fsync(fd))
+ ohshite(_("unable to sync file '%.255s'"), path);
+
+done:
+ if (close(fd))
+ ohshite(_("error closing/writing `%.255s'"), path);
+}
+
void
tar_deferred_extract(struct fileinlist *files, struct pkginfo *pkg)
{
@@ -936,15 +998,7 @@ tar_deferred_extract(struct fileinlist *files, struct
pkginfo *pkg)
int fd;
debug(dbg_eachfiledetail, "deferred extract needs fsync");
-
- fd = open(fnamenewvb.buf, O_WRONLY);
- if (fd < 0)
- ohshite(_("unable to open '%.255s'"), fnamenewvb.buf);
- if (fsync(fd))
- ohshite(_("unable to sync file '%.255s'"), fnamenewvb.buf);
- if (close(fd))
- ohshite(_("error closing/writing `%.255s'"), fnamenewvb.buf);
-
+ deferred_extract_fsync(fnamenewvb.buf);
cfile->namenode->flags &= ~fnnf_deferred_fsync;
}
--
1.7.6
--
To UNSUBSCRIBE, email to debian-dpkg-bugs-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
