Skip to contentSkip to conversation

Add the field checking for missing value after opration

PATCH Applied Zhang Xiliang 2 participants 4 messages Aug 6, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/5 Add the field checking for missing value after opration

Hello Steve,

I think the missing value for -F should be checking.

For example,
auditctl -a exit,always -F pid=

No error message is output and "pid=0" is added to rule.

---
 lib/deprecated.c |    3 +++
 lib/libaudit.c   |    3 +++
 src/auditctl.c   |    5 +++++
 3 files changed, 11 insertions(+), 0 deletions(-)
lib/deprecated.c +3 -0
index e05e826..5a1c04f 100644
@@ -230,6 +230,9 @@ int audit_rule_fieldpair(struct audit_rule *rule, const char *pair, int flags)
 	if (v == NULL || f == v)
 		return -1;
 
+	if (*v == NULL)
+		return -20;
+
 	audit_msg(LOG_DEBUG,"pair=%s\n", f);
 	if ((field = audit_name_to_field(f)) < 0) 
 		return -2;
lib/libaudit.c +3 -0
index 4bedfaf..6ec15d8 100644
@@ -823,6 +823,9 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
 	if (v == NULL || f == v)
 		return -1;
 
+	if (*v == NULL)
+		return -20;
+
 	if ((field = audit_name_to_field(f)) < 0) 
 		return -2;
 
src/auditctl.c +5 -0
index 48f1369..3958afb 100644
@@ -847,6 +847,11 @@ static int setopt(int count, char *vars[])
 					"Key field needs a watch or syscall given prior to it\n");
 				retval = -1;
 				break;
+			case -20:
+				fprintf(stderr,
+					"-F missing value after opration for %s\n", optarg);
+				retval = -1;
+				break;
 			default:
 				retval = -1;
 				break;
#1 lore raw

Hello Steve,

Review

Quoted 2 lines
(quoted diff) +0 -0
 	if (v == NULL || f == v)
 		return -1;

I have some questions for "f == v".

I think it only checks the address of "f" and "v". It seems unuseful.
If we need to check whether the value is equal to the field, I think we should use strcmp(). What your opinion about it?
If you agree with me, I will make another patch for it.

#2 lore raw
Quoted 8 lines · On Wednesday 06 August 2008 04:46:27 Zhang Xiliang wrote:

Hello Steve,

Review

Quoted 2 lines
(quoted diff) +0 -0
 	if (v == NULL || f == v)
 		return -1;

I have some questions for "f == v".

I think it only checks the address of "f" and "v".

Yes, this was in attempt to make sure that they didn't type -F =10. In that case v will equal f because they start at the same address.

-Steve

#3 lore raw
In reply to Zhang Xiliang
Quoted 4 lines · On Wednesday 06 August 2008 04:12:42 Zhang Xiliang wrote:

I think the missing value for -F should be checking.

For example,
auditctl -a exit,always -F pid=

<snip>

Quoted 2 lines

+       if (*v == NULL)
+               return -20;

I changed these to (*v == 0) since it is a char and not a pointer. Thanks for the patch! Applied

-Steve