Package: dpkg
Version: 1.14.5
The attached patch fixes various error-handling bugs in dpkg. Most of
them are minor but there is one particularly important bug which is
the erroneous repeated closure of the fsys tarfile pipe fd, which
in principle could cause arbitrary weirdness.
I suggest that this patch should be applied to sid's dpkg ASAP. I
have merged it into my triggers branch (and published at the previous
git URL) and will merge it into my flex parser branch shortly.
Ian.
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/debian/changelog
dpkg-1.14.5ubuntu12/debian/changelog
--- old/dpkg-1.14.5ubuntu14/debian/changelog 2007-09-18 17:21:02.000000000
+0100
+++ dpkg-1.14.5ubuntu12/debian/changelog 2007-09-20 18:12:26.000000000
+0100
@@ -1,3 +1,24 @@
+dpkg (1.14.5ubuntu15) gutsy; urgency=low
+
+ * Bugfixes to fd cleanup handling:
+ - avoid closing fsys tarfile pipe twice even in normal
+ operation - normally EBADF but might sometimes close some other
+ desired fd and cause hideous doom. (LP: #137191.)
+ - avoid duplicate attempts to [f]close in obscure error
+ situations which might conceiveably close wrong fds
+ - cast &fd to void* when passing to push_cleanup cu_closefd
+ - fix parse.c:parsedb to use ehflag_normaltidy in a sane way
+ - when passing &fd to push_cleanup cu_closefd, make fd always static
+ * Bugfix in trigger deferred file processing: reset lexer start state
+ when calling yyrestart (has no effect except after parsing/reading
+ errors in the deferred file).
+ * Fix some error handling bugs in tarfn.c:
+ - Avoid freeing uninitialised h.[Link]Name (can cause crash if .deb
+ becomes unreadable while we start up). (LP: #138887.)
+ - Use m_malloc instead of malloc (and ditch ad-hoc error handling).
+
+ -- Ian Jackson <[EMAIL PROTECTED]> Thu, 20 Sep 2007 18:12:20 +0100
+
dpkg (1.14.5ubuntu14) gutsy; urgency=low
* Change syntax of `processing:...' status fd outputs so
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/lib/dpkg.h
dpkg-1.14.5ubuntu12/lib/dpkg.h
--- old/dpkg-1.14.5ubuntu14/lib/dpkg.h 2007-09-18 12:13:17.000000000 +0100
+++ dpkg-1.14.5ubuntu12/lib/dpkg.h 2007-09-20 16:52:46.000000000 +0100
@@ -236,6 +236,16 @@
void cu_closefd(int argc, void **argv);
void cu_closedir(int argc, void **argv);
+int ferror_fclose_pop_cleanup(FILE *f);
+ /* calls ferror and fclose on f, and pop_cleanup
+ * file= fopen
+ * push_cleanup(cu_closefile,~ehflag_normaltidy, 0,0, 1,(void*)file
+ * return is 0 on success or EOF if fclose or ferror failed
+ * all three calls are always made regardless, and in the right order
+ * so you can just call ohshite if this returns EOF
+ */
+
+
/*** from mlib.c ***/
void setcloexec(int fd, const char* fn);
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/lib/ehandle.c
dpkg-1.14.5ubuntu12/lib/ehandle.c
--- old/dpkg-1.14.5ubuntu14/lib/ehandle.c 2007-09-18 12:13:49.000000000
+0100
+++ dpkg-1.14.5ubuntu12/lib/ehandle.c 2007-09-20 17:03:31.000000000 +0100
@@ -330,6 +330,14 @@
if (f) fclose(f);
}
+int ferror_fclose_pop_cleanup(FILE *f) {
+ int r1, r2;
+ r1= ferror(f);
+ pop_cleanup(ehflag_normaltidy);
+ r2= fclose(f);
+ return r1 ? r1 : r2;
+}
+
void cu_closedir(int argc, void **argv) {
DIR *d= (DIR*)(argv[0]);
if (d) closedir(d);
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/lib/parse.c
dpkg-1.14.5ubuntu12/lib/parse.c
--- old/dpkg-1.14.5ubuntu14/lib/parse.c 2007-08-16 00:44:00.000000000 +0100
+++ dpkg-1.14.5ubuntu12/lib/parse.c 2007-09-20 17:19:53.000000000 +0100
@@ -87,7 +87,7 @@
* If donep is not null only one package's information is expected.
*/
- int fd;
+ static int fd;
struct pkginfo newpig, *pigp;
struct pkginfoperfile *newpifp, *pifp;
struct arbitraryfield *arp, **larpp;
@@ -109,7 +109,7 @@
fd= open(filename, O_RDONLY);
if (fd == -1) ohshite(_("failed to open package info file `%.255s' for
reading"),filename);
- push_cleanup(cu_parsedb,~0, NULL,0, 1,&fd);
+ push_cleanup(cu_parsedb,~ehflag_normaltidy, NULL,0, 1,&fd);
if (fstat(fd, &stat) == -1)
ohshite(_("can't stat package info file `%.255s'"),filename);
@@ -355,7 +355,6 @@
if (EOF_mmap(dataptr, endptr)) break;
if (c == '\n') lno++;
}
- pop_cleanup(0);
if (data != NULL) {
#ifdef HAVE_MMAP
munmap(data, stat.st_size);
@@ -364,6 +363,7 @@
#endif
}
free(value);
+ pop_cleanup(ehflag_normaltidy);
if (close(fd)) ohshite(_("failed to close after read: `%.255s'"),filename);
if (donep && !pdone) ohshit(_("no package information in
`%.255s'"),filename);
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/lib/tarfn.c
dpkg-1.14.5ubuntu12/lib/tarfn.c
--- old/dpkg-1.14.5ubuntu14/lib/tarfn.c 2007-07-04 15:26:32.000000000 +0100
+++ dpkg-1.14.5ubuntu12/lib/tarfn.c 2007-09-20 18:06:29.000000000 +0100
@@ -61,7 +61,7 @@
char * str;
len = strnlen(s, size);
- str = malloc(len + 1);
+ str = m_malloc(len + 1);
memcpy(str, s, len);
str[len] = 0;
@@ -137,9 +137,11 @@
next_long_name = NULL;
next_long_link = NULL;
long_read = 0;
- symListBottom = symListPointer = symListTop = malloc(sizeof(symlinkList));
+ symListBottom = symListPointer = symListTop = m_malloc(sizeof(symlinkList));
symListTop->next = NULL;
+ h.Name = NULL;
+ h.LinkName = NULL;
h.UserData = userData;
while ( (status = functions->Read(userData, buffer, 512)) == 512 ) {
@@ -206,13 +208,7 @@
errno = 0;
break;
}
- if ((symListBottom->next = malloc(sizeof(symlinkList))) == NULL) {
- free(symListBottom->h.LinkName);
- free(symListBottom->h.Name);
- status = -1;
- errno = 0;
- break;
- }
+ symListBottom->next = m_malloc(sizeof(symlinkList));
symListBottom = symListBottom->next;
symListBottom->next = NULL;
status = 0;
@@ -233,12 +229,7 @@
if (*longp)
free(*longp);
- if (NULL == (*longp = (char *)malloc(h.Size))) {
- /* malloc failed, so bail */
- errno = 0;
- status = -1;
- break;
- }
+ *longp = (char *)m_malloc(h.Size);
bp = *longp;
// the way the GNU long{link,name} stuff works is like this:
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/lib/trigdeferred.l
dpkg-1.14.5ubuntu12/lib/trigdeferred.l
--- old/dpkg-1.14.5ubuntu14/lib/trigdeferred.l 2007-08-21 15:22:28.000000000
+0100
+++ dpkg-1.14.5ubuntu12/lib/trigdeferred.l 2007-09-20 15:18:27.000000000
+0100
@@ -142,6 +142,7 @@
return 1;
trigdef_yyrestart(old_deferred);
+ BEGIN(0);
return 2;
}
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/lib/triglib.c
dpkg-1.14.5ubuntu12/lib/triglib.c
--- old/dpkg-1.14.5ubuntu14/lib/triglib.c 2007-08-16 16:52:53.000000000
+0100
+++ dpkg-1.14.5ubuntu12/lib/triglib.c 2007-09-20 17:38:53.000000000 +0100
@@ -296,9 +296,8 @@
}
if (signum > 0)
fprintf(nf,"%s\n",pkg->name);
- if (ferror(nf) || fclose(nf))
+ if (ferror_fclose_pop_cleanup(nf))
ohshite(_("unable to write new trigger interest file `%.250s'"),newfn.buf);
- pop_cleanup(ehflag_normaltidy);
if (rename(newfn.buf,trk_explicit_fn.buf))
ohshite(_("unable to install new trigger interest file `%.250s'"),
@@ -369,10 +368,9 @@
for (tfi=filetriggers.head; tfi; tfi=tfi->inoverall.next)
fprintf(nf, "%s %s\n", trigh.namenode_name(tfi->fnn), tfi->pkg->name);
- if (ferror(nf) || fclose(nf))
+ if (ferror_fclose_pop_cleanup(nf))
ohshite(_("unable to write new file triggers file `%.250s'"),
triggersnewfilefile);
- pop_cleanup(ehflag_normaltidy);
if (rename(triggersnewfilefile, triggersfilefile))
ohshite(_("unable to install new file triggers file as `%.250s'"),
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/src/archives.c
dpkg-1.14.5ubuntu12/src/archives.c
--- old/dpkg-1.14.5ubuntu14/src/archives.c 2007-09-18 12:17:35.000000000
+0100
+++ dpkg-1.14.5ubuntu12/src/archives.c 2007-09-20 16:56:46.000000000 +0100
@@ -367,11 +367,12 @@
int tarobject(struct TarInfo *ti) {
static struct varbuf conffderefn, hardlinkfn, symlinkfn;
+ static int fd;
const char *usename;
struct conffile *conff;
struct tarcontext *tc= (struct tarcontext*)ti->UserData;
- int statr, fd, i, existingdirectory, keepexisting;
+ int statr, i, existingdirectory, keepexisting;
size_t r;
struct stat stab, stabtmp;
char databuf[TARBLKSZ];
@@ -641,7 +642,7 @@
*/
fd= open(fnamenewvb.buf, (O_CREAT|O_EXCL|O_WRONLY), 0);
if (fd < 0) ohshite(_("unable to create `%.255s'"),ti->Name);
- push_cleanup(cu_closefd,ehflag_bombout, 0,0, 1,&fd);
+ push_cleanup(cu_closefd,ehflag_bombout, 0,0, 1,(void*)&fd);
debug(dbg_eachfiledetail,"tarobject NormalFile[01] open size=%lu",
(unsigned long)ti->Size);
{ char fnamebuf[256];
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/src/configure.c
dpkg-1.14.5ubuntu12/src/configure.c
--- old/dpkg-1.14.5ubuntu14/src/configure.c 2007-09-18 12:10:00.000000000
+0100
+++ dpkg-1.14.5ubuntu12/src/configure.c 2007-09-20 16:59:11.000000000 +0100
@@ -425,7 +425,7 @@
fd=open(fn,O_RDONLY);
if (fd>=0) {
- push_cleanup(cu_closefd,ehflag_bombout, 0,0, 1,&fd);
+ push_cleanup(cu_closefd,ehflag_bombout, 0,0, 1,(void*)&fd);
fd_md5(fd, hashbuf, -1, _("md5hash"));
pop_cleanup(ehflag_normaltidy); /* fd= open(cdr.buf) */
close(fd);
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/src/filesdb.c
dpkg-1.14.5ubuntu12/src/filesdb.c
--- old/dpkg-1.14.5ubuntu14/src/filesdb.c 2007-08-16 00:44:00.000000000
+0100
+++ dpkg-1.14.5ubuntu12/src/filesdb.c 2007-09-20 16:59:11.000000000 +0100
@@ -133,7 +133,7 @@
return;
}
- push_cleanup(cu_closefd,ehflag_bombout, 0,0, 1,&fd);
+ push_cleanup(cu_closefd,ehflag_bombout, 0,0, 1,(void*)&fd);
if(fstat(fd, &stat_buf))
ohshite("unable to stat files list file for package `%.250s'",pkg->name);
diff --exclude='*~' -ru old/dpkg-1.14.5ubuntu14/src/processarc.c
dpkg-1.14.5ubuntu12/src/processarc.c
--- old/dpkg-1.14.5ubuntu14/src/processarc.c 2007-09-18 11:35:37.000000000
+0100
+++ dpkg-1.14.5ubuntu12/src/processarc.c 2007-09-20 15:46:48.000000000
+0100
@@ -562,13 +562,12 @@
execlp(BACKEND, BACKEND, "--fsys-tarfile", filename, (char*)0);
ohshite(_("unable to exec dpkg-deb to get filesystem archive"));
}
- close(p1[1]);
+ close(p1[1]); p1[1]= -1;
newfileslist= 0; tc.newfilesp= &newfileslist;
push_cleanup(cu_fileslist,~0, 0, 0, 0);
tc.pkg= pkg;
tc.backendpipe= p1[0];
- push_cleanup(cu_closefd,~ehflag_bombout, 0,0, 1,&tc.backendpipe);
r= TarExtractor((void*)&tc, &tf);
if (r) {
@@ -578,8 +577,8 @@
ohshit(_("corrupted filesystem tarfile - corrupted package archive"));
}
}
- fd_null_copy(tc.backendpipe,-1,_("dpkg-deb: zap possible trailing zeros"));
- close(tc.backendpipe);
+ fd_null_copy(p1[0],-1,_("dpkg-deb: zap possible trailing zeros"));
+ close(p1[0]); p1[0]= -1;
waitsubproc(c1,BACKEND " --fsys-tarfile",PROCPIPE);
if (oldversionstatus == stat_halfinstalled || oldversionstatus ==
stat_unpacked) {
