partial read() bug

[Christoph Pfisterer <cp@chrisp.de>]

Hi!

While porting dpkg 1.8.3.1 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 1.8.3.1, 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.

-chrisp


--- dpkg-1.8.3.1/main/archives.c.orig	Sun May  6 12:07:50 2001
+++ dpkg-1.8.3.1/main/archives.c	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",
 			  nifd->namenode->statoverride->uid,

--
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."