On 11/09/2011 12:46 PM, Eric Blake wrote:
If /tmp is mounted with the noexec flag (common on security-conscious
systems), then nwfilter will fail to initialize, because we cannot
run any temporary script via virRun("/tmp/script"); but we _can_
use "/bin/sh /tmp/script". For that matter, using /tmp risks collisions
with other unrelated programs; we already have /var/run/libvirt as a
dedicated temporary directory for use by libvirt.
* src/nwfilter/nwfilter_ebiptables_driver.c
(ebiptablesWriteToTempFile): Use internal directory, not /tmp;
drop attempts to make script executable; and detect close error.
(ebiptablesExecCLI): Switch to virCommand, and invoke the shell to
read the script, rather than requiring an executable script.
---
src/nwfilter/nwfilter_ebiptables_driver.c | 76 ++++++++---------------------
1 files changed, 21 insertions(+), 55 deletions(-)
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index f87cfa1..c9c194c 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -40,6 +40,7 @@
#include "nwfilter_ebiptables_driver.h"
#include "virfile.h"
#include "command.h"
+#include "configmake.h"
#define VIR_FROM_THIS VIR_FROM_NWFILTER
@@ -2482,29 +2483,17 @@ ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED,
* NULL in case of error with the error reported.
*
* Write the string into a temporary file and return the name of
- * the temporary file. The string is assumed to contain executable
- * commands. A line '#!/bin/sh' will automatically be written
- * as the first line in the file. The permissions of the file are
- * set so that the file can be run as an executable script.
+ * the temporary file. The file can then be read as a /bin/sh script.
+ * No '#!/bin/sh' header is needed, since the file will be read and not
+ * directly executed.
*/
static char *
ebiptablesWriteToTempFile(const char *string) {
- char filename[] = "/tmp/virtdXXXXXX";
- int len;
+ char filename[] = LOCALSTATEDIR "/run/libvirt/nwfilt-XXXXXX";
+ size_t len;
char *filnam;
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- char *header;
size_t written;
- virBufferAddLit(&buf, "#!/bin/sh\n");
-
- if (virBufferError(&buf)) {
- virBufferFreeAndReset(&buf);
- virReportOOMError();
- return NULL;
- }
- header = virBufferContentAndReset(&buf);
-
int fd = mkstemp(filename);
if (fd< 0) {
@@ -2514,15 +2503,8 @@ ebiptablesWriteToTempFile(const char *string) {
goto err_exit;
}
- if (fchmod(fd, S_IXUSR| S_IRUSR | S_IWUSR)< 0) {
- virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- "%s",
- _("cannot change permissions on temp. file"));
- goto err_exit;
- }
-
- len = strlen(header);
- written = safewrite(fd, header, len);
+ len = strlen(string);
+ written = safewrite(fd, string, len);
if (written != len) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
"%s",
@@ -2530,9 +2512,7 @@ ebiptablesWriteToTempFile(const char *string) {
goto err_exit;
}
- len = strlen(string);
- written = safewrite(fd, string, len);
- if (written != len) {
+ if (VIR_CLOSE(fd)< 0) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
"%s",
_("cannot write string to file"));
@@ -2545,12 +2525,9 @@ ebiptablesWriteToTempFile(const char *string) {
goto err_exit;
}
- VIR_FREE(header);
- VIR_FORCE_CLOSE(fd);
return filnam;
err_exit:
- VIR_FREE(header);
VIR_FORCE_CLOSE(fd);
unlink(filename);
return NULL;
@@ -2564,7 +2541,7 @@ err_exit:
* @status: Pointer to an integer for returning the WEXITSTATUS of the
* commands executed via the script the was run.
*
- * Returns 0 in case of success, != 0 in case of an error. The returned
+ * Returns 0 in case of success,< 0 in case of an error. The returned
* value is NOT the result of running the commands inside the shell
* script.
*
@@ -2577,53 +2554,42 @@ ebiptablesExecCLI(virBufferPtr buf,
{
char *cmds;
char *filename;
- int rc;
- const char *argv[] = {NULL, NULL};
+ int rc = -1;
+ virCommandPtr cmd;
if (virBufferError(buf)) {
virReportOOMError();
virBufferFreeAndReset(buf);
- return 1;
+ return -1;
}
*status = 0;
cmds = virBufferContentAndReset(buf);
-
VIR_DEBUG("%s", NULLSTR(cmds));
-
if (!cmds)
return 0;
filename = ebiptablesWriteToTempFile(cmds);
- VIR_FREE(cmds);
-
if (!filename)
- return 1;
+ goto cleanup;
- argv[0] = filename;
+ cmd = virCommandNew("/bin/sh");
+ virCommandAddArg(cmd, filename);
virMutexLock(&execCLIMutex);
- rc = virRun(argv, status);
+ rc = virCommandRun(cmd, status);
virMutexUnlock(&execCLIMutex);
- if (rc == 0) {
- if (WIFEXITED(*status)) {
- *status = WEXITSTATUS(*status);
- } else {
- rc = -1;
- *status = 1;
- }
- }
-
- VIR_DEBUG("rc = %d, status = %d", rc, *status);
-
unlink(filename);
-
VIR_FREE(filename);
+cleanup:
+ VIR_FREE(cmds);
+ virCommandFree(cmd);
+
return rc;
}
ACK, still works with this one ...
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list