[PATCH] rpm: Fix cpio 32 bit overflow issues on 64 bit inode filesystems

[PATCH] rpm: Fix cpio 32 bit overflow issues on 64 bit inode filesystems

[Richard Purdie]

When building on XFS filesystems, the resulting rpms can be corrupted
with the same inode number being used for multiple hardlinked files. 
There are two fixes, one to stop rpm crashing when accessing a broken
binary rpm, the other to stop generating them in the first places. Full
descriptions in the patch headers.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

diff --git a/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch b/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch
new file mode 100644
index 0000000..d49de6f
--- /dev/null
+++ b/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch
@@ -0,0 +1,43 @@
+We need to sanity check that the nlink size and our linksLeft counter
+do match. If an rpm is badly constucted with identical inode values
+for multiple hardlinked files, such an rpm will overwise access memory
+out of array bounds and cause memory corruption and crashes.
+
+The fix is to add in the sanity check and exit if bad circumstances
+are found. We need to fix the caller to check the return code too.
+
+RP 10/6/1024
+
+Upstream-Status: Pending
+
+Index: rpm-5.4.9/lib/fsm.c
+===================================================================
+--- rpm-5.4.9.orig/lib/fsm.c	2014-06-10 10:54:08.601049402 +0000
++++ rpm-5.4.9/lib/fsm.c	2014-06-10 10:55:45.633046077 +0000
+@@ -495,6 +495,11 @@
+     }
+ 
+     if (fsm->goal == IOSM_PKGBUILD) --fsm->li->linksLeft;
++    if (fsm->li->linksLeft > st->st_nlink) {
++	rpmlog(RPMLOG_ERR, _("Corrupted hardlinks found (count %d does not match %d), exitting.\n"), fsm->li->linksLeft, st->st_nlink);
++	return -1;
++    }
++
+     fsm->li->filex[fsm->li->linksLeft] = fsm->ix;
+     /*@-observertrans -dependenttrans@*/
+     fsm->li->nsuffix[fsm->li->linksLeft] = fsm->nsuffix;
+@@ -1876,8 +1881,13 @@
+ 	fsm->postpone = iosmFileActionSkipped(fsm->action);
+ 	if (fsm->goal == IOSM_PKGINSTALL || fsm->goal == IOSM_PKGBUILD) {
+ 	    /*@-evalorder@*/ /* FIX: saveHardLink can modify fsm */
+-	    if (S_ISREG(st->st_mode) && st->st_nlink > 1)
++	    if (S_ISREG(st->st_mode) && st->st_nlink > 1) {
+ 		fsm->postpone = saveHardLink(fsm);
++		if (fsm->postpone < 0) {
++		    rc = RPMRC_FAIL;
++		    break;
++		}
++	    }
+ 	    /*@=evalorder@*/
+ 	}
+ if (fsmGetFi(fsm)->mapflags & IOSM_PAYLOAD_LIST) fsm->postpone = 1;
diff --git a/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch b/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch
new file mode 100644
index 0000000..f054546
--- /dev/null
+++ b/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch
@@ -0,0 +1,39 @@
+If we run builds on a filesystem with 64 bit inodes like XFS, we need to
+map the inode numbers to something 32 bit since the cpio header only allows
+for 32 bit inode values. If we don't do this:
+
+#define SET_NUM_FIELD(phys, val, space) \
+        sprintf(space, "%8.8lx", (unsigned long) (val)); \
+        memcpy(phys, space, 8)
+
+from cpio.c will print larger that 8 character values and then truncate the 
+LSBs. This generates cpio files where hardlinked files may have the same
+inode number. The resulting rpms are then corrupted.
+
+There is a sperate patch for the crash the identical inode numbers causes
+when extracting the rpm.
+
+Patch taken from http://git.pld-linux.org/?p=packages/rpm.git;a=commitdiff;h=10526c23aac60b7b636e4c93862887dbef8e8f15
+
+RP 10/6/2014
+
+Upstream-Status: Pending
+
+--- rpm-5.4.10/lib/fsm.c~
++++ rpm-5.4.10/lib/fsm.c
+@@ -898,6 +898,7 @@ int fsmMapAttrs(IOSM_t fsm)
+ 
+     if (fi && i >= 0 && i < (int) fi->fc) {
+ 	mode_t perms = (S_ISDIR(st->st_mode) ? fi->dperms : fi->fperms);
++	ino_t finalInode = (fi->finodes ? (ino_t)fi->finodes[i] : 0);
+ 	mode_t finalMode = (fi->fmodes ? (mode_t)fi->fmodes[i] : perms);
+ 	dev_t finalRdev = (dev_t)(fi->frdevs ? fi->frdevs[i] : 0);
+ 	rpmuint32_t finalMtime = (fi->fmtimes ? fi->fmtimes[i] : 0);
+@@ -937,6 +938,7 @@ int fsmMapAttrs(IOSM_t fsm)
+ 	    if ((S_ISCHR(st->st_mode) || S_ISBLK(st->st_mode))
+ 	    && st->st_nlink == 0)
+ 		st->st_nlink = 1;
++	    st->st_ino = finalInode;
+ 	    st->st_rdev = finalRdev;
+ 	    st->st_mtime = finalMtime;
+ 	}
diff --git a/meta/recipes-devtools/rpm/rpm_5.4.9.bb b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
index 43f46ed..6934749 100644
--- a/meta/recipes-devtools/rpm/rpm_5.4.9.bb
+++ b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
@@ -91,6 +91,8 @@ SRC_URI = "http://www.rpm5.org/files/rpm/rpm-5.4/rpm-5.4.9-0.20120508.src.rpm;ex
 	   file://rpm-lsb-compatibility.patch \
 	   file://rpm-tag-generate-endian-conversion-fix.patch \
 	   file://rpm-verify-files.patch \
+	   file://rpm-payload-use-hashed-inode.patch \
+	   file://rpm-hardlink-segfault-fix.patch \
 	  "
 
 # Uncomment the following line to enable platform score debugging

Re: [PATCH] rpm: Fix cpio 32 bit overflow issues on 64 bit inode filesystems

[Mark Hatle]

On 6/10/14, 11:32 AM, Richard Purdie wrote:
> When building on XFS filesystems, the resulting rpms can be corrupted
> with the same inode number being used for multiple hardlinked files.
> There are two fixes, one to stop rpm crashing when accessing a broken
> binary rpm, the other to stop generating them in the first places. Full
> descriptions in the patch headers.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>
> diff --git a/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch b/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch
> new file mode 100644
> index 0000000..d49de6f
> --- /dev/null
> +++ b/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch
> @@ -0,0 +1,43 @@
> +We need to sanity check that the nlink size and our linksLeft counter
> +do match. If an rpm is badly constucted with identical inode values

s/constucted/constructed

> +for multiple hardlinked files, such an rpm will overwise access memory

s/overwise/otherwise

> +out of array bounds and cause memory corruption and crashes.
> +
> +The fix is to add in the sanity check and exit if bad circumstances
> +are found. We need to fix the caller to check the return code too.
> +
> +RP 10/6/1024

2014?

> +Upstream-Status: Pending
> +
> +Index: rpm-5.4.9/lib/fsm.c
> +===================================================================
> +--- rpm-5.4.9.orig/lib/fsm.c	2014-06-10 10:54:08.601049402 +0000
> ++++ rpm-5.4.9/lib/fsm.c	2014-06-10 10:55:45.633046077 +0000
> +@@ -495,6 +495,11 @@
> +     }
> +
> +     if (fsm->goal == IOSM_PKGBUILD) --fsm->li->linksLeft;
> ++    if (fsm->li->linksLeft > st->st_nlink) {
> ++	rpmlog(RPMLOG_ERR, _("Corrupted hardlinks found (count %d does not match %d), exitting.\n"), fsm->li->linksLeft, st->st_nlink);

exiting

> ++	return -1;
> ++    }
> ++
> +     fsm->li->filex[fsm->li->linksLeft] = fsm->ix;
> +     /*@-observertrans -dependenttrans@*/
> +     fsm->li->nsuffix[fsm->li->linksLeft] = fsm->nsuffix;
> +@@ -1876,8 +1881,13 @@
> + 	fsm->postpone = iosmFileActionSkipped(fsm->action);
> + 	if (fsm->goal == IOSM_PKGINSTALL || fsm->goal == IOSM_PKGBUILD) {
> + 	    /*@-evalorder@*/ /* FIX: saveHardLink can modify fsm */
> +-	    if (S_ISREG(st->st_mode) && st->st_nlink > 1)
> ++	    if (S_ISREG(st->st_mode) && st->st_nlink > 1) {
> + 		fsm->postpone = saveHardLink(fsm);
> ++		if (fsm->postpone < 0) {
> ++		    rc = RPMRC_FAIL;
> ++		    break;
> ++		}
> ++	    }
> + 	    /*@=evalorder@*/
> + 	}
> + if (fsmGetFi(fsm)->mapflags & IOSM_PAYLOAD_LIST) fsm->postpone = 1;
> diff --git a/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch b/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch
> new file mode 100644
> index 0000000..f054546
> --- /dev/null
> +++ b/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch
> @@ -0,0 +1,39 @@
> +If we run builds on a filesystem with 64 bit inodes like XFS, we need to
> +map the inode numbers to something 32 bit since the cpio header only allows
> +for 32 bit inode values. If we don't do this:
> +
> +#define SET_NUM_FIELD(phys, val, space) \
> +        sprintf(space, "%8.8lx", (unsigned long) (val)); \
> +        memcpy(phys, space, 8)
> +
> +from cpio.c will print larger that 8 character values and then truncate the
> +LSBs. This generates cpio files where hardlinked files may have the same
> +inode number. The resulting rpms are then corrupted.
> +
> +There is a sperate patch for the crash the identical inode numbers causes

separate

> +when extracting the rpm.
> +
> +Patch taken from http://git.pld-linux.org/?p=packages/rpm.git;a=commitdiff;h=10526c23aac60b7b636e4c93862887dbef8e8f15
> +
> +RP 10/6/2014
> +
> +Upstream-Status: Pending
> +
> +--- rpm-5.4.10/lib/fsm.c~
> ++++ rpm-5.4.10/lib/fsm.c
> +@@ -898,6 +898,7 @@ int fsmMapAttrs(IOSM_t fsm)
> +
> +     if (fi && i >= 0 && i < (int) fi->fc) {
> + 	mode_t perms = (S_ISDIR(st->st_mode) ? fi->dperms : fi->fperms);
> ++	ino_t finalInode = (fi->finodes ? (ino_t)fi->finodes[i] : 0);
> + 	mode_t finalMode = (fi->fmodes ? (mode_t)fi->fmodes[i] : perms);
> + 	dev_t finalRdev = (dev_t)(fi->frdevs ? fi->frdevs[i] : 0);
> + 	rpmuint32_t finalMtime = (fi->fmtimes ? fi->fmtimes[i] : 0);
> +@@ -937,6 +938,7 @@ int fsmMapAttrs(IOSM_t fsm)
> + 	    if ((S_ISCHR(st->st_mode) || S_ISBLK(st->st_mode))
> + 	    && st->st_nlink == 0)
> + 		st->st_nlink = 1;
> ++	    st->st_ino = finalInode;
> + 	    st->st_rdev = finalRdev;
> + 	    st->st_mtime = finalMtime;
> + 	}
> diff --git a/meta/recipes-devtools/rpm/rpm_5.4.9.bb b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
> index 43f46ed..6934749 100644
> --- a/meta/recipes-devtools/rpm/rpm_5.4.9.bb
> +++ b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
> @@ -91,6 +91,8 @@ SRC_URI = "http://www.rpm5.org/files/rpm/rpm-5.4/rpm-5.4.9-0.20120508.src.rpm;ex
>   	   file://rpm-lsb-compatibility.patch \
>   	   file://rpm-tag-generate-endian-conversion-fix.patch \
>   	   file://rpm-verify-files.patch \
> +	   file://rpm-payload-use-hashed-inode.patch \
> +	   file://rpm-hardlink-segfault-fix.patch \
>   	  "
>
>   # Uncomment the following line to enable platform score debugging
>
>

Re: [PATCH] rpm: Fix cpio 32 bit overflow issues on 64 bit inode filesystems

[Gary Thomas]

On 2014-06-10 10:37, Mark Hatle wrote:
> On 6/10/14, 11:32 AM, Richard Purdie wrote:
>> When building on XFS filesystems, the resulting rpms can be corrupted
>> with the same inode number being used for multiple hardlinked files.
>> There are two fixes, one to stop rpm crashing when accessing a broken
>> binary rpm, the other to stop generating them in the first places. Full
>> descriptions in the patch headers.
>>
>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>>
>> diff --git a/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch b/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch
>> new file mode 100644
>> index 0000000..d49de6f
>> --- /dev/null
>> +++ b/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch
>> @@ -0,0 +1,43 @@
>> +We need to sanity check that the nlink size and our linksLeft counter
>> +do match. If an rpm is badly constucted with identical inode values
>
> s/constucted/constructed
>
>> +for multiple hardlinked files, such an rpm will overwise access memory
>
> s/overwise/otherwise
>
>> +out of array bounds and cause memory corruption and crashes.
>> +
>> +The fix is to add in the sanity check and exit if bad circumstances
>> +are found. We need to fix the caller to check the return code too.
>> +
>> +RP 10/6/1024
>
> 2014?

Perhaps even an ISO date (2014-06-10) since that's what's used everywhere
else (and it's not October yet, at least not on this side of the pond)

>
>> +Upstream-Status: Pending
>> +
>> +Index: rpm-5.4.9/lib/fsm.c
>> +===================================================================
>> +--- rpm-5.4.9.orig/lib/fsm.c    2014-06-10 10:54:08.601049402 +0000
>> ++++ rpm-5.4.9/lib/fsm.c    2014-06-10 10:55:45.633046077 +0000
>> +@@ -495,6 +495,11 @@
>> +     }
>> +
>> +     if (fsm->goal == IOSM_PKGBUILD) --fsm->li->linksLeft;
>> ++    if (fsm->li->linksLeft > st->st_nlink) {
>> ++    rpmlog(RPMLOG_ERR, _("Corrupted hardlinks found (count %d does not match %d), exitting.\n"), fsm->li->linksLeft, st->st_nlink);
>
> exiting
>
>> ++    return -1;
>> ++    }
>> ++
>> +     fsm->li->filex[fsm->li->linksLeft] = fsm->ix;
>> +     /*@-observertrans -dependenttrans@*/
>> +     fsm->li->nsuffix[fsm->li->linksLeft] = fsm->nsuffix;
>> +@@ -1876,8 +1881,13 @@
>> +     fsm->postpone = iosmFileActionSkipped(fsm->action);
>> +     if (fsm->goal == IOSM_PKGINSTALL || fsm->goal == IOSM_PKGBUILD) {
>> +         /*@-evalorder@*/ /* FIX: saveHardLink can modify fsm */
>> +-        if (S_ISREG(st->st_mode) && st->st_nlink > 1)
>> ++        if (S_ISREG(st->st_mode) && st->st_nlink > 1) {
>> +         fsm->postpone = saveHardLink(fsm);
>> ++        if (fsm->postpone < 0) {
>> ++            rc = RPMRC_FAIL;
>> ++            break;
>> ++        }
>> ++        }
>> +         /*@=evalorder@*/
>> +     }
>> + if (fsmGetFi(fsm)->mapflags & IOSM_PAYLOAD_LIST) fsm->postpone = 1;
>> diff --git a/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch b/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch
>> new file mode 100644
>> index 0000000..f054546
>> --- /dev/null
>> +++ b/meta/recipes-devtools/rpm/rpm/rpm-payload-use-hashed-inode.patch
>> @@ -0,0 +1,39 @@
>> +If we run builds on a filesystem with 64 bit inodes like XFS, we need to
>> +map the inode numbers to something 32 bit since the cpio header only allows
>> +for 32 bit inode values. If we don't do this:
>> +
>> +#define SET_NUM_FIELD(phys, val, space) \
>> +        sprintf(space, "%8.8lx", (unsigned long) (val)); \
>> +        memcpy(phys, space, 8)
>> +
>> +from cpio.c will print larger that 8 character values and then truncate the
>> +LSBs. This generates cpio files where hardlinked files may have the same
>> +inode number. The resulting rpms are then corrupted.
>> +
>> +There is a sperate patch for the crash the identical inode numbers causes
>
> separate
>
>> +when extracting the rpm.
>> +
>> +Patch taken from http://git.pld-linux.org/?p=packages/rpm.git;a=commitdiff;h=10526c23aac60b7b636e4c93862887dbef8e8f15
>> +
>> +RP 10/6/2014
>> +
>> +Upstream-Status: Pending
>> +
>> +--- rpm-5.4.10/lib/fsm.c~
>> ++++ rpm-5.4.10/lib/fsm.c
>> +@@ -898,6 +898,7 @@ int fsmMapAttrs(IOSM_t fsm)
>> +
>> +     if (fi && i >= 0 && i < (int) fi->fc) {
>> +     mode_t perms = (S_ISDIR(st->st_mode) ? fi->dperms : fi->fperms);
>> ++    ino_t finalInode = (fi->finodes ? (ino_t)fi->finodes[i] : 0);
>> +     mode_t finalMode = (fi->fmodes ? (mode_t)fi->fmodes[i] : perms);
>> +     dev_t finalRdev = (dev_t)(fi->frdevs ? fi->frdevs[i] : 0);
>> +     rpmuint32_t finalMtime = (fi->fmtimes ? fi->fmtimes[i] : 0);
>> +@@ -937,6 +938,7 @@ int fsmMapAttrs(IOSM_t fsm)
>> +         if ((S_ISCHR(st->st_mode) || S_ISBLK(st->st_mode))
>> +         && st->st_nlink == 0)
>> +         st->st_nlink = 1;
>> ++        st->st_ino = finalInode;
>> +         st->st_rdev = finalRdev;
>> +         st->st_mtime = finalMtime;
>> +     }
>> diff --git a/meta/recipes-devtools/rpm/rpm_5.4.9.bb b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
>> index 43f46ed..6934749 100644
>> --- a/meta/recipes-devtools/rpm/rpm_5.4.9.bb
>> +++ b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
>> @@ -91,6 +91,8 @@ SRC_URI = "http://www.rpm5.org/files/rpm/rpm-5.4/rpm-5.4.9-0.20120508.src.rpm;ex
>>          file://rpm-lsb-compatibility.patch \
>>          file://rpm-tag-generate-endian-conversion-fix.patch \
>>          file://rpm-verify-files.patch \
>> +       file://rpm-payload-use-hashed-inode.patch \
>> +       file://rpm-hardlink-segfault-fix.patch \
>>         "
>>
>>   # Uncomment the following line to enable platform score debugging
>>
>>
>

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: [PATCH] rpm: Fix cpio 32 bit overflow issues on 64 bit inode filesystems

