audit_status in kernel

Hello,

I was looking at a new kernel and see that the audit_status structure has changed. The first member of the structure is a bit mask that tells what all is in the structure. So, if we add this:

__u32 version; /* audit api version number */
__u32 backlog_wait_time;/* message queue wait timeout */
};

Then we need to have a define for those two:

#define AUDIT_STATUS_BACKLOG_LIMIT 0x0010
+#define AUDIT_STATTUS_VERSION 0x0020
-#define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0020
+#define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0040

IOW, each entry in the structure is supposed to have a mask value.

-Steve

Actually, I think the problems are worse. We have the issue of an expanding data structure over time as new things get added. But yet we have a fixed sized audit_status structure. I could copy that into the audit package's source code so that I have the new structure definition. But I will have to do the same thing each time. Also, how would an old kernel tolerate a bigger audit_status structure being sent with AUDIT_SET commands by a new auditctl?

What we should do is leave AUDIT_GET the way it was. We should then define AUDIT_GET_EXT and then use it a lot like audit_rule_data which has

struct audit_status_ext {
        __u32           field_count;
        __u32           fields[AUDIT_MAX_FIELDS];
        __u32           values[AUDIT_MAX_FIELDS];
};

Then insert the field identifier in fields and the value in the other. This way the format is defined once and we can reuse it for a long time.

AUDIT_GET. New auditctl would try AUDIT_GET_EXT and if that's not recognized, drop back to AUDIT_GET. Then one day down the road we remove AUDIT_GET in the kernel.

This is how we did the migration from AUDIT_RULES to AUDIT_RULES_DATA.

-Steve

That's not what the bit does. The bit tells what portions of the struct contain updates the kernel should upon. Notice that the 'lost' and 'backlog' fields have no bit assigned.

If the version were a writable field then a bit would be necessary. But as of right now, it isn't writable, therefore the bit serves no purpose. Just like 'lost' and 'backlog'

Sure, every program is compiled with a fixed size. This is very common. I don't recognize the problem. You have to compile a new userspace to make use of new functions/features. No matter what.

As userspace comes to understand new parts of the structure userspace would need to grow the definition. Agreed. Every design has this issue. Userspace can't understand a feature unless you tell it.

Unknown bits have always been ignored. I thought we returned EINVAL on unknown bits, but it seems we don't. We could change that going forward, but, it doesn't matter. Userspace must query the kernel for the version. And can then obviously know what is/isn't supported.

That adds absolutely nothing except wasted memory and greater readability problems... We'd be sending around gigantic empty stuctures
for no gain... (although on AUDIT_FEATURE_SET the current interface is
totally craptastic as all but one of the fields is typically useless, but at least this is the craptastic we have today)

Great. But this isn't needed here. This isn't an ABI change. Old userspace will run just fine on new kernels. New userspace will run just fine on old kernels. (Although (poorly coded userspace sending) new features will be silently ignored instead of rejected, as I would prefer)

It really is easy. On GET userspace just needs to use

	struct audit_status     s;
	memset(&s, 0, sizeof(s));
	/* guard against past and future API changes */
	memcpy(&s, data, min_t(size_t, sizeof(s), nlmsg_len(nlh)));

if (s.version == 0)
no new features, legacy api (notice this works because you memset the
userspace status struct before you copied in the kernel verion. So stuff at the end is all 0's

if (s.version == AUDIT_VERSION_BACKLOG_LIMIT)
version field, but nothing else

if (s.version >= AUDIT_VERSION_BACKLOG_WAIT_TIME)
you can set the version field!

And the struct can keep growing and keep being used! Both ends know how to handle things, all with no new API, no new deprecation, no breaking working code. I have actually written kernel<->userspace APIs before. Admittedly this time I was starting with junk, but we still found a way to do it allowing us to grow into the future and keep everything working just the way it always has.

-Eric