Re: Refactoring src/ausearch-report.c:output_interpreted_node()

Burn Alting
2014-10-07T09:31:30Z

[-- Attachment #1: Type: text/plain, Size: 3918 bytes --]

All,

This patch prevents ausearch from parsing an event's "header" of node,
type, time and serial twice. That is
[node=<node>] type=<type> msg=audit(<epochsecs>.<msecs>:<serial>)

It does this by passing the llist->e value to output_interpreted_node()
as this structure holds the already parsed event "header" information.

The code does not change the ausearch -i output.

Rgds

On Thu, 2014-10-02 at 19:29 +1000, Burn Alting wrote:
> 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@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

[-- Attachment #2: audit-2.4_ausearch_refactor_01.patch --]
[-- Type: text/x-patch, Size: 4026 bytes --]

diff -Npru audit-2.4/src/ausearch-report.c audit-2.4_ausearch_refactor_01/src/ausearch-report.c
--- audit-2.4/src/ausearch-report.c 2014-08-25 02:39:20.000000000 +1000
+++ audit-2.4_ausearch_refactor_01/src/ausearch-report.c 2014-10-06 17:32:43.916773469 +1100
@@ -37,7 +37,7 @@
static void output_raw(llist *l);
static void output_default(llist *l);
static void output_interpreted(llist *l);
-static void output_interpreted_node(const lnode *n);
+static void output_interpreted_node(const lnode *n, const event *e);
static void interpret(char *name, char *val, int comma, int rtype);

/* The machine based on elf type */
@@ -125,10 +125,10 @@ static void output_interpreted(llist *l)
return;
}
if (n->type >= AUDIT_DAEMON_START && n->type < AUDIT_SYSCALL)
- output_interpreted_node(n);
+ output_interpreted_node(n, &(l->e));
else {
do {
- output_interpreted_node(n);
+ output_interpreted_node(n, &(l->e));
} while ((n=list_prev(l)));
}
}
@@ -137,21 +137,25 @@ static void output_interpreted(llist *l)
* This function will cycle through a single record and lookup each field's
* value that it finds.
*/
-static void output_interpreted_node(const lnode *n)
+static void output_interpreted_node(const lnode *n, const event *e)
{
char *ptr, *str = n->message, *node = NULL;
int found, comma = 0;
+ int num = n->type;
+ struct tm *btm;
+ char tmp[32];

// Reset these because each record could be different
machine = -1;
cur_syscall = -1;

- /* Check and see if we start with a node */
- if (str[0] == 'n') {
- ptr=strchr(str, ' ');
- if (ptr) {
- *ptr = 0;
- node = str;
+ /* Check and see if we start with a node
+ * If we do, and there is a space in the line
+ * move the pointer to the first character past
+ * the space
+ */
+ if (e->node) {
+ if ((ptr=strchr(str, ' ')) != NULL) {
str = ptr+1;
}
}
@@ -161,76 +165,34 @@ static void output_interpreted_node(cons
if (ptr == NULL) {
fprintf(stderr, "can't find time stamp\n");
return;
- } else {
- time_t t;
- int milli,num = n->type;
- unsigned long serial;
- struct tm *btm;
- char tmp[32];
- const char *bptr;
-
- *ptr++ = 0;
- 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;
- }
- }
- }
+ }
+
+ *ptr++ = 0; /* move to the start of the timestamp */

- // print everything up to it.
- if (num >= 0) {
- bptr = audit_msg_type_to_name(num);
- if (bptr) {
- if (node)
- printf("%s ", node);
- printf("type=%s msg=audit(", bptr);
- goto no_print;
- }
- }
- if (node)
- printf("%s ", node);
- printf("%s(", str);
+ // print everything up to it.
+ if (num >= 0) {
+ const char * bptr;
+ bptr = audit_msg_type_to_name(num);
+ if (bptr) {
+ if (e->node)
+ printf("node=%s ", e->node);
+ printf("type=%s msg=audit(", bptr);
+ goto no_print;
+ }
+ }
+ if (e->node)
+ printf("node=%s ", e->node);
+ printf("%s(", str);
no_print:

- // output formatted time.
- str = strchr(ptr, '.');
- if (str == NULL)
- return;
- *str++ = 0;
- errno = 0;
- t = strtoul(ptr, NULL, 10);
- if (errno)
- return;
- ptr = strchr(str, ':');
- if (ptr == NULL)
- return;
- *ptr++ = 0;
- milli = strtoul(str, NULL, 10);
- if (errno)
- return;
- str = strchr(ptr, ')');
- if(str == NULL)
- return;
- *str++ = 0;
- serial = strtoul(ptr, NULL, 10);
- if (errno)
- return;
- btm = localtime(&t);
- strftime(tmp, sizeof(tmp), "%x %T", btm);
- printf("%s", tmp);
- printf(".%03d:%lu) ", milli, serial);
- }
+ str = strchr(ptr, ')');
+ if(str == NULL)
+ return;
+ *str++ = 0;
+ btm = localtime(&e->sec);
+ strftime(tmp, sizeof(tmp), "%x %T", btm);
+ printf("%s", tmp);
+ printf(".%03d:%lu) ", e->milli, e->serial);

if (n->type == AUDIT_SYSCALL) {
a0 = n->a0;

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]