From c01ed1d57b6115067f43a405b14acf24d84ff03a Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Thu, 11 Jun 2020 22:15:07 +0200 Subject: [PATCH 1/5] Error out on unhandled parts when parsing a profile ... (using `%option nodefault`) instead of echoing the unknown parts to stdout, and ignoring the error. This will cause the parser to error out with flex scanner jammed and $?=2 if a profile contains unknown/invalid parts. That's not really a helpful error message, but still better than ignoring errors. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569 Signed-off-by: Christian Boltz Acked-by: John Johansen Acked-by: Steve Beattie --- parser/parser_lex.l | 1 + 1 file changed, 1 insertion(+) diff --git a/parser/parser_lex.l b/parser/parser_lex.l index 97c2e849b..97250f958 100644 --- a/parser/parser_lex.l +++ b/parser/parser_lex.l @@ -25,6 +25,7 @@ %option noyy_top_state %option nounput %option stack +%option nodefault %{ #include From 7d062917aa34c62b2eafc3c20f77d0be903b90bc Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Thu, 11 Jun 2020 22:18:31 +0200 Subject: [PATCH 2/5] Remove TODO for half-quoted abi rule With %option nodefault, the parser now errors out as expected, even if the error message isn't too helpful. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569 Signed-off-by: Christian Boltz Acked-by: John Johansen Acked-by: Steve Beattie --- parser/tst/simple_tests/abi/bad_1.sd | 1 - 1 file changed, 1 deletion(-) diff --git a/parser/tst/simple_tests/abi/bad_1.sd b/parser/tst/simple_tests/abi/bad_1.sd index 1f2e58ae7..1afbbe02f 100644 --- a/parser/tst/simple_tests/abi/bad_1.sd +++ b/parser/tst/simple_tests/abi/bad_1.sd @@ -1,7 +1,6 @@ # #=DESCRIPTION abi testing - abi relative path in quotes #=EXRESULT FAIL -#=TODO abi "simple_tests/includes/abi/4.19, From 1a4288886bcb46dcfc19ad61d648a30488f44619 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 12 Jun 2020 01:10:07 -0700 Subject: [PATCH 3/5] parser: add missing states to the default rule and improve the error msg There were several states missing from the default rule which catches unexpected input in a state. Update the default rule to catch all input including newlines and update its error message to include information about which state the failure occured in. Also update the comment about what to do when adding new states. While the lexer now has the "nodefault" option set, it doesn't provide as much information as the default rule does, so we prefer states to use our provided default rule. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569 Signed-off-by: John Johansen Acked-by: Steve Beattie --- parser/parser_lex.l | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/parser/parser_lex.l b/parser/parser_lex.l index 97250f958..c1598eff3 100644 --- a/parser/parser_lex.l +++ b/parser/parser_lex.l @@ -265,7 +265,16 @@ LT_EQUAL <= LT < GT > -/* IF adding new state please update state_names table at eof */ +/* IF adding new state please update state_names table and default rule (just + * above the state_names table) at the eof. + * + * The nodefault option is set so missing adding to the default rule isn't + * fatal but can't take advantage of additional debug the default rule might + * have. + * + * If a state is not added to the default rule it can result in the message + * "flex scanner jammed" + */ %x SUB_ID %x SUB_ID_WS %x SUB_VALUE @@ -700,11 +709,11 @@ include/{WS} { } } -{ - [^\n] { +{ + (.|\n) { DUMP_PREPROCESS; /* Something we didn't expect */ - yyerror(_("Found unexpected character: '%s'"), yytext); + yyerror(_("Lexer found unexpected character: '%s' (0x%x) in state: %s"), yytext, yytext[0], state_names[YY_START].c_str()); } } %% From fffca2ffa00d86b0f3a11eea066cf651486331ea Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 12 Jun 2020 02:05:35 -0700 Subject: [PATCH 4/5] parser: split newline and end of rule handling into separate rules Split the newline processing into a separate rule block so that it can be shared with states that need to process newlines without processing end of rule conditions. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569 Signed-off-by: John Johansen Acked-by: Steve Beattie --- parser/parser_lex.l | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/parser/parser_lex.l b/parser/parser_lex.l index c1598eff3..b2318f667 100644 --- a/parser/parser_lex.l +++ b/parser/parser_lex.l @@ -476,6 +476,7 @@ GT > \\\n { DUMP_PREPROCESS; current_lineno++ ; } \r?\n { + /* don't use shared rule because we need POP() here */ DUMP_PREPROCESS; current_lineno++; POP(); @@ -702,8 +703,10 @@ include/{WS} { POP_NODUMP(); RETURN_TOKEN(TOK_END_OF_RULE); } +} - \r?\n { +{ + \r?\n { DUMP_PREPROCESS; current_lineno++; } From 21498ff9a434117516477b83c6c380b0c26a6037 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 12 Jun 2020 02:12:41 -0700 Subject: [PATCH 5/5] parser: update rule to process newlines to include states that eat WS Newlines should generally be treated as whitespace. Expand the list of states using the newline rule to include almost all rules that eat WS. There are two exceptions assign and comment which have special handling of newlines. this fixes the failures not ok 71543 - ./simple_tests//vars/vars_simple_assignment_13.sd: quoted commas should not trigger an error not ok 71544 - ./simple_tests//vars/vars_simple_assignment_14.sd: quoted commas should not trigger an error found by introducing nodefault MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569 Signed-off-by: John Johansen Acked-by: Steve Beattie --- parser/parser_lex.l | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parser/parser_lex.l b/parser/parser_lex.l index b2318f667..424f8e1cb 100644 --- a/parser/parser_lex.l +++ b/parser/parser_lex.l @@ -705,7 +705,7 @@ include/{WS} { } } -{ +{ \r?\n { DUMP_PREPROCESS; current_lineno++;