On Fri, Sep 19, 2014 at 12:27:21PM -0700, John Johansen wrote:
> fix: Make the parser behave the same as when driven with xargs -n1
> 
> Currently the parser is bailing when it fails to load a profile,
> not processing any potential subsequent profiles in the dir or passed
> in list. This results in all policy after the first error failing
> to load, instead of just the profile(s) with the error.
> 
> This is a different behavior than what has been done by initscripts
> that have driven it with xargs -n1, passing it a single profile
> at a time.
> 
> Fix this so that the parser only exits on first error if specifically
> told to do so.
> 
> Note: this does not fix the various failure points in the parser
> that call exit, instead of returning an error.
> 
> Signed-off-by: John Johansen <[email protected]>

I believe this will address the failures that Jamie's seen. I'm .. on the
fence about the idea as a whole but I believe this implementation is good.

Acked-by: Seth Arnold <[email protected]>

Thanks

> ---
>  apparmor/2.9-new/parser/parser_main.c |   31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> --- jj.orig/apparmor/2.9-new/parser/parser_main.c
> +++ jj/apparmor/2.9-new/parser/parser_main.c
> @@ -74,6 +74,7 @@
>  int create_cache_dir = 0;            /* create the cache dir if missing? */
>  int preprocess_only = 0;
>  int skip_mode_force = 0;
> +int abort_on_error = 0;                      /* stop processing profiles if 
> error */
>  struct timespec mru_tstamp;
>  
>  #define FEATURES_STRING_SIZE 8192
> @@ -123,6 +124,7 @@
>       {"optimize",            1, 0, 'O'},
>       {"Optimize",            1, 0, 'O'},
>       {"preprocess",          0, 0, 'p'},
> +     {"abort-on-error",      0, 0, 132},     /* no short option */
>       {NULL, 0, 0, 0},
>  };
>  
> @@ -172,6 +174,7 @@
>              "-D [n], --dump          Dump internal info for debugging\n"
>              "-O [n], --Optimize      Control dfa optimizations\n"
>              "-h [cmd], --help[=cmd]  Display this text or info about cmd\n"
> +            "--abort-on-error        abort processing of profiles on first 
> error\n"
>              ,command);
>  }
>  
> @@ -410,6 +413,9 @@
>       case 131:
>               create_cache_dir = 1;
>               break;
> +     case 132:
> +             abort_on_error = 1;
> +             break;
>       case 'L':
>               cacheloc = strdup(optarg);
>               break;
> @@ -718,9 +724,10 @@
>       if (profilename) {
>               fd = open(profilename, O_RDONLY);
>               if (fd == -1) {
> +                     retval = errno;
>                       PERROR(_("Error: Could not read binary profile or cache 
> file %s: %s.\n"),
>                              profilename, strerror(errno));
> -                     exit(errno);
> +                     return retval;
>               }
>       } else {
>               fd = dup(0);
> @@ -733,7 +740,7 @@
>                       chunksize <<= 1;
>                       if (!buffer) {
>                               PERROR(_("Memory allocation error."));
> -                             exit(errno);
> +                             return ENOMEM;
>                       }
>               }
>  
> @@ -859,7 +866,7 @@
>               if ( !(yyin = fopen(profilename, "r")) ) {
>                       PERROR(_("Error: Could not read profile %s: %s.\n"),
>                              profilename, strerror(errno));
> -                     exit(errno);
> +                     return errno;
>               }
>       }
>       else {
> @@ -921,7 +928,7 @@
>           !skip_cache) {
>               if (asprintf(&cachename, "%s/%s", cacheloc, basename)<0) {
>                       PERROR(_("Memory allocation error."));
> -                     exit(1);
> +                     return ENOMEM;
>               }
>               /* Load a binary cache if it exists and is newest */
>               if (!skip_read_cache &&
> @@ -937,11 +944,11 @@
>                       /* Otherwise, set up to save a cached copy */
>                       if (asprintf(&cachetemp, "%s-XXXXXX", cachename)<0) {
>                               perror("asprintf");
> -                             exit(1);
> +                             return ENOMEM;
>                       }
>                       if ( (cache_fd = mkstemp(cachetemp)) < 0) {
>                               perror("mkstemp");
> -                             exit(1);
> +                             return ENOMEM;
>                       }
>               }
>       }
> @@ -1203,12 +1210,14 @@
>       setup_flags();
>  
>       retval = 0;
> -     for (i = optind; retval == 0 && i <= argc; i++) {
> +     for (i = optind; i <= argc; i++) {
>               struct stat stat_file;
>  
>               if (i < argc && !(profilename = strdup(argv[i]))) {
>                       perror("strdup");
> -                     return -1;
> +                     if (abort_on_error)
> +                             return -1;
> +                     continue;
>               }
>               /* skip stdin if we've seen other command line arguments */
>               if (i == argc && optind != argc)
> @@ -1223,10 +1232,9 @@
>                       int (*cb)(DIR *dir, const char *name, struct stat *st,
>                                 void *data);
>                       cb = binary_input ? binary_dir_cb : profile_dir_cb;
> -                     if (dirat_for_each(NULL, profilename, profilename, cb)) 
> {
> +                     if ((retval = dirat_for_each(NULL, profilename, 
> profilename, cb))) {
>                               PDEBUG("Failed loading profiles from %s\n",
>                                      profilename);
> -                             exit(1);
>                       }
>               } else if (binary_input) {
>                       retval = process_binary(option, profilename);
> @@ -1236,6 +1244,9 @@
>  
>               if (profilename) free(profilename);
>               profilename = NULL;
> +
> +             if (abort_on_error && retval != 0)
> +                     break;
>       }
>  
>       if (ofile)
> 
> -- 
> 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