[apparmor] [patch] fix load of binary cache files

John Johansen john.johansen at canonical.com
Fri Mar 21 11:25:02 UTC 2014


Fix profile loads from cache files that contain multiple profiles

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 |   43 ++++++++++++++++++++++++++++++++-----------
 parser/parser_main.c      |    2 ++
 4 files changed, 36 insertions(+), 11 deletions(-)

--- 2.9-test.orig/parser/parser.h
+++ 2.9-test/parser/parser.h
@@ -222,6 +222,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_mount;
 extern int kernel_supports_dbus;
--- 2.9-test.orig/parser/parser_common.c
+++ 2.9-test/parser/parser_common.c
@@ -25,6 +25,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 = 1;        /* kernel supports network rules */
 int kernel_supports_mount = 0;	        /* kernel supports mount rules */
 int kernel_supports_dbus = 0;		/* kernel supports dbus rules */
--- 2.9-test.orig/parser/parser_interface.c
+++ 2.9-test/parser/parser_interface.c
@@ -871,7 +871,6 @@
 	int fd = -1;
 	int error = -ENOMEM, wsize, bsize;
 	char *filename = NULL;
-	char *b;
 
 	switch (option) {
 	case OPTION_ADD:
@@ -898,19 +897,41 @@
 	}
 
 	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) {
+		wsize = write(fd, buffer, size);
+		if (wsize == 0)
+			error = -errno;
+		else if (wsize < size)
+			PERROR(_("%s: Unable to write entire profile set\n"),
+			       progname);
+	} else {
+		char *b, *next;
+
+		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;
+			if (kernel_load) {
+				wsize = write(fd, b, bsize);
+				if (wsize < 0) {
+					error = -errno;
+					PERROR(_("%s: Unable to write profile\n"),
+					       progname);
+					break;
+				} else if (wsize < bsize) {
+					PERROR(_("%s: Unable to write entire profile entry\n"),
+					       progname);
+				}
 			}
 		}
 	}
-	if (kernel_load) close(fd);
+	if (kernel_load)
+		close(fd);
+
 exit:
 	free(filename);
 	return error;
--- 2.9-test.orig/parser/parser_main.c
+++ 2.9-test/parser/parser_main.c
@@ -761,6 +761,8 @@
 
 		flags_string = (char *) malloc(FLAGS_STRING_SIZE);
 		handle_features_dir(FLAGS_FILE, &flags_string, FLAGS_STRING_SIZE, flags_string);
+		if (strstr(flags_string, "set_load"))
+			kernel_supports_setload = 1;
 		if (strstr(flags_string, "network"))
 			kernel_supports_network = 1;
 		else



More information about the AppArmor mailing list