In lib/lookup_table.c:audit_name_to_msg_type(), the event type value is parsed and converted to an integer as per, Given type=<type_value> then <type_value> is parsed for - a known string - a long integer number, n, found in the specific string "UNKNOWN[n]" - a long integer number, n, found in the specific string "n" In src/ausearch-report.c:output_interpreted_node() it additionally parses for a <type_value> of - a long integer number, n, found in the string "[^\[]*[n].*" i.e. type=something[n]something_else

Is there any reason against adding this additional parsing into lib/lookup_table.c:audit_name_to_msg_type()?

If we can, then output_interpreted_node() can be re-factored so we are not parsing the same data twice for every event.

I am uncertain what effect of accepting this additional format would have when adding rules to the running audit system - i.e. audit_name_to_msg_type() is called by autrace/auditctl when parsing rules (ie the msgtype field name).

On Wed, 2014-10-01 at 14:54 -0400, Steve Grubb wrote: > > I am uncertain what effect of accepting this additional format would > > have when adding rules to the running audit system - i.e. > > audit_name_to_msg_type() is called by autrace/auditctl when parsing > > rules (ie the msgtype field name). > > I think ausearch-report.c might be the place that needs updating. So, could we modify output_interpreted_node() to no longer re-parse the [node=<node>] type=<type> msg=audit(<epochsecs>.<msecs>:<serial>) header and pass both the lnode and llist->e which has this data already as the code if (num == -1) { // see if we are older and wiser now. bptr = strchr(str, '['); if (bptr && bptr < ptr) { char *eptr; bptr++; eptr = strchr(bptr, ']'); if (eptr) { *eptr = 0; errno = 0; num = strtoul(bptr, NULL, 10); *eptr = ']'; if (errno) num = -1; } } } which parses for type=.*[n].* is no longer needed as we don't have that format any more?

On Wednesday, October 01, 2014 05:19:39 PM Steve Grubb wrote: > On Thursday, October 02, 2014 07:08:13 AM Burn Alting wrote: > > On Wed, 2014-10-01 at 14:54 -0400, Steve Grubb wrote: > > > > I am uncertain what effect of accepting this additional format would > > > > have when adding rules to the running audit system - i.e. > > > > audit_name_to_msg_type() is called by autrace/auditctl when parsing > > > > rules (ie the msgtype field name). > > > > > > I think ausearch-report.c might be the place that needs updating. > > > > So, could we modify output_interpreted_node() to no longer re-parse the > > > > [node=<node>] type=<type> msg=audit(<epochsecs>.<msecs>:<serial>) > > > > header and pass both the lnode and llist->e which has this data already > > as the code > > > > if (num == -1) { > > > > // see if we are older and wiser now. > > bptr = strchr(str, '['); > > if (bptr && bptr < ptr) { > > char *eptr; > > bptr++; > > eptr = strchr(bptr, ']'); > > if (eptr) { > > *eptr = 0; > > errno = 0; > > num = strtoul(bptr, NULL, 10); > > *eptr = ']'; > > if (errno) > > num = -1; > > } > > } > > } > > > > which parses for > > > > type=.*[n].* > > > > is no longer needed as we don't have that format any more? > > That is a very loose check for UNKNOWN[####]. If you see a performance > improvement by refactoring this function, please send a patch. The output > needs to be identical to the old way. Note that this section of code is on the "slow path". The value of interest extraction and matching is the performance critical area. Once an event is selected for output, the tty scrolling and buffering probably dominate the speed. So, I would take a patch to optimize this, but I don't expect it to make a big difference. -Steve

On Wed, 2014-10-01 at 17:19 -0400, Steve Grubb wrote: > On Thursday, October 02, 2014 07:08:13 AM Burn Alting wrote: > > On Wed, 2014-10-01 at 14:54 -0400, Steve Grubb wrote: > > > > I am uncertain what effect of accepting this additional format would > > > > have when adding rules to the running audit system - i.e. > > > > audit_name_to_msg_type() is called by autrace/auditctl when parsing > > > > rules (ie the msgtype field name). > > > > > > I think ausearch-report.c might be the place that needs updating. > > > > So, could we modify output_interpreted_node() to no longer re-parse the > > > > [node=<node>] type=<type> msg=audit(<epochsecs>.<msecs>:<serial>) > > > > header and pass both the lnode and llist->e which has this data already > > as the code > > > > if (num == -1) { > > > > // see if we are older and wiser now. > > bptr = strchr(str, '['); > > if (bptr && bptr < ptr) { > > > > char *eptr; > > bptr++; > > eptr = strchr(bptr, ']'); > > if (eptr) { > > > > *eptr = 0; > > errno = 0; > > num = strtoul(bptr, NULL, 10); > > *eptr = ']'; > > if (errno) > > > > num = -1; > > > > } > > > > } > > > > } > > > > which parses for > > > > type=.*[n].* > > > > is no longer needed as we don't have that format any more? > > That is a very loose check for UNKNOWN[####]. If you see a performance > improvement by refactoring this function, please send a patch. The output > needs to be identical to the old way. > > Thanks, > -Steve I can provide a patch to refactor this part of the code, but I want to confirm there is no longer a need to parse for type=some_text '[' integer_type ']' some_other_text

given my refactoring will rely upon the parsing already done by lib/lookup_table.c:audit_name_to_msg_type(). Remember this routine only parses for Given type=<type_value> then <type_value> is parsed for - a known string - a long integer number, n, found in the specific string "UNKNOWN[n]" - a long integer number, n, found in the specific string "n"

Thanks Steve, Patch to follow this weekend. Rgds On Wed, 2014-10-01 at 18:28 -0400, Steve Grubb wrote: > On Thursday, October 02, 2014 07:52:47 AM Burn Alting wrote: > > On Wed, 2014-10-01 at 17:19 -0400, Steve Grubb wrote: > > > On Thursday, October 02, 2014 07:08:13 AM Burn Alting wrote: > > > > On Wed, 2014-10-01 at 14:54 -0400, Steve Grubb wrote: > > > > > > I am uncertain what effect of accepting this additional format would > > > > > > have when adding rules to the running audit system - i.e. > > > > > > audit_name_to_msg_type() is called by autrace/auditctl when parsing > > > > > > rules (ie the msgtype field name). > > > > > > > > > > I think ausearch-report.c might be the place that needs updating. > > > > > > > > So, could we modify output_interpreted_node() to no longer re-parse the > > > > > > > > [node=<node>] type=<type> msg=audit(<epochsecs>.<msecs>:<serial>) > > > > > > > > header and pass both the lnode and llist->e which has this data already > > > > as the code > > > > > > > > if (num == -1) { > > > > > > > > // see if we are older and wiser now. > > > > bptr = strchr(str, '['); > > > > if (bptr && bptr < ptr) { > > > > > > > > char *eptr; > > > > bptr++; > > > > eptr = strchr(bptr, ']'); > > > > if (eptr) { > > > > > > > > *eptr = 0; > > > > errno = 0; > > > > num = strtoul(bptr, NULL, 10); > > > > *eptr = ']'; > > > > if (errno) > > > > > > > > num = -1; > > > > > > > > } > > > > > > > > } > > > > > > > > } > > > > > > > > which parses for > > > > > > > > type=.*[n].* > > > > > > > > is no longer needed as we don't have that format any more? > > > > > > That is a very loose check for UNKNOWN[####]. If you see a performance > > > improvement by refactoring this function, please send a patch. The output > > > needs to be identical to the old way. > > > > > > Thanks, > > > -Steve > > > > I can provide a patch to refactor this part of the code, but I want to > > confirm there is no longer a need to parse for > > > > type=some_text '[' integer_type ']' some_other_text > > While this may have been implied by the code, the fact is that [ ] would only > be in type fields when its unknown[####]. > > > > given my refactoring will rely upon the parsing already done by > > lib/lookup_table.c:audit_name_to_msg_type(). Remember this routine only > > parses for > > Given > > type=<type_value> > > then > > <type_value> > > is parsed for > > - a known string > > - a long integer number, n, found in the specific string > > "UNKNOWN[n]" > > - a long integer number, n, found in the specific string > > "n" > > These 3 formats are all that it can ever be. So, I think you have a correct > understanding. > > -Steve -- Linux-audit mailing list Linux-audit(a)redhat.com https://www.redhat.com/mailman/listinfo/linux-audit