On Thu, Jun 18, 2015 at 02:35:53AM -0700, John Johansen wrote:
> Here is a different attempt to fix this
> 
> ---
> 
> commit 0ca80c5efb5ec4b6ba84bcb7e7a8c11afbd1fdf6
> Author: John Johansen <[email protected]>
> Date:   Thu Jun 18 02:22:42 2015 -0700
> 
>     fix: rlimit unit parsing for time
>     
>     currently the parser supports ambiguous units like m for time,
>     which could mean minutes or milliseconds. Fix this and refactor the
>     time parsing into a single routine.
>     
>     Signed-off-by: John Johansen <[email protected]>
> 
> diff --git a/parser/apparmor.d.pod b/parser/apparmor.d.pod
> index 9ed5961..1ab8ab0 100644
> --- a/parser/apparmor.d.pod
> +++ b/parser/apparmor.d.pod
> @@ -231,11 +231,13 @@ B<RLIMIT RULE> = 'set' 'rlimit' [I<RLIMIT> 'E<lt>=' 
> I<RLIMIT VALUE> ]
>  
>  B<RLIMIT> = ( 'cpu' | 'fsize' | 'data' | 'stack' | 'core' | 'rss' | 'nofile' 
> | 'ofile' | 'as' | 'nproc' | 'memlock' | 'locks' | 'sigpending' | 'msgqueue' 
> | 'nice' | 'rtprio' | 'rttime' )
>  
> -B<RLIMIT VALUE> = ( I<RLIMIT SIZE> | I<RLIMIT NUMBER> | I<RLIMIT NICE> )
> +B<RLIMIT VALUE> = ( I<RLIMIT SIZE> | I<RLIMIT NUMBER> | I<RLIMIT TIME> | 
> I<RLIMIT NICE> )
>  
>  B<RLIMIT SIZE> = I<NUMBER> ( 'K' | 'M' | 'G' ) Only applies to RLIMIT of 
> 'fsize', 'data', 'stack', 'core', 'rss', 'as', 'memlock', 'msgqueue'.
>  
> -B<RLIMIT NUMBER> = number from 0 to max rlimit value. Only applies ot RLIMIT 
> of 'nofile', 'locks', 'sigpending', 'nproc', 'rtprio', 'cpu'
> +B<RLIMIT NUMBER> = number from 0 to max rlimit value. Only applies ot RLIMIT 
> of 'ofile', 'nofile', 'locks', 'sigpending', 'nproc', 'rtprio'
> +
> +B<RLIMIT TIME> = I<NUMBER> ( 'us' | 'microsecond' | 'microseconds' | 'ms' | 
> 'millisecond' | 'milliseconds' | 's' | 'second' | 'seconds' | 'min' | 
> 'minute' | 'minutes' | 'h' | 'hour' | 'hours' | 'day' | 'days' | 'week' | 
> 'weeks' ) Only applies to RLIMIT of 'cpu', 'rttime',

Do we need to note somehow that there can't be any spaces between the
number and the time unit:

  $ echo 'profile /t { set rlimit rttime <= 1second, } ' | ./apparmor_parser 
-qQK
  $ echo 'profile /t { set rlimit rttime <= 1 second, } ' | ./apparmor_parser 
-qQK
  AppArmor parser error, in stdin line 1: syntax error, unexpected TOK_ID, 
expecting TOK_END_OF_RULE

>  
>  B<RLIMIT NICE> = a number between -20 and 19. Only applies to RLIMIT of 
> 'nice'
>  
> diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
> index d529e97..7d238d8 100644
> --- a/parser/parser_yacc.y
> +++ b/parser/parser_yacc.y
> @@ -78,9 +78,15 @@ mnt_rule *do_mnt_rule(struct cond_entry *src_conds, char 
> *src,
>                     int mode);
>  mnt_rule *do_pivot_rule(struct cond_entry *old, char *root,
>                       char *transition);
> -
>  void add_local_entry(Profile *prof);
>  
> +#define SECONDS_P_MS (1000LL * 1000LL)
> +static long long convert_time_units(long long value, long long base, const 
> char *units);
> +struct time_units {
> +     const char *str;
> +     long long value;
> +};
> +
>  %}
>  
>  %token TOK_ID
> @@ -867,11 +873,6 @@ rules: rules TOK_SET TOK_RLIMIT TOK_ID TOK_LE TOK_VALUE 
> TOK_END_OF_RULE
>               if (strcmp($6, "infinity") == 0) {
>                       value = RLIM_INFINITY;
>               } else {
> -                     const char *seconds = "seconds";
> -                     const char *milliseconds = "ms";
> -                     const char *minutes = "minutes";
> -                     const char *hours = "hours";
> -                     const char *days = "days";
>                       const char *kb = "KB";
>                       const char *mb = "MB";
>                       const char *gb = "GB";
> @@ -881,34 +882,21 @@ rules: rules TOK_SET TOK_RLIMIT TOK_ID TOK_LE TOK_VALUE 
> TOK_END_OF_RULE
>                       case RLIMIT_CPU:
>                               if (!end || $6 == end || tmp < 0)
>                                       yyerror("RLIMIT '%s' invalid value 
> %s\n", $4, $6);
> -                             if (*end == '\0' ||
> -                                 strstr(seconds, end) == seconds) {
> -                                     value = tmp;
> -                             } else if (strstr(minutes, end) == minutes) {
> -                                     value = tmp * 60;
> -                             } else if (strstr(hours, end) == hours) {
> -                                     value = tmp * 60 * 60;
> -                             } else if (strstr(days, end) == days) {
> -                                     value = tmp * 60 * 60 * 24;
> -                             } else {
> +                             tmp = convert_time_units(tmp, SECONDS_P_MS, 
> end);
> +                             if (tmp == -1LL)
> +                                     yyerror("RLIMIT '%s %s' < minimum value 
> of 1s\n", $4, $6);
> +                             else

ITYM:                           else if (tmp = -2LL)
>                                       yyerror("RLIMIT '%s' invalid value 
> %s\n", $4, $6);

.. because otherwise there's no way for this to not invoke yyerror.

> -                             }
> +                             value = tmp;
>                               break;
>                       case RLIMIT_RTTIME:
>                               /* RTTIME is measured in microseconds */
>                               if (!end || $6 == end || tmp < 0)
>                                       yyerror("RLIMIT '%s' invalid value 
> %s\n", $4, $6);
> -                             if (*end == '\0') {
> -                                     value = tmp;
> -                             } else if (strstr(milliseconds, end) == 
> milliseconds) {
> -                                     value = tmp * 1000;
> -                             } else if (strstr(seconds, end) == seconds) {
> -                                     value = tmp * 1000 * 1000;
> -                             } else if (strstr(minutes, end) == minutes) {
> -                                     value = tmp * 1000 * 1000 * 60;
> -                             } else {
> +                             tmp = convert_time_units(tmp, 1LL, end);
> +                             if (tmp == -2LL)
>                                       yyerror("RLIMIT '%s' invalid value 
> %s\n", $4, $6);
> -                             }
> +                             value = tmp;
>                               break;
>                       case RLIMIT_NOFILE:
>                       case RLIMIT_NPROC:
> @@ -1695,3 +1683,40 @@ mnt_rule *do_pivot_rule(struct cond_entry *old, char 
> *root, char *transition)
>  
>       return ent;
>  }
> +
> +static struct time_units time_units[] = { 
> +     { "us", 1LL },
> +     { "microsecond", 1LL },
> +     { "microseconds", 1LL },
> +     { "ms", 1000LL },
> +     { "millisecond", 1000LL },
> +     { "milliseconds", 1000LL },
> +     { "s", 1000LL * 1000LL },

"sec" might also be a useful shortcut for second.

> +     { "second", SECONDS_P_MS },
> +     { "seconds", SECONDS_P_MS },
> +     { "min" , 60LL * SECONDS_P_MS },
> +     { "minute", 60LL * SECONDS_P_MS },
> +     { "minutes", 60LL * SECONDS_P_MS },
> +     { "h", 60LL * 60LL * SECONDS_P_MS },
> +     { "hour", 60LL * 60LL * SECONDS_P_MS },
> +     { "hours", 60LL * 60LL * SECONDS_P_MS },
> +     { "day", 24LL * 60LL * 60LL * SECONDS_P_MS },
> +     { "days", 24LL * 60LL * 60LL * SECONDS_P_MS },
> +     { "week", 7LL * 24LL * 60LL * 60LL * SECONDS_P_MS },
> +     { "weekss", 7LL * 24LL * 60LL * 60LL * SECONDS_P_MS },

Typo: "weekss" versus "weeks"

> +     { NULL, 0 }
> +};
> +
> +static long long convert_time_units(long long value, long long base, const 
> char *units)
> +{
> +     struct time_units *ent;
> +     for (ent = time_units; ent->str; ent++) {
> +             if (strcmp(ent->str, units) == 0) {
> +                     if (value * ent->value < base)
> +                             return -1LL;
> +                     return value * ent->value / base;
> +             }
> +     }
> +     return -2LL;
> +}

I can haz unit tests?

> diff --git a/parser/tst/simple_tests/rlimits/ok_rlimit_01.sd 
> b/parser/tst/simple_tests/rlimits/ok_rlimit_01.sd
> index b13bdd2..8f13257 100644
> --- a/parser/tst/simple_tests/rlimits/ok_rlimit_01.sd
> +++ b/parser/tst/simple_tests/rlimits/ok_rlimit_01.sd
> @@ -1,6 +1,6 @@
>  #
> -#=DESCRIPTION simple cpu rlimit test
> -#=EXRESULT PASS
> +#=DESCRIPTION simple cpu rlimit test, cpu must have units
> +#=EXRESULT FAIL
>  
>  profile rlimit {
>    set rlimit cpu <= 1024,
> diff --git a/parser/tst/simple_tests/rlimits/ok_rlimit_13.sd 
> b/parser/tst/simple_tests/rlimits/ok_rlimit_13.sd
> index e4cf929..02c6562 100644
> --- a/parser/tst/simple_tests/rlimits/ok_rlimit_13.sd
> +++ b/parser/tst/simple_tests/rlimits/ok_rlimit_13.sd
> @@ -1,7 +1,7 @@
>  #
> -#=DESCRIPTION simple cpu rlimit test
> -#=EXRESULT PASS
> +#=DESCRIPTION simple rttime rlimit must have units
> +#=EXRESULT FAIL
>  
>  profile rlimit {
> -  set rlimit cpu <= 12,
> +  set rlimit rttime <= 12,
>  }

-- 
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/

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