Skip to contentSkip to conversation

rpm: Fix patch to ensure variables aren't used uninitialised

PATCH New Peter Kjellerstedt 1 participant 3 messages Jun 9, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 rpm: Fix patch to ensure variables aren't used uninitialised

From: Richard Purdie <[email protected]>

Recent patches were applied which removed a function which set the xx variable. This means xx can be uninitalised and prerm functions can fail.

This adjusts the patch to ensure the xx value isn't used in such a way.

---
 ...t-reset-the-PATH-environment-variable-before-ru.patch | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
meta/recipes-devtools/rpm/files/0001-Do-not-reset-the-PATH-environment-variable-before-ru.patch +13 -3
index a38675f89c..bf24d43eb9 100644
@@ -6,6 +6,9 @@ Subject: [PATCH 1/2] Do not reset the PATH environment variable before running
 
 We add lots of native stuff into it and scriptlets rely on that.
 
+Also need to remove the xx test later in the function since the
+value could now be used un-initialised.
+
 Upstream-Status: Inappropriate [oe-core specific]
 Signed-off-by: Alexander Kanavin <[email protected]>
 ---
@@ -25,6 +28,13 @@ index 92f949fa2..7c1aa75a8 100644
  	free(ipath);
      }
  
--- 
-2.11.0
-
+@@ -206,9 +206,7 @@ static void doScriptExec(ARGV_const_t ar
+ 	/* XXX Don't mtrace into children. */
+ 	unsetenv("MALLOC_CHECK_");
+ 
+-	if (xx == 0) {
+ 	    xx = execv(argv[0], argv);
+-	}
+     }
+     _exit(127); /* exit 127 for compatibility with bash(1) */
+ }
Signature · 1 line
2.12.0
#1 lore raw

PATCH 2/3 Revert "rpm: add a patch to help with Docker performance issues"

This reverts commit 6f1822e5f1eaafd8bc46e999de730c1fcca77f3a.

This patch only solved a part of the problem.

---
 ...FD_CLOEXEC-on-opened-files-before-exec-fr.patch | 49 ----------------------
 meta/recipes-devtools/rpm/rpm_4.14.1.bb            |  1 -
 2 files changed, 50 deletions(-)
 delete mode 100644 meta/recipes-devtools/rpm/files/0001-Revert-Set-FD_CLOEXEC-on-opened-files-before-exec-fr.patch
meta/recipes-devtools/rpm/files/0001-Revert-Set-FD_CLOEXEC-on-opened-files-before-exec-fr.patch deleted +0 -49
deleted file mode 100644
index 4651409a65..0000000000
@@ -1,49 +0,0 @@
-From 982e47df7b82c5ffe3c414cf5641f08dba0f0e64 Mon Sep 17 00:00:00 2001
-From: Alexander Kanavin <[email protected]>
-Date: Fri, 26 Jan 2018 16:32:04 +0200
-Subject: [PATCH] Revert "Set FD_CLOEXEC on opened files before exec from lua
- script is called"
-
-This reverts commit 7a7c31f551ff167f8718aea6d5048f6288d60205.
-The reason is that when _SC_OPEN_MAX is much higher than the usual 1024
-(for example inside docker), the performance drops sharply.
-
-Upstream has been notified:
-https://bugzilla.redhat.com/show_bug.cgi?id=1537564
-
-Upstream-Status: Inappropriate [upstream needs to come up with a better fix]
-Signed-off-by: Alexander Kanavin <[email protected]>
----
- luaext/lposix.c | 12 ------------
- 1 file changed, 12 deletions(-)
-
-diff --git a/luaext/lposix.c b/luaext/lposix.c
-index 0a7c26c71..c578c5a11 100644
---- a/luaext/lposix.c
-+++ b/luaext/lposix.c
-@@ -335,22 +335,10 @@ static int Pexec(lua_State *L)			/** exec(path,[args]) */
- 	const char *path = luaL_checkstring(L, 1);
- 	int i,n=lua_gettop(L);
- 	char **argv;
--	int flag, fdno, open_max;
- 
- 	if (!have_forked)
- 	    return luaL_error(L, "exec not permitted in this context");
- 
--	open_max = sysconf(_SC_OPEN_MAX);
--	if (open_max == -1) {
--	    open_max = 1024;
--	}
--	for (fdno = 3; fdno < open_max; fdno++) {
--	    flag = fcntl(fdno, F_GETFD);
--	    if (flag == -1 || (flag & FD_CLOEXEC))
--		continue;
--	    fcntl(fdno, F_SETFD, FD_CLOEXEC);
--	}
--
- 	argv = malloc((n+1)*sizeof(char*));
- 	if (argv==NULL) return luaL_error(L,"not enough memory");
- 	argv[0] = (char*)path;
--- 
-2.15.1
-
meta/recipes-devtools/rpm/rpm_4.14.1.bb +0 -1
index 818d3ef9e3..85d791ce5c 100644
@@ -39,7 +39,6 @@ SRC_URI = "git://github.com/rpm-software-management/rpm;branch=rpm-4.14.x \
            file://0003-rpmstrpool.c-make-operations-over-string-pools-threa.patch \
            file://0004-build-pack.c-remove-static-local-variables-from-buil.patch \
            file://0001-perl-disable-auto-reqs.patch \
-           file://0001-Revert-Set-FD_CLOEXEC-on-opened-files-before-exec-fr.patch \
            file://0001-configure.ac-add-option-for-dbus.patch \
            "
 
