one more thing...

On 2014-03-21 04:25:02, John Johansen wrote:
> 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 <[email protected]>
> ---
>  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)

It looks like this should be checking if wsize is less than 0.

Tyler

> +                     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
> 
> -- 
> AppArmor mailing list
> [email protected]
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to