Re: [systemd-devel] [PATCH] ima-setup: write policy one line at a time
On Thu, 11.06.15 00:34, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
On Thu, Jun 11, 2015 at 01:16:47AM +0200, Lennart Poettering wrote:
On Wed, 10.06.15 15:38, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
wrote:
ima_write_policy() expects data to be written as one or more
rules, no more than PAGE_SIZE at a time. Easiest way to ensure
that we are not splitting rules is to read and write on line at
a time.
https://bugzilla.redhat.com/show_bug.cgi?id=1226948
---
src/core/ima-setup.c | 39 +--
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 4d8b638115..5b3d16cd31 100644
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -23,9 +23,6 @@
#include unistd.h
#include errno.h
-#include fcntl.h
-#include sys/stat.h
-#include sys/mman.h
#include ima-setup.h
#include util.h
@@ -36,20 +33,19 @@
#define IMA_POLICY_PATH /etc/ima/ima-policy
int ima_setup(void) {
-int r = 0;
-
#ifdef HAVE_IMA
-_cleanup_close_ int policyfd = -1, imafd = -1;
-struct stat st;
-char *policy;
+_cleanup_fclose_ FILE *input = NULL;
+_cleanup_close_ int imafd = -1;
+char line[LINE_MAX];
Hmm, I wonder if this might bite us. LINE_MAX is a good choice as max
line length for formats we define in systemd, but the question of
course is what the the max line length is for IMA...
It's PAGE_SIZE ;) Making this dynamic doesn't make much sense to me,
but we could make it 4096, as this is the lowest (and common) size.
I don't think this is actually really that bad:
_cleanup_free_ void *line = NULL;
line = malloc(page_size());
Or, we could even just do alloca(page_size())...
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] ima-setup: write policy one line at a time
On Thu, Jun 11, 2015 at 11:28:06AM +0200, Lennart Poettering wrote:
On Thu, 11.06.15 00:34, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
On Thu, Jun 11, 2015 at 01:16:47AM +0200, Lennart Poettering wrote:
On Wed, 10.06.15 15:38, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
wrote:
ima_write_policy() expects data to be written as one or more
rules, no more than PAGE_SIZE at a time. Easiest way to ensure
that we are not splitting rules is to read and write on line at
a time.
https://bugzilla.redhat.com/show_bug.cgi?id=1226948
---
src/core/ima-setup.c | 39 +--
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 4d8b638115..5b3d16cd31 100644
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -23,9 +23,6 @@
#include unistd.h
#include errno.h
-#include fcntl.h
-#include sys/stat.h
-#include sys/mman.h
#include ima-setup.h
#include util.h
@@ -36,20 +33,19 @@
#define IMA_POLICY_PATH /etc/ima/ima-policy
int ima_setup(void) {
-int r = 0;
-
#ifdef HAVE_IMA
-_cleanup_close_ int policyfd = -1, imafd = -1;
-struct stat st;
-char *policy;
+_cleanup_fclose_ FILE *input = NULL;
+_cleanup_close_ int imafd = -1;
+char line[LINE_MAX];
Hmm, I wonder if this might bite us. LINE_MAX is a good choice as max
line length for formats we define in systemd, but the question of
course is what the the max line length is for IMA...
It's PAGE_SIZE ;) Making this dynamic doesn't make much sense to me,
but we could make it 4096, as this is the lowest (and common) size.
I don't think this is actually really that bad:
_cleanup_free_ void *line = NULL;
line = malloc(page_size());
Or, we could even just do alloca(page_size())...
Either would break FOR_EACH_LINE, but line[page_size()] should work.
https://github.com/systemd/systemd/pull/167
What I don't like about having a non-fixed value for the line size is
that the syntactic validity of configuration depends on the kernel you
are running. In practice not an issue, unless you like really long
lines.
@Mimi: could you check that the patch works for you?
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] ima-setup: write policy one line at a time
ima_write_policy() expects data to be written as one or more
rules, no more than PAGE_SIZE at a time. Easiest way to ensure
that we are not splitting rules is to read and write on line at
a time.
https://bugzilla.redhat.com/show_bug.cgi?id=1226948
---
src/core/ima-setup.c | 39 +--
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 4d8b638115..5b3d16cd31 100644
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -23,9 +23,6 @@
#include unistd.h
#include errno.h
-#include fcntl.h
-#include sys/stat.h
-#include sys/mman.h
#include ima-setup.h
#include util.h
@@ -36,20 +33,19 @@
#define IMA_POLICY_PATH /etc/ima/ima-policy
int ima_setup(void) {
-int r = 0;
-
#ifdef HAVE_IMA
-_cleanup_close_ int policyfd = -1, imafd = -1;
-struct stat st;
-char *policy;
+_cleanup_fclose_ FILE *input = NULL;
+_cleanup_close_ int imafd = -1;
+char line[LINE_MAX];
+unsigned lineno = 0;
if (access(IMA_SECFS_DIR, F_OK) 0) {
log_debug(IMA support is disabled in the kernel, ignoring.);
return 0;
}
-policyfd = open(IMA_POLICY_PATH, O_RDONLY|O_CLOEXEC);
-if (policyfd 0) {
+input = fopen(IMA_POLICY_PATH, re);
+if (!input) {
log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING,
errno,
Failed to open the IMA custom policy file
IMA_POLICY_PATH, ignoring: %m);
return 0;
@@ -66,20 +62,19 @@ int ima_setup(void) {
return 0;
}
-if (fstat(policyfd, st) 0)
-return log_error_errno(errno, Failed to fstat
IMA_POLICY_PATH: %m);
+FOREACH_LINE(line, input,
+ return log_error_errno(errno, Failed to read the IMA
custom policy file IMA_POLICY_PATH: %m)) {
+size_t len;
-policy = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, policyfd, 0);
-if (policy == MAP_FAILED)
-return log_error_errno(errno, Failed to mmap
IMA_POLICY_PATH: %m);
+len = strlen(line);
+lineno++;
-r = loop_write(imafd, policy, (size_t) st.st_size, false);
-if (r 0)
-log_error_errno(r, Failed to load the IMA custom policy file
IMA_POLICY_PATH: %m);
-else
-log_info(Successfully loaded the IMA custom policy
IMA_POLICY_PATH.);
+if (len 0 write(imafd, line, len) 0)
+return log_error_errno(errno, Failed to load the IMA
custom policy file IMA_POLICY_PATH%u: %m,
+ lineno);
+}
-munmap(policy, st.st_size);
+log_info(Successfully loaded the IMA custom policy
IMA_POLICY_PATH.);
#endif /* HAVE_IMA */
-return r;
+return 0;
}
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] ima-setup: write policy one line at a time
Patchset imported to github. To create a pull request, one of the main developers has to initiate one via: https://github.com/systemd/systemd/compare/master...systemd-mailing-devs:1433965127-19648-1-git-send-email-zbyszek%40in.waw.pl -- Generated by https://github.com/haraldh/mail2git ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] ima-setup: write policy one line at a time
On Thu, Jun 11, 2015 at 01:16:47AM +0200, Lennart Poettering wrote:
On Wed, 10.06.15 15:38, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
ima_write_policy() expects data to be written as one or more
rules, no more than PAGE_SIZE at a time. Easiest way to ensure
that we are not splitting rules is to read and write on line at
a time.
https://bugzilla.redhat.com/show_bug.cgi?id=1226948
---
src/core/ima-setup.c | 39 +--
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 4d8b638115..5b3d16cd31 100644
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -23,9 +23,6 @@
#include unistd.h
#include errno.h
-#include fcntl.h
-#include sys/stat.h
-#include sys/mman.h
#include ima-setup.h
#include util.h
@@ -36,20 +33,19 @@
#define IMA_POLICY_PATH /etc/ima/ima-policy
int ima_setup(void) {
-int r = 0;
-
#ifdef HAVE_IMA
-_cleanup_close_ int policyfd = -1, imafd = -1;
-struct stat st;
-char *policy;
+_cleanup_fclose_ FILE *input = NULL;
+_cleanup_close_ int imafd = -1;
+char line[LINE_MAX];
Hmm, I wonder if this might bite us. LINE_MAX is a good choice as max
line length for formats we define in systemd, but the question of
course is what the the max line length is for IMA...
It's PAGE_SIZE ;) Making this dynamic doesn't make much sense to me,
but we could make it 4096, as this is the lowest (and common) size.
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] ima-setup: write policy one line at a time
On Wed, 10.06.15 15:38, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
ima_write_policy() expects data to be written as one or more
rules, no more than PAGE_SIZE at a time. Easiest way to ensure
that we are not splitting rules is to read and write on line at
a time.
https://bugzilla.redhat.com/show_bug.cgi?id=1226948
---
src/core/ima-setup.c | 39 +--
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 4d8b638115..5b3d16cd31 100644
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -23,9 +23,6 @@
#include unistd.h
#include errno.h
-#include fcntl.h
-#include sys/stat.h
-#include sys/mman.h
#include ima-setup.h
#include util.h
@@ -36,20 +33,19 @@
#define IMA_POLICY_PATH /etc/ima/ima-policy
int ima_setup(void) {
-int r = 0;
-
#ifdef HAVE_IMA
-_cleanup_close_ int policyfd = -1, imafd = -1;
-struct stat st;
-char *policy;
+_cleanup_fclose_ FILE *input = NULL;
+_cleanup_close_ int imafd = -1;
+char line[LINE_MAX];
Hmm, I wonder if this might bite us. LINE_MAX is a good choice as max
line length for formats we define in systemd, but the question of
course is what the the max line length is for IMA...
Looks good otherwise.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