Signature · 1 line
2.12.0
#2 lore raw

PATCH 3/3 rpm: Restore performance in Docker containers

In reply to Peter Kjellerstedt

From: Peter Kjellerstedt <[email protected]>

If the maximum number of open file descriptors is much greater than the usual 1024 (for example inside a Docker container), the performance drops significantly.

This was reported upstream in:
https://bugzilla.redhat.com/show_bug.cgi?id=1537564
which resulted in:
https://github.com/rpm-software-management/rpm/pull/444

The pull request above has now been integrated and this commit contains a backport of its three patches, which together change the behavior of rpm so that its performance is now independent of the maximum number of open file descriptors.

---
 ...0001-Factor-out-and-unify-setting-CLOEXEC.patch | 148 +++++++++++++++++++++
 .../files/0002-Optimize-rpmSetCloseOnExec.patch    | 100 ++++++++++++++
 .../0003-rpmSetCloseOnExec-use-getrlimit.patch     |  53 ++++++++
 meta/recipes-devtools/rpm/rpm_4.14.1.bb            |   3 +
 4 files changed, 304 insertions(+)
 create mode 100644 meta/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch
 create mode 100644 meta/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch
 create mode 100644 meta/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch
meta/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch new file +148 -0
new file mode 100644
index 0000000000..6f440c6178
@@ -0,0 +1,148 @@
+From 9c3e5de3240554c8ea1b29d52eeadee4840fefac Mon Sep 17 00:00:00 2001
+From: Kir Kolyshkin <[email protected]>
+Date: Tue, 29 May 2018 17:37:05 -0700
+Subject: [PATCH 1/3] Factor out and unify setting CLOEXEC
+
+Commit 7a7c31f5 ("Set FD_CLOEXEC on opened files before exec from
+lua script is called") copied the code that sets CLOEXEC flag on all
+possible file descriptors from lib/rpmscript.c to luaext/lposix.c,
+essentially creating two copies of the same code (modulo comments
+and the unused assignment).
+
+This commit moves the functionality into its own function, without
+any code modifications, using the version from luaext/lposix.c.
+
+Signed-off-by: Kir Kolyshkin <[email protected]>
+Upstream-Status: Accepted [https://github.com/rpm-software-management/rpm/pull/444]
+Signed-off-by: Peter Kjellerstedt <[email protected]>
+---
+ lib/rpmscript.c        | 18 ++----------------
+ luaext/lposix.c        | 13 ++-----------
+ rpmio/rpmio.c          | 16 ++++++++++++++++
+ rpmio/rpmio_internal.h |  6 ++++++
+ 4 files changed, 26 insertions(+), 27 deletions(-)
+
+diff --git a/lib/rpmscript.c b/lib/rpmscript.c
+index 747385a5b..b4ccd3246 100644
+--- a/lib/rpmscript.c
++++ b/lib/rpmscript.c
+@@ -3,7 +3,6 @@
+ #include <sys/types.h>
+ #include <sys/wait.h>
+ #include <errno.h>
+-#include <unistd.h>
+ 
+ #include <rpm/rpmfileutil.h>
+ #include <rpm/rpmmacro.h>
+@@ -14,6 +13,7 @@
+ 
+ #include "rpmio/rpmlua.h"
+ #include "lib/rpmscript.h"
++#include "rpmio/rpmio_internal.h"
+ 
+ #include "lib/rpmplugins.h"     /* rpm plugins hooks */
+ 
+@@ -170,26 +170,12 @@ static const char * const SCRIPT_PATH = "PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr
+ static void doScriptExec(ARGV_const_t argv, ARGV_const_t prefixes,
+ 			FD_t scriptFd, FD_t out)
+ {
+-    int flag;
+-    int fdno;
+     int xx;
+-    int open_max;
+ 
+     /* SIGPIPE is ignored in rpm, reset to default for the scriptlet */
+     (void) signal(SIGPIPE, SIG_DFL);
+ 
+-    /* XXX Force FD_CLOEXEC on all inherited fdno's. */
+-    open_max = sysconf(_SC_OPEN_MAX);
+-    if (open_max == -1) {
+-	open_max = 1024;
+-    }
+-    for (fdno = 3; fdno < open_max; fdno++) {
+-	flag = fcntl(fdno, F_GETFD);
+-	if (flag == -1 || (flag & FD_CLOEXEC))
+-	    continue;
+-	xx = fcntl(fdno, F_SETFD, FD_CLOEXEC);
+-	/* XXX W2DO? debug msg for inheirited fdno w/o FD_CLOEXEC */
+-    }
++    rpmSetCloseOnExec();
+ 
+     if (scriptFd != NULL) {
+ 	int sfdno = Fileno(scriptFd);
+diff --git a/luaext/lposix.c b/luaext/lposix.c
+index 0a7c26c71..5d7ad3c87 100644
+--- a/luaext/lposix.c
++++ b/luaext/lposix.c
+@@ -27,6 +27,7 @@
+ #include <unistd.h>
+ #include <utime.h>
+ #include <rpm/rpmutil.h>
++#include "rpmio/rpmio_internal.h"
+ 
+ #define MYNAME		"posix"
+ #define MYVERSION	MYNAME " library for " LUA_VERSION " / Nov 2003"
+@@ -335,21 +336,11 @@ static int Pexec(lua_State *L)			/** exec(path,[args]) */
+ 	const char *path = luaL_checkstring(L, 1);
+ 	int i,n=lua_gettop(L);
+ 	char **argv;
+-	int flag, fdno, open_max;
+ 
+ 	if (!have_forked)
+ 	    return luaL_error(L, "exec not permitted in this context");
+ 
+-	open_max = sysconf(_SC_OPEN_MAX);
+-	if (open_max == -1) {
+-	    open_max = 1024;
+-	}
+-	for (fdno = 3; fdno < open_max; fdno++) {
+-	    flag = fcntl(fdno, F_GETFD);
+-	    if (flag == -1 || (flag & FD_CLOEXEC))
+-		continue;
+-	    fcntl(fdno, F_SETFD, FD_CLOEXEC);
+-	}
++	rpmSetCloseOnExec();
+ 
+ 	argv = malloc((n+1)*sizeof(char*));
+ 	if (argv==NULL) return luaL_error(L,"not enough memory");
+diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c
+index c7cbc32aa..ea111d2ec 100644
+--- a/rpmio/rpmio.c
++++ b/rpmio/rpmio.c
+@@ -1759,3 +1759,19 @@ DIGEST_CTX fdDupDigest(FD_t fd, int id)
+ 
+     return ctx;
+ }
++
++void rpmSetCloseOnExec(void)
++{
++	int flag, fdno, open_max;
++
++	open_max = sysconf(_SC_OPEN_MAX);
++	if (open_max == -1) {
++		open_max = 1024;
++	}
++	for (fdno = 3; fdno < open_max; fdno++) {
++		flag = fcntl(fdno, F_GETFD);
++		if (flag == -1 || (flag & FD_CLOEXEC))
++			continue;
++		fcntl(fdno, F_SETFD, FD_CLOEXEC);
++	}
++}
+diff --git a/rpmio/rpmio_internal.h b/rpmio/rpmio_internal.h
+index fbed183b0..370cbdc75 100644
+--- a/rpmio/rpmio_internal.h
++++ b/rpmio/rpmio_internal.h
+@@ -41,6 +41,12 @@ DIGEST_CTX fdDupDigest(FD_t fd, int id);
+ int rpmioSlurp(const char * fn,
+                 uint8_t ** bp, ssize_t * blenp);
+ 
++/**
++ * Set close-on-exec flag for all opened file descriptors, except
++ * stdin/stdout/stderr.
++ */
++void rpmSetCloseOnExec(void);
++
+ #ifdef __cplusplus
+ }
+ #endif
meta/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch new file +100 -0

100 lines not on this page · raw message

meta/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch new file +53 -0

53 lines not on this page · raw message

meta/recipes-devtools/rpm/rpm_4.14.1.bb +3 -0
index 85d791ce5c..6012922adb 100644
@@ -40,6 +40,9 @@ SRC_URI = "git://github.com/rpm-software-management/rpm;branch=rpm-4.14.x \
            file://0004-build-pack.c-remove-static-local-variables-from-buil.patch \
            file://0001-perl-disable-auto-reqs.patch \
            file://0001-configure.ac-add-option-for-dbus.patch \
+           file://0001-Factor-out-and-unify-setting-CLOEXEC.patch \
+           file://0002-Optimize-rpmSetCloseOnExec.patch \
+           file://0003-rpmSetCloseOnExec-use-getrlimit.patch \
            "
 
 PE = "1"
Signature · 1 line
2.12.0