[apparmor] [patch] fix load of binary cache files, v3
John Johansen
john.johansen at canonical.com
Tue May 6 21:30:44 UTC 2014
Fix profile loads from cache files that contain multiple profiles
v3: fix freeing of filename when undefined
v2: address tyhicks feedback
refactor to have a common write routine
fix issue with set profile load being done even if !kernel_load
Profile loads from cache files that contain multiple profiles can
result in multiple reloads of the same profile or error messages about
failure to load profiles if the --add option is used. eg.
apparmor="STATUS" operation="profile_load"
name="/usr/lib/apache2/mpm-prefork/apache2" pid=8631
comm="apparmor_parser"
<sth0R> [82932.058388] type=1400 audit(1395415826.937:616):
apparmor="STATUS" operation="profile_load" name="DEFAULT_URI" pid=8631
comm="apparmor_parser"
<sth0R> [82932.058391] type=1400 audit(1395415826.937:617):
apparmor="STATUS" operation="profile_load"
name="HANDLING_UNTRUSTED_INPUT" pid=8631 comm="apparmor_parser"
<sth0R> [82932.058394] type=1400 audit(1395415826.937:618):
apparmor="STATUS" operation="profile_load" name="phpsysinfo" pid=8631
comm="apparmor_parser"
<sth0R> [82932.059058] type=1400 audit(1395415826.937:619):
apparmor="STATUS" operation="profile_replace" info="profile can not be
replaced" error=-17
name="/usr/lib/apache2/mpm-prefork/apache2//DEFAULT_URI" pid=8631
comm="apparmor_parser"
<sth0R> [82932.059574] type=1400 audit(1395415826.937:620):
apparmor="STATUS" operation="profile_replace" info="profile can not be
replaced" error=-17
name="/usr/lib/apache2/mpm-prefork/apache2//HANDLING_UNTRUSTED_INPUT"
pid=8631 comm="apparmor_parser"
The reason this happens is that the cache file is a container that
can contain multiple profiles in sequential order
profile1
profile2
profile3
The parser loads the entire cache file to memory and the writes the
whole file to the kernel interface. It then skips foward in the file
to the next profile and reloads the file from that profile into
the kernel.
eg. First load
profile1
profile2
profile3
advance to profile2, do second load
profile2
profile3
advance to profile3, do third load
profile3
With older kernels the interface would stop after the first profile and
return that it had processed the whole file, thus while wasting compute
resources copying extra data no errors occurred. However newer kernels
now support atomic loading of multipe profiles, so that all the profiles
passed in to the interface get processed.
This means on newer kernels the current parser load behavior results
in multiple loads/replacements when a cache file contains more than
one profile (note: loads from a compile do not have this problem).
To fix this, detect if the kernel supports atomic set loads, and load
the cache file once. If it doesn't only load one profile section
from a cache file at a time.
Signed-off-by: John Johansen <john.johansen at canonical.com>
---
parser/parser.h | 1
parser/parser_common.c | 1
parser/parser_interface.c | 69 ++++++++++++++++++++++++++++++----------------
parser/parser_main.c | 2 +
4 files changed, 49 insertions(+), 24 deletions(-)
--- 2.9-test.orig/parser/parser.h
+++ 2.9-test/parser/parser.h
@@ -302,6 +302,7 @@
extern int perms_create;
extern int net_af_max_override;
extern int kernel_load;
+extern int kernel_supports_setload;
extern int kernel_supports_network;
extern int kernel_supports_policydb;
extern int kernel_supports_diff_encode;
--- 2.9-test.orig/parser/parser_common.c
+++ 2.9-test/parser/parser_common.c
@@ -66,6 +66,7 @@
int perms_create = 0; /* perms contain create flag */
int net_af_max_override = -1; /* use kernel to determine af_max */
int kernel_load = 1;
+int kernel_supports_setload = 0; /* kernel supports atomic set loads */
int kernel_supports_network = 0; /* kernel supports network rules */
int kernel_supports_policydb = 0; /* kernel supports new policydb */
int kernel_supports_mount = 0; /* kernel supports mount rules */
--- 2.9-test.orig/parser/parser_interface.c
+++ 2.9-test/parser/parser_interface.c
@@ -634,52 +634,73 @@
return NULL;
}
+static int write_buffer(int fd, char *buffer, int size, bool set)
+{
+ const char *err_str = set ? "profile set" : "profile";
+ int wsize = write(fd, buffer, size);
+ if (wsize < 0) {
+ PERROR(_("%s: Unable to write %s\n"), progname, err_str);
+ return -errno;
+ } else if (wsize < size) {
+ PERROR(_("%s: Unable to write %s\n"), progname, err_str);
+ return -EPROTO;
+ }
+ return 0;
+}
+
int sd_load_buffer(int option, char *buffer, int size)
{
int fd = -1;
- int error = -ENOMEM, wsize, bsize;
+ int error, bsize;
char *filename = NULL;
- char *b;
+
+ /* TODO: push backup into caller */
+ if (!kernel_load)
+ return 0;
switch (option) {
case OPTION_ADD:
if (asprintf(&filename, "%s/.load", subdomainbase) == -1)
- goto exit;
- if (kernel_load) fd = open(filename, O_WRONLY);
+ return -ENOMEM;
break;
case OPTION_REPLACE:
if (asprintf(&filename, "%s/.replace", subdomainbase) == -1)
- goto exit;
- if (kernel_load) fd = open(filename, O_WRONLY);
+ return -ENOMEM;
break;
default:
- error = -EINVAL;
- goto exit;
- break;
+ return -EINVAL;
}
- if (kernel_load && fd < 0) {
+ fd = open(filename, O_WRONLY);
+ if (fd < 0) {
PERROR(_("Unable to open %s - %s\n"), filename,
strerror(errno));
error = -errno;
- goto exit;
+ goto out;
}
- error = 0;
- for (b = buffer; b ; b = next_profile_buffer(b + sizeof(header_version), bsize)) {
- bsize = size - (b - buffer);
- if (kernel_load) {
- wsize = write(fd, b, bsize);
- if (wsize < 0) {
- error = -errno;
- } else if (wsize < bsize) {
- PERROR(_("%s: Unable to write entire profile entry\n"),
- progname);
- }
+ if (kernel_supports_setload) {
+ error = write_buffer(fd, buffer, size, true);
+ } else {
+ char *b, *next;
+
+ error = 0; /* in case there are no profiles */
+ for (b = buffer; b; b = next, size -= bsize) {
+ next = next_profile_buffer(b + sizeof(header_version),
+ size);
+ if (next)
+ bsize = next - b;
+ else
+ bsize = size;
+ error = write_buffer(fd, b, bsize, false);
+ if (error)
+ break;
}
}
- if (kernel_load) close(fd);
-exit:
+ close(fd);
+
+out:
free(filename);
+
return error;
}
--- 2.9-test.orig/parser/parser_main.c
+++ 2.9-test/parser/parser_main.c
@@ -689,6 +689,8 @@
kernel_supports_policydb = 1;
if (strstr(features_string, "v6"))
kernel_abi_version = 6;
+ if (strstr(features_string, "set_load"))
+ kernel_supports_setload = 1;
if (strstr(features_string, "network"))
kernel_supports_network = 1;
if (strstr(features_string, "mount"))
More information about the AppArmor
mailing list