[apparmor] [patch] Let the parser reject ambiguous unit 'm' for rlimit rttime

John Johansen john.johansen at canonical.com
Thu Jul 9 11:22:52 UTC 2015


v2
move convert_time_units to parser_misc.c and add unit tests
add "sec" abreviation of secons
optionally allow whitespace between rlimit value and unit
allow time the unit to be optional, but output a warning
update man page to add a note about 'cpu' using units >= seconds
fix "else" error by using else if (tmp < 0)
fix "weekss"

---

commit 5ee47621cc28b745d508b0c69046ce1b3776f8b4
Author: John Johansen <john.johansen at canonical.com>
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 <john.johansen at canonical.com>

diff --git a/parser/apparmor.d.pod b/parser/apparmor.d.pod
index 93ea835..4b089a8 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'. RLIMIT 'cpu' only allows units >= 'seconds'.
 
 B<RLIMIT NICE> = a number between -20 and 19. Only applies to RLIMIT of 'nice'
 
diff --git a/parser/parser.h b/parser/parser.h
index dfd195d..58bd00a 100644
--- a/parser/parser.h
+++ b/parser/parser.h
@@ -402,6 +402,9 @@ extern void free_cod_entries(struct cod_entry *list);
 extern void __debug_capabilities(uint64_t capset, const char *name);
 void debug_cod_entries(struct cod_entry *list);
 
+#define SECONDS_P_MS (1000LL * 1000LL)
+long long convert_time_units(long long value, long long base, const char *units);
+
 
 /* parser_symtab.c */
 struct set_value {
diff --git a/parser/parser_lex.l b/parser/parser_lex.l
index 2832a1c..86825fa 100644
--- a/parser/parser_lex.l
+++ b/parser/parser_lex.l
@@ -447,7 +447,7 @@ LT_EQUAL	<=
 }
 
 <RLIMIT_MODE>{
-	-?{NUMBER}[[:alpha:]]*  {
+	-?{NUMBER} {
 	        yylval.var_val = strdup(yytext);
 		RETURN_TOKEN(TOK_VALUE);
 	}
diff --git a/parser/parser_misc.c b/parser/parser_misc.c
index e362f24..ef1e0aa 100644
--- a/parser/parser_misc.c
+++ b/parser/parser_misc.c
@@ -867,6 +867,54 @@ void print_cond_entry(struct cond_entry *ent)
 	}
 }
 
+
+struct time_units {
+	const char *str;
+	long long value;
+};
+
+static struct time_units time_units[] = {
+	{ "us", 1LL },
+	{ "microsecond", 1LL },
+	{ "microseconds", 1LL },
+	{ "ms", 1000LL },
+	{ "millisecond", 1000LL },
+	{ "milliseconds", 1000LL },
+	{ "s", 1000LL * 1000LL },
+	{ "sec", SECONDS_P_MS },
+	{ "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 },
+	{ "d", 24LL * 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 },
+	{ "weeks", 7LL * 24LL * 60LL * 60LL * SECONDS_P_MS },
+	{ NULL, 0 }
+};
+
+long long convert_time_units(long long value, long long base, const char *units)
+{
+	struct time_units *ent;
+	if (!units)
+		/* default to base if no units */
+		return value;
+
+	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;
+}
+
 #ifdef UNIT_TEST
 
 #include "unit_test.h"
@@ -1085,6 +1133,50 @@ int test_processquoted(void)
 	return rc;
 }
 
+#define TIME_TEST(V, B, U, R)					\
+MY_TEST(convert_time_units((V), (B), U) == (R),		\
+	"convert " #V " with base of " #B ", " #U " units")
+
+int test_convert_time_units()
+{
+	int rc = 0;
+
+	TIME_TEST(1LL, 1LL, NULL, 1LL);
+	TIME_TEST(12345LL, 1LL, NULL, 12345LL);
+	TIME_TEST(10LL, 10LL, NULL, 10LL);
+	TIME_TEST(123450LL, 10LL, NULL, 123450LL);
+
+        TIME_TEST(12345LL, 1LL, "us", 12345LL);
+	TIME_TEST(12345LL, 1LL, "microsecond", 12345LL);
+	TIME_TEST(12345LL, 1LL, "microseconds", 12345LL);
+
+	TIME_TEST(12345LL, 1LL, "ms", 12345LL*1000LL);
+	TIME_TEST(12345LL, 1LL, "millisecond", 12345LL*1000LL);
+	TIME_TEST(12345LL, 1LL, "milliseconds", 12345LL*1000LL);
+
+	TIME_TEST(12345LL, 1LL, "s", 12345LL*1000LL*1000LL);
+	TIME_TEST(12345LL, 1LL, "sec", 12345LL*1000LL*1000LL);
+	TIME_TEST(12345LL, 1LL, "second", 12345LL*1000LL*1000LL);
+	TIME_TEST(12345LL, 1LL, "seconds", 12345LL*1000LL*1000LL);
+
+	TIME_TEST(12345LL, 1LL, "min", 12345LL*1000LL*1000LL*60LL);
+	TIME_TEST(12345LL, 1LL, "minute", 12345LL*1000LL*1000LL*60LL);
+	TIME_TEST(12345LL, 1LL, "minutes", 12345LL*1000LL*1000LL*60LL);
+
+	TIME_TEST(12345LL, 1LL, "h", 12345LL*1000LL*1000LL*60LL*60LL);
+	TIME_TEST(12345LL, 1LL, "hour", 12345LL*1000LL*1000LL*60LL*60LL);
+	TIME_TEST(12345LL, 1LL, "hours", 12345LL*1000LL*1000LL*60LL*60LL);
+
+	TIME_TEST(12345LL, 1LL, "d", 12345LL*1000LL*1000LL*60LL*60LL*24LL);
+	TIME_TEST(12345LL, 1LL, "day", 12345LL*1000LL*1000LL*60LL*60LL*24LL);
+	TIME_TEST(12345LL, 1LL, "days", 12345LL*1000LL*1000LL*60LL*60LL*24LL);
+
+	TIME_TEST(12345LL, 1LL, "week", 12345LL*1000LL*1000LL*60LL*60LL*24LL*7LL);
+	TIME_TEST(12345LL, 1LL, "weeks", 12345LL*1000LL*1000LL*60LL*60LL*24LL*7LL);
+	
+	return rc;
+}
+
 int main(void)
 {
 	int rc = 0;
@@ -1102,6 +1194,10 @@ int main(void)
 	if (retval != 0)
 		rc = retval;
 
+	retval = test_convert_time_units();
+	if (retval != 0)
+		rc = retval;
+
 	return rc;
 }
 #endif /* UNIT_TEST */
diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
index b3083d5..48fbe3c 100644
--- a/parser/parser_yacc.y
+++ b/parser/parser_yacc.y
@@ -78,7 +78,6 @@ 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);
 
 %}
@@ -853,7 +852,7 @@ rules:	rules cond_rule
 		$$ = merge_policy($1, $2);
 	}
 
-rules: rules TOK_SET TOK_RLIMIT TOK_ID TOK_LE TOK_VALUE TOK_END_OF_RULE
+rules: rules TOK_SET TOK_RLIMIT TOK_ID TOK_LE TOK_VALUE opt_id TOK_END_OF_RULE
 	{
 		rlim_t value = RLIM_INFINITY;
 		long long tmp;
@@ -866,11 +865,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";
@@ -880,34 +874,25 @@ 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, $7);
+				if (tmp == -1LL)
+					yyerror("RLIMIT '%s %s' < minimum value of 1s\n", $4, $6);
+				else if (tmp < 0LL)
 					yyerror("RLIMIT '%s' invalid value %s\n", $4, $6);
-				}
+				if (!$7)
+					pwarn(_("RLIMIT 'cpu' no units specified using default units of seconds\n"));
+				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 {
-					yyerror("RLIMIT '%s' invalid value %s\n", $4, $6);
-				}
+					yyerror("RLIMIT '%s' invalid value %s %s\n", $4, $6, $7 ? $7 : "");
+				tmp = convert_time_units(tmp, 1LL, $7);
+				if (tmp < 0LL)
+					yyerror("RLIMIT '%s' invalid value %s %s\n", $4, $6, $7 ? $7 : "");
+				if (!$7)
+					pwarn(_("RLIMIT 'rttime' no units specified using default units of microseconds\n"));
+				value = tmp;
 				break;
 			case RLIMIT_NOFILE:
 			case RLIMIT_NPROC:
@@ -915,15 +900,15 @@ rules: rules TOK_SET TOK_RLIMIT TOK_ID TOK_LE TOK_VALUE TOK_END_OF_RULE
 			case RLIMIT_SIGPENDING:
 #ifdef RLIMIT_RTPRIO
 			case RLIMIT_RTPRIO:
-				if (!end || $6 == end || *end != '\0' || tmp < 0)
-					yyerror("RLIMIT '%s' invalid value %s\n", $4, $6);
+				if (!end || $6 == end || $7 || tmp < 0)
+					yyerror("RLIMIT '%s' invalid value %s %s\n", $4, $6, $7 ? $7 : "");
 				value = tmp;
 				break;
 #endif
 #ifdef RLIMIT_NICE
 			case RLIMIT_NICE:
-				if (!end || $6 == end || *end != '\0')
-					yyerror("RLIMIT '%s' invalid value %s\n", $4, $6);
+				if (!end || $6 == end || $7)
+					yyerror("RLIMIT '%s' invalid value %s %s\n", $4, $6, $7 ? $7 : "");
 				if (tmp < -20 || tmp > 19)
 					yyerror("RLIMIT '%s' out of range (-20 .. 19) %d\n", $4, tmp);
 				value = tmp + 20;
@@ -938,15 +923,17 @@ rules: rules TOK_SET TOK_RLIMIT TOK_ID TOK_LE TOK_VALUE TOK_END_OF_RULE
 			case RLIMIT_MEMLOCK:
 			case RLIMIT_MSGQUEUE:
 				if ($6 == end || tmp < 0)
-					yyerror("RLIMIT '%s' invalid value %s\n", $4, $6);
-				if (strstr(kb, end) == kb) {
+					yyerror("RLIMIT '%s' invalid value %s %s\n", $4, $6, $7 ? $7 : "");
+				if (!$7) {
+					; /* use default of bytes */
+				} else if (strstr(kb, $7) == kb) {
 					tmp *= 1024;
-				} else if (strstr(mb, end) == mb) {
+				} else if (strstr(mb, $7) == mb) {
 					tmp *= 1024*1024;
-				} else if (strstr(gb, end) == gb) {
+				} else if (strstr(gb, $7) == gb) {
 					tmp *= 1024*1024*1024;
-				} else if (*end != '\0') {
-					yyerror("RLIMIT '%s' invalid value %s\n", $4, $6);
+				} else {
+					yyerror("RLIMIT '%s' invalid value %s %s\n", $4, $6, $7);
 				}
 				value = tmp;
 				break;
diff --git a/parser/tst/simple_tests/rlimits/bad_rlimit_01.sd b/parser/tst/simple_tests/rlimits/bad_rlimit_01.sd
new file mode 100644
index 0000000..317ea6f
--- /dev/null
+++ b/parser/tst/simple_tests/rlimits/bad_rlimit_01.sd
@@ -0,0 +1,7 @@
+#
+#=DESCRIPTION realtime time rlimit test with ambiguous unit 'm' which could mean 'ms' or 'minutes'
+#=EXRESULT FAIL
+
+profile rlimit {
+  set rlimit rttime <= 60m,
+}
diff --git a/parser/tst/simple_tests/rlimits/ok_rlimit_01.sd b/parser/tst/simple_tests/rlimits/ok_rlimit_01.sd
index b13bdd2..6757da3 100644
--- a/parser/tst/simple_tests/rlimits/ok_rlimit_01.sd
+++ b/parser/tst/simple_tests/rlimits/ok_rlimit_01.sd
@@ -1,5 +1,5 @@
 #
-#=DESCRIPTION simple cpu rlimit test
+#=DESCRIPTION simple cpu rlimit test, cpu allows default units
 #=EXRESULT PASS
 
 profile rlimit {
diff --git a/parser/tst/simple_tests/rlimits/ok_rlimit_13.sd b/parser/tst/simple_tests/rlimits/ok_rlimit_13.sd
index e4cf929..50b9a6f 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
+#=DESCRIPTION simple rttime rlimit allows default units
 #=EXRESULT PASS
 
 profile rlimit {
-  set rlimit cpu <= 12,
+  set rlimit rttime <= 12,
 }





More information about the AppArmor mailing list