[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

partial read() bug


While porting dpkg to Darwin / Mac OS X, I encountered a sleeping bug in the part of dpkg that reads the tarfile from the dpkg-deb pipe. In two places partial read()'s aren't handled. By partial read I mean calling read(2) for n bytes and getting m bytes back, with 0 < m < n. This is perfectly legal behaviour for read(2).

One place is tarfileread() in main/archives.c, which is used to read header blocks. Here a partial read() results in dpkg stopping processing with an error message. The other place is in tarobject() in the same source file, when skipping the whole-block padding after a file's contents have been read. A partial read() here leads to the next header block failing the checksum test. In most cases, dpkg will falsely detect an end-of-file block, which results in an only partially extracted package and *no error message*.

Below is a patch that fixes this with a wrapper for read() that handles partial reads and interrupted system calls. (See also buffer_read() in lib/mlib.c.) The patch is for dpkg, but a quick look shows that the problem still exists in 1.9.x.

I guess the reason why this has gone unnoticed for so long is that the pipe implementation in Linux tries to avoid partial reads unless it hits end-of-file. Darwin doesn't, and it is likely that the *BSDs also don't.


--- dpkg-	Sun May  6 12:07:50 2001
+++ dpkg-	Sun May  6 12:10:12 2001
@@ -47,6 +47,24 @@
 struct pkginfo *conflictor[20];
 int cflict_index = 0;

+/* special routine to handle partial reads from the tarfile */
+static int safe_read(int fd, void *buf, int len)
+  int r, have = 0;
+  char *p = (char *)buf;
+  while (have < len) {
+    if ((r = read(fd,p,len-have)) == -1) {
+      if (errno == EINTR || errno == EAGAIN) continue;
+      return r;
+    }
+    if (r == 0)
+      break;
+    have += r;
+    p += r;
+  }
+  return have;
 int filesavespackage(struct fileinlist *file, struct pkginfo *pkgtobesaved,
                      struct pkginfo *pkgbeinginstalled) {
   struct pkginfo *divpkg, *thirdpkg;
@@ -118,7 +136,7 @@
 int tarfileread(void *ud, char *buf, int len) {
   struct tarcontext *tc= (struct tarcontext*)ud;
   int r;
-  if ((r= read(tc->backendpipe,buf,len)) == -1)
+  if ((r= safe_read(tc->backendpipe,buf,len)) == -1)
     ohshite(_("error reading from dpkg-deb pipe"));
   return r;
@@ -396,7 +414,7 @@
           (unsigned long)ti->Size);
fd_fd_copy(tc->backendpipe, fd, ti->Size, _("backend dpkg-deb during `%.255s'"),ti->Name);
     r= ti->Size % TARBLKSZ;
-    if (r > 0) r= read(tc->backendpipe,databuf,TARBLKSZ - r);
+    if (r > 0) r= safe_read(tc->backendpipe,databuf,TARBLKSZ - r);
     if (nifd->namenode->statoverride)
debug(dbg_eachfile, "tarobject ... stat override, uid=%d, gid=%d, mode=%04o",

chrisp a.k.a. Christoph Pfisterer   "Any sufficiently advanced
cp@chrisp.de - http://chrisp.de      bug is indistinguishable
PGP key & geek code available        from a feature."

Reply to: