PR#14: Fix two issues in RPM CoW - rpms/rpm

rpms / rpm

#14 Fix two issues in RPM CoW

Merged 8 months ago by chantra. Opened 8 months ago by richardphibel.

rpms/ richardphibel/rpm c8s-sig-hyperscale into c8s-sig-hyperscale

Richard Phibel • 8 months ago

51dac1

SOURCES/0033-rpmcow-fix-stack-overflow-in-rpm2extents.patch

file added

+80

		`@@ -0,0 +1,80 @@`
		`+ From 937f9bc67b905851c78719d8397926eaa97b174a Mon Sep 17 00:00:00 2001`
		`+ From: Richard Phibel <richardphibel@meta.com>`
		`+ Date: Mon, 22 May 2023 05:16:51 +0200`
		`+ Subject: [PATCH] Fix stack overflow`
		`+`
		`+ Creation of array struct digestoffset offsets[rpmfiFC(fi)] caused a`
		`+ stack overflow because the total size is greater than 8M which is the`
		`+ stack size limit on Linux. To fix the issue, the array is allocated on`
		`+ the heap.`
		`+`
		`+ I used AddressSanitizer to find the root cause of the issue. It found a`
		`+ number of memory leaks so I fixed them as well.`
		`+ ---`
		`+ rpm2extents.c \| 15 +++++++++++----`
		`+ 1 file changed, 11 insertions(+), 4 deletions(-)`
		`+`
		`+ diff --git a/rpm2extents.c b/rpm2extents.c`
		`+ index c2a373914..0ee8666fa 100644`
		`+ --- a/rpm2extents.c`
		`+ +++ b/rpm2extents.c`
		`+ @@ -226,6 +226,7 @@ exit:`
		`+ if(msg) {`
		`+ free(msg);`
		`+ }`
		`+ + rpmtsFree(ts);`
		`+ return rc;`
		`+ }`
		`+`
		`+ @@ -243,6 +244,7 @@ static void sanitizeSignatureHeader(Header * sigh)`
		`+ *sigh = headerLink(nh);`
		`+ headerFree(nh);`
		`+ }`
		`+ + rpmtdFreeData(&td);`
		`+ }`
		`+`
		`+ static rpmRC process_package(FD_t fdi, FD_t digestori, FD_t validationi)`
		`+ @@ -281,6 +283,8 @@ static rpmRC process_package(FD_t fdi, FD_t digestori, FD_t validationi)`
		`+ rpmfiles files = NULL;`
		`+ rpmfi fi = NULL;`
		`+ char *msg = NULL;`
		`+ + struct digestoffset *offsets = NULL;`
		`+ + digestSet ds = NULL;`
		`+`
		`+ fdo = fdDup(STDOUT_FILENO);`
		`+`
		`+ @@ -357,10 +361,8 @@ static rpmRC process_package(FD_t fdi, FD_t digestori, FD_t validationi)`
		`+ * now?)`
		`+ */`
		`+ diglen = (uint32_t) rpmDigestLength(rpmfiDigestAlgo(fi));`
		`+ - digestSet ds =`
		`+ - digestSetCreate(rpmfiFC(fi), digestSetHash, digestSetCmp,`
		`+ - NULL);`
		`+ - struct digestoffset offsets[rpmfiFC(fi)];`
		`+ + ds = digestSetCreate(rpmfiFC(fi), digestSetHash, digestSetCmp, NULL);`
		`+ + offsets = xcalloc(rpmfiFC(fi), sizeof(*offsets));`
		`+ pos = RPMLEAD_SIZE + headerSizeof(sigh, HEADER_MAGIC_YES);`
		`+`
		`+ /* main headers are aligned to 8 byte boundry */`
		`+ @@ -494,6 +496,10 @@ static rpmRC process_package(FD_t fdi, FD_t digestori, FD_t validationi)`
		`+ rpmfilesFree(files);`
		`+ rpmfiFree(fi);`
		`+ headerFree(h);`
		`+ + headerFree(sigh);`
		`+ + free(offsets);`
		`+ + Fclose(fdo);`
		`+ + digestSetFree(ds);`
		`+ return rc;`
		`+ }`
		`+`
		`+ @@ -693,6 +699,7 @@ int main(int argc, char *argv[]) {`
		`+`
		`+ FD_t fdi = fdDup(STDIN_FILENO);`
		`+ rc = teeRpm(fdi, algos, nb_algos);`
		`+ + Fclose(fdi);`
		`+ if (rc != RPMRC_OK) {`
		`+ /* translate rpmRC into generic failure return code. */`
		`+ return EXIT_FAILURE;`
		`+ --`
		`+ 2.40.1`
		`+`

SOURCES/0034-rpmcow-fix-issue-for-transaction-with-transcoded-and-untranscoded-packages.patch

file added

+28

		`@@ -0,0 +1,28 @@`
		`+ From a3b6102b4d2e79a8b74b036c6a29272a7f6e5c6a Mon Sep 17 00:00:00 2001`
		`+ From: Richard Phibel <richardphibel@meta.com>`
		`+ Date: Fri, 11 Aug 2023 00:43:21 +0200`
		`+ Subject: [PATCH] Fix issue for transaction with transcoded and non-transcoded`
		`+ packages`
		`+`
		`+ The flag saying whether a package is transcoded is not clean-up between`
		`+ each packages. Because of that if a non-transcoded package is treated`
		`+ after a transcoded one, the package is treated as transcoded`
		`+ ---`
		`+ plugins/reflink.c \| 1 +`
		`+ 1 file changed, 1 insertion(+)`
		`+`
		`+ diff --git a/plugins/reflink.c b/plugins/reflink.c`
		`+ index 986cbd172..20d35eefd 100644`
		`+ --- a/plugins/reflink.c`
		`+ +++ b/plugins/reflink.c`
		`+ @@ -234,6 +234,7 @@ static rpmRC reflink_psm_post(rpmPlugin plugin, rpmte te, int res)`
		`+ inodeIndexHashFree(state->inodeIndexes);`
		`+ state->inodeIndexes = NULL;`
		`+ }`
		`+ + state->transcoded = 0;`
		`+ return RPMRC_OK;`
		`+ }`
		`+`
		`+ --`
		`+ 2.40.1`
		`+`

SPECS/rpm.spec

file modified

+7 -1

		`@@ -42,7 +42,7 @@`

		`%global rpmver 4.14.3`
		`#global snapver rc2`
		`- %global rel 26.1`
		`+ %global rel 26.2`

		`%global srcver %{version}%{?snapver:-%{snapver}}`
		`%global srcdir %{?snapver:testing}%{!?snapver:%{name}-%(echo %{version} \| cut -d'.' -f1-2).x}`
		`@@ -246,6 +246,8 @@`
		`Patch9930: 0030-rpmcow-Make-rpm-i-install-package-without-the-need-o.patch`
		`Patch9931: 0031-rpmcow-denylist.patch`
		`Patch9932: 0032-rpmcow-workaround.patch`
		`+ Patch9933: 0033-rpmcow-fix-stack-overflow-in-rpm2extents.patch`
		`+ Patch9934: 0034-rpmcow-fix-issue-for-transaction-with-transcoded-and-untranscoded-packages.patch`
		`Provides: rpm(pr1470)`
		`Provides: rpm(pr1470_1)`

		`@@ -843,6 +845,10 @@`
		`%doc doc/librpm/html/*`

		`%changelog`
		`+ * Thu Aug 17 2023 Richard Phibel <richardphibel@meta.com> - 4.14.3-26.2`
		`+ - Fix issue for transaction with transcoded and non-transcoded packages`
		`+ - Fix stack overflow in rpm2extents and various memory leaks`
		`+`
		`* Wed Dec 21 2022 Davide Cavalca <dcavalca@centosproject.org> - 4.14.3-26.1`
		`- Merge upstream changes for Hyperscale`

richardphibel commented 8 months ago

Fix for mix transcoded/untranscoded transactions
The flag saying whether a package is transcoded is not clean-up between
each packages. Because of that if a non-transcoded package is treated
after a transcoded one, the package is treated as transcoded
Fix stack overflow in rpm2extents
Creation of array struct digestoffset offsets[rpmfiFC(fi)] caused a
stack overflow because the total size is greater than 8M which is the
stack size limit on Linux. To fix the issue, the array is allocated on
the heap.
I used AddressSanitizer to find the root cause of the issue. It found a
number of memory leaks so I fixed them as well.