Skip to contentSkip to conversation

Fix the error in the output of "auditctl -s" when auditd is stoped

PATCH In review Chu Li 4 participants 6 messages Aug 7, 2008 – Aug 8, 2008

lore
Share & tools
lore

Chu Li

author
#0 lore raw

PATCH 1/1 Fix the error in the output of "auditctl -s" when auditd is stoped

Hi Steve,
When auditd is stoped, "auditctl -s" will show "pid=0". I think it's not
correct information. It's better to tell users "auditd not started".

---

src/auditctl.c +8 -5
index 10894f9..b26dd82 100755
@@ -1411,12 +1411,15 @@ static int audit_print_reply(struct audit_reply *rep)
 			printed = 1;
 			return 0;
 		case AUDIT_GET:
-			printf("AUDIT_STATUS: enabled=%d flag=%d pid=%d"
-			" rate_limit=%d backlog_limit=%d lost=%d backlog=%u\n",
+			printf("AUDIT_STATUS: enabled=%d flag=%d"
+			" rate_limit=%d backlog_limit=%d lost=%d backlog=%u ",
 			rep->status->enabled, rep->status->failure,
-			rep->status->pid, rep->status->rate_limit,
-			rep->status->backlog_limit, rep->status->lost,
-			rep->status->backlog);
+			rep->status->rate_limit, rep->status->backlog_limit,
+			rep->status->lost, rep->status->backlog);
+			if(rep->status->pid != 0)
+				printf("pid=%d\n", rep->status->pid);
+			else
+				printf("auditd_not_started\n");
 			printed = 1;
 			return 0;
 		case AUDIT_LIST:

Regards
Chu Li

#1 lore raw
Quoted 3 lines · Chu Li

Hi Steve,
When auditd is stoped, "auditctl -s" will show "pid=0". I think it's not
correct information. It's better to tell users "auditd not started".

We do try to keep the whole key=value pair thing in audit records. I'd be willing to go with something like -1 to make it really clear, but with the number of complaints about the inconsistencies of audit records from people like John Dennis I'm not sure I'm a fan of this patch....

-Eric

Quoted 37 lines

---

src/auditctl.c +8 -5
index 10894f9..b26dd82 100755
@@ -1411,12 +1411,15 @@ static int audit_print_reply(struct audit_reply *rep)
 			printed = 1;
 			return 0;
 		case AUDIT_GET:
-			printf("AUDIT_STATUS: enabled=%d flag=%d pid=%d"
-			" rate_limit=%d backlog_limit=%d lost=%d backlog=%u\n",
+			printf("AUDIT_STATUS: enabled=%d flag=%d"
+			" rate_limit=%d backlog_limit=%d lost=%d backlog=%u ",
 			rep->status->enabled, rep->status->failure,
-			rep->status->pid, rep->status->rate_limit,
-			rep->status->backlog_limit, rep->status->lost,
-			rep->status->backlog);
+			rep->status->rate_limit, rep->status->backlog_limit,
+			rep->status->lost, rep->status->backlog);
+			if(rep->status->pid != 0)
+				printf("pid=%d\n", rep->status->pid);
+			else
+				printf("auditd_not_started\n");
 			printed = 1;
 			return 0;
 		case AUDIT_LIST:

Regards
Chu Li

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

#2 lore raw
Quoted 4 lines · On Thursday 07 August 2008 09:39:37 Eric Paris wrote:
Quoted 2 lines

  When auditd is stoped, "auditctl -s" will show "pid=0". I think it's not correct information. It's better to tell users "auditd not started".

We do try to keep the whole key=value pair thing in audit records.

This is for the display when you type auditctl -s and doesn't have anything to do with audit records.

Quoted 3 lines

I'd be willing to go with something like -1 to make it really clear, but with the number of complaints about the inconsistencies of audit records from people like John Dennis I'm not sure I'm a fan of this patch....

I don't think that's an issue since this is not in the records. My only concern is what this might do to our test suites. For the moment, I'm just trying to finish off what we will have in RHEL5 without changes to API that might cause any regressions in the test suites.

Around the time that Fedora 11 work starts, I'd like to start making changes to clean things up and have new ideas. That time is coming soon...but not yet.

-Steve

#3 lore raw
In reply to Eric Paris
Quoted 7 lines · Eric Paris
Quoted 4 lines · Chu Li

Hi Steve,
When auditd is stoped, "auditctl -s" will show "pid=0". I think it's not
correct information. It's better to tell users "auditd not started".

Quoted 5 lines

We do try to keep the whole key=value pair thing in audit records. I'd be willing to go with something like -1 to make it really clear, but with the number of complaints about the inconsistencies of audit records from people like John Dennis I'm not sure I'm a fan of this patch....

Thank you Eric! We should be moving toward consistent formatting of audit records and one of the most important consistencies to achieve if we do modify the records is that all data is in the form of key=value. Embedding random strings into records makes parsing difficult especially when the record format varies by kernel version, which this patch would do (at least the proposed string didn't contain any embedded spaces!)

NAK to this patch.

I think Eric's suggestion of pid=-1 is a good one.

Signature · 1 line
John Dennis <[email protected]>
#4 lore raw
In reply to Chu Li
Quoted 2 lines · On Thursday 07 August 2008 00:50:45 Chu Li wrote:

  When auditd is stoped, "auditctl -s" will show "pid=0". I think it's not correct information. It's better to tell users "auditd not started".

At the moment, I don't want to change this behavior. In about a month, I think we can make updates that change the external behavior. If you want, we can document this better in man pages that audit pid of 0 means its not running.

Thanks,
-Steve

author
#5 lore raw

Hi Steve,

Quoted 2 lines

we can make updates that change the external behavior. If you want, we can document this better in man pages that audit pid of 0 means its not running.

I agree with it. At least it's necessary to add some simple explanation about
"pid=0" in the manual.

Regards
Chu Li

Quoted 18 lines

-----Original Message-----
From: Steve Grubb [mailto:[email protected]]
Sent: Friday, August 08, 2008 5:02 AM
To: Chu Li
Cc: 'linux-audit'
Subject: Re: [Patch]Fix the error in the output of "auditctl -s" when auditd is stoped

Quoted 2 lines · On Thursday 07 August 2008 00:50:45 Chu Li wrote:

When auditd is stoped, "auditctl -s" will show "pid=0". I think it's not
correct information. It's better to tell users "auditd not started".

At the moment, I don't want to change this behavior. In about a month, I think we can make updates that change the external behavior. If you want, we can document this better in man pages that audit pid of 0 means its not running.

Thanks,
-Steve