[Richard Purdie]

On Tue, 2014-06-10 at 10:42 -0600, Gary Thomas wrote:
> On 2014-06-10 10:37, Mark Hatle wrote:
> > On 6/10/14, 11:32 AM, Richard Purdie wrote:
> >> When building on XFS filesystems, the resulting rpms can be corrupted
> >> with the same inode number being used for multiple hardlinked files.
> >> There are two fixes, one to stop rpm crashing when accessing a broken
> >> binary rpm, the other to stop generating them in the first places. Full
> >> descriptions in the patch headers.
> >>
> >> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> >>
> >> diff --git a/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch b/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch
> >> new file mode 100644
> >> index 0000000..d49de6f
> >> --- /dev/null
> >> +++ b/meta/recipes-devtools/rpm/rpm/rpm-hardlink-segfault-fix.patch
> >> @@ -0,0 +1,43 @@
> >> +We need to sanity check that the nlink size and our linksLeft counter
> >> +do match. If an rpm is badly constucted with identical inode values
> >
> > s/constucted/constructed
> >
> >> +for multiple hardlinked files, such an rpm will overwise access memory
> >
> > s/overwise/otherwise
> >
> >> +out of array bounds and cause memory corruption and crashes.
> >> +
> >> +The fix is to add in the sanity check and exit if bad circumstances
> >> +are found. We need to fix the caller to check the return code too.
> >> +
> >> +RP 10/6/1024
> >
> > 2014?
> 
> Perhaps even an ISO date (2014-06-10) since that's what's used everywhere
> else (and it's not October yet, at least not on this side of the pond)

I do try and use ISO dates in patches after a comment from you a while
back. On this occasion lets just say it wasn't the top thing on my mind
(nor was spelling, clearly :/).

I've been staring at debugging this issue for 48 hours and there is some
pressure on for getting it fixed and several things unblocked. I wasn't
feeling particularly well when I started and am not much better now so
let me just say thanks for the fixes and leave this there :)

Cheers,

Richard