[PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[Deepa Dinamani]

struct timespec is not y2038 safe.
Audit timestamps are recorded in string format into
an audit buffer for a given context.
These mark the entry timestamps for the syscalls.
Use y2038 safe struct timespec64 to represent the times.
The log strings can handle this transition as strings can
hold upto 1024 characters.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Eric Paris <eparis@redhat.com>
Cc: linux-audit@redhat.com
---
 include/linux/audit.h |  4 ++--
 kernel/audit.c        | 10 +++++-----
 kernel/audit.h        |  2 +-
 kernel/auditsc.c      |  6 +++---
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 961a417..2f6a1123 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -335,7 +335,7 @@ static inline void audit_ptrace(struct task_struct *t)
 				/* Private API (for audit.c only) */
 extern unsigned int audit_serial(void);
 extern int auditsc_get_stamp(struct audit_context *ctx,
-			      struct timespec *t, unsigned int *serial);
+			      struct timespec64 *t, unsigned int *serial);
 extern int audit_set_loginuid(kuid_t loginuid);
 
 static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
@@ -510,7 +510,7 @@ static inline void __audit_seccomp(unsigned long syscall, long signr, int code)
 static inline void audit_seccomp(unsigned long syscall, long signr, int code)
 { }
 static inline int auditsc_get_stamp(struct audit_context *ctx,
-			      struct timespec *t, unsigned int *serial)
+			      struct timespec64 *t, unsigned int *serial)
 {
 	return 0;
 }
diff --git a/kernel/audit.c b/kernel/audit.c
index 22bb4f2..6c2f405 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1325,10 +1325,10 @@ unsigned int audit_serial(void)
 }
 
 static inline void audit_get_stamp(struct audit_context *ctx,
-				   struct timespec *t, unsigned int *serial)
+				   struct timespec64 *t, unsigned int *serial)
 {
 	if (!ctx || !auditsc_get_stamp(ctx, t, serial)) {
-		*t = CURRENT_TIME;
+		ktime_get_real_ts64(t);
 		*serial = audit_serial();
 	}
 }
@@ -1370,7 +1370,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 				     int type)
 {
 	struct audit_buffer	*ab	= NULL;
-	struct timespec		t;
+	struct timespec64	t;
 	unsigned int		uninitialized_var(serial);
 	int reserve = 5; /* Allow atomic callers to go up to five
 			    entries over the normal backlog limit */
@@ -1422,8 +1422,8 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 
 	audit_get_stamp(ab->ctx, &t, &serial);
 
-	audit_log_format(ab, "audit(%lu.%03lu:%u): ",
-			 t.tv_sec, t.tv_nsec/1000000, serial);
+	audit_log_format(ab, "audit(%llu.%03lu:%u): ",
+			 (unsigned long long)t.tv_sec, t.tv_nsec/1000000, serial);
 	return ab;
 }
 
diff --git a/kernel/audit.h b/kernel/audit.h
index cbbe6bb..029d674 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -111,7 +111,7 @@ struct audit_context {
 	enum audit_state    state, current_state;
 	unsigned int	    serial;     /* serial number for record */
 	int		    major;      /* syscall number */
-	struct timespec	    ctime;      /* time of syscall entry */
+	struct timespec64   ctime;      /* time of syscall entry */
 	unsigned long	    argv[4];    /* syscall arguments */
 	long		    return_code;/* syscall return code */
 	u64		    prio;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 62ab53d..ecebb3c 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1523,7 +1523,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
 		return;
 
 	context->serial     = 0;
-	context->ctime      = CURRENT_TIME;
+	ktime_get_real_ts64(&context->ctime);
 	context->in_syscall = 1;
 	context->current_state  = state;
 	context->ppid       = 0;
@@ -1932,13 +1932,13 @@ EXPORT_SYMBOL_GPL(__audit_inode_child);
 /**
  * auditsc_get_stamp - get local copies of audit_context values
  * @ctx: audit_context for the task
- * @t: timespec to store time recorded in the audit_context
+ * @t: timespec64 to store time recorded in the audit_context
  * @serial: serial value that is recorded in the audit_context
  *
  * Also sets the context as auditable.
  */
 int auditsc_get_stamp(struct audit_context *ctx,
-		       struct timespec *t, unsigned int *serial)
+		       struct timespec64 *t, unsigned int *serial)
 {
 	if (!ctx->in_syscall)
 		return 0;
-- 
1.9.1

Re: [PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[Steve Grubb]

On Wednesday, June 08, 2016 10:05:01 PM Deepa Dinamani wrote:
> struct timespec is not y2038 safe.
> Audit timestamps are recorded in string format into
> an audit buffer for a given context.
> These mark the entry timestamps for the syscalls.
> Use y2038 safe struct timespec64 to represent the times.
> The log strings can handle this transition as strings can
> hold upto 1024 characters.

Have you tested this with ausearch or any audit utilities? As an aside, a time 
stamp that is up to 1024 characters long is terribly wasteful considering how 
many events we get.

-Steve


> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Eric Paris <eparis@redhat.com>
> Cc: linux-audit@redhat.com
> ---
>  include/linux/audit.h |  4 ++--
>  kernel/audit.c        | 10 +++++-----
>  kernel/audit.h        |  2 +-
>  kernel/auditsc.c      |  6 +++---
>  4 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 961a417..2f6a1123 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -335,7 +335,7 @@ static inline void audit_ptrace(struct task_struct *t)
>  				/* Private API (for audit.c only) */
>  extern unsigned int audit_serial(void);
>  extern int auditsc_get_stamp(struct audit_context *ctx,
> -			      struct timespec *t, unsigned int *serial);
> +			      struct timespec64 *t, unsigned int *serial);
>  extern int audit_set_loginuid(kuid_t loginuid);
> 
>  static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
> @@ -510,7 +510,7 @@ static inline void __audit_seccomp(unsigned long
> syscall, long signr, int code) static inline void audit_seccomp(unsigned
> long syscall, long signr, int code) { }
>  static inline int auditsc_get_stamp(struct audit_context *ctx,
> -			      struct timespec *t, unsigned int *serial)
> +			      struct timespec64 *t, unsigned int *serial)
>  {
>  	return 0;
>  }
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 22bb4f2..6c2f405 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1325,10 +1325,10 @@ unsigned int audit_serial(void)
>  }
> 
>  static inline void audit_get_stamp(struct audit_context *ctx,
> -				   struct timespec *t, unsigned int *serial)
> +				   struct timespec64 *t, unsigned int *serial)
>  {
>  	if (!ctx || !auditsc_get_stamp(ctx, t, serial)) {
> -		*t = CURRENT_TIME;
> +		ktime_get_real_ts64(t);
>  		*serial = audit_serial();
>  	}
>  }
> @@ -1370,7 +1370,7 @@ struct audit_buffer *audit_log_start(struct
> audit_context *ctx, gfp_t gfp_mask, int type)
>  {
>  	struct audit_buffer	*ab	= NULL;
> -	struct timespec		t;
> +	struct timespec64	t;
>  	unsigned int		uninitialized_var(serial);
>  	int reserve = 5; /* Allow atomic callers to go up to five
>  			    entries over the normal backlog limit */
> @@ -1422,8 +1422,8 @@ struct audit_buffer *audit_log_start(struct
> audit_context *ctx, gfp_t gfp_mask,
> 
>  	audit_get_stamp(ab->ctx, &t, &serial);
> 
> -	audit_log_format(ab, "audit(%lu.%03lu:%u): ",
> -			 t.tv_sec, t.tv_nsec/1000000, serial);
> +	audit_log_format(ab, "audit(%llu.%03lu:%u): ",
> +			 (unsigned long long)t.tv_sec, t.tv_nsec/1000000, serial);
>  	return ab;
>  }
> 
> diff --git a/kernel/audit.h b/kernel/audit.h
> index cbbe6bb..029d674 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -111,7 +111,7 @@ struct audit_context {
>  	enum audit_state    state, current_state;
>  	unsigned int	    serial;     /* serial number for record */
>  	int		    major;      /* syscall number */
> -	struct timespec	    ctime;      /* time of syscall entry */
> +	struct timespec64   ctime;      /* time of syscall entry */
>  	unsigned long	    argv[4];    /* syscall arguments */
>  	long		    return_code;/* syscall return code */
>  	u64		    prio;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 62ab53d..ecebb3c 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1523,7 +1523,7 @@ void __audit_syscall_entry(int major, unsigned long
> a1, unsigned long a2, return;
> 
>  	context->serial     = 0;
> -	context->ctime      = CURRENT_TIME;
> +	ktime_get_real_ts64(&context->ctime);
>  	context->in_syscall = 1;
>  	context->current_state  = state;
>  	context->ppid       = 0;
> @@ -1932,13 +1932,13 @@ EXPORT_SYMBOL_GPL(__audit_inode_child);
>  /**
>   * auditsc_get_stamp - get local copies of audit_context values
>   * @ctx: audit_context for the task
> - * @t: timespec to store time recorded in the audit_context
> + * @t: timespec64 to store time recorded in the audit_context
>   * @serial: serial value that is recorded in the audit_context
>   *
>   * Also sets the context as auditable.
>   */
>  int auditsc_get_stamp(struct audit_context *ctx,
> -		       struct timespec *t, unsigned int *serial)
> +		       struct timespec64 *t, unsigned int *serial)
>  {
>  	if (!ctx->in_syscall)
>  		return 0;

Re: [PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[Richard Guy Briggs]

On 16/06/09, Steve Grubb wrote:
> On Wednesday, June 08, 2016 10:05:01 PM Deepa Dinamani wrote:
> > struct timespec is not y2038 safe.
> > Audit timestamps are recorded in string format into
> > an audit buffer for a given context.
> > These mark the entry timestamps for the syscalls.
> > Use y2038 safe struct timespec64 to represent the times.
> > The log strings can handle this transition as strings can
> > hold upto 1024 characters.
> 
> Have you tested this with ausearch or any audit utilities? As an aside, a time 
> stamp that is up to 1024 characters long is terribly wasteful considering how 
> many events we get.

Steve,

I don't expect the size of the time stamp text to change since the
format isn't being changed and I don't expect the date stamp text length
to change until Y10K, but you never know what will happen in 8
millenia...  (Who knows, maybe that damn Linux server in my basement
will still be running then...)

Isn't the maximum message length MAX_AUDIT_MESSAGE_LENGTH (8970 octets)?

> -Steve
> 
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: Eric Paris <eparis@redhat.com>
> > Cc: linux-audit@redhat.com
> > ---
> >  include/linux/audit.h |  4 ++--
> >  kernel/audit.c        | 10 +++++-----
> >  kernel/audit.h        |  2 +-
> >  kernel/auditsc.c      |  6 +++---
> >  4 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 961a417..2f6a1123 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -335,7 +335,7 @@ static inline void audit_ptrace(struct task_struct *t)
> >  				/* Private API (for audit.c only) */
> >  extern unsigned int audit_serial(void);
> >  extern int auditsc_get_stamp(struct audit_context *ctx,
> > -			      struct timespec *t, unsigned int *serial);
> > +			      struct timespec64 *t, unsigned int *serial);
> >  extern int audit_set_loginuid(kuid_t loginuid);
> > 
> >  static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
> > @@ -510,7 +510,7 @@ static inline void __audit_seccomp(unsigned long
> > syscall, long signr, int code) static inline void audit_seccomp(unsigned
> > long syscall, long signr, int code) { }
> >  static inline int auditsc_get_stamp(struct audit_context *ctx,
> > -			      struct timespec *t, unsigned int *serial)
> > +			      struct timespec64 *t, unsigned int *serial)
> >  {
> >  	return 0;
> >  }
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 22bb4f2..6c2f405 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1325,10 +1325,10 @@ unsigned int audit_serial(void)
> >  }
> > 
> >  static inline void audit_get_stamp(struct audit_context *ctx,
> > -				   struct timespec *t, unsigned int *serial)
> > +				   struct timespec64 *t, unsigned int *serial)
> >  {
> >  	if (!ctx || !auditsc_get_stamp(ctx, t, serial)) {
> > -		*t = CURRENT_TIME;
> > +		ktime_get_real_ts64(t);
> >  		*serial = audit_serial();
> >  	}
> >  }
> > @@ -1370,7 +1370,7 @@ struct audit_buffer *audit_log_start(struct
> > audit_context *ctx, gfp_t gfp_mask, int type)
> >  {
> >  	struct audit_buffer	*ab	= NULL;
> > -	struct timespec		t;
> > +	struct timespec64	t;
> >  	unsigned int		uninitialized_var(serial);
> >  	int reserve = 5; /* Allow atomic callers to go up to five
> >  			    entries over the normal backlog limit */
> > @@ -1422,8 +1422,8 @@ struct audit_buffer *audit_log_start(struct
> > audit_context *ctx, gfp_t gfp_mask,
> > 
> >  	audit_get_stamp(ab->ctx, &t, &serial);
> > 
> > -	audit_log_format(ab, "audit(%lu.%03lu:%u): ",
> > -			 t.tv_sec, t.tv_nsec/1000000, serial);
> > +	audit_log_format(ab, "audit(%llu.%03lu:%u): ",
> > +			 (unsigned long long)t.tv_sec, t.tv_nsec/1000000, serial);
> >  	return ab;
> >  }
> > 
> > diff --git a/kernel/audit.h b/kernel/audit.h
> > index cbbe6bb..029d674 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -111,7 +111,7 @@ struct audit_context {
> >  	enum audit_state    state, current_state;
> >  	unsigned int	    serial;     /* serial number for record */
> >  	int		    major;      /* syscall number */
> > -	struct timespec	    ctime;      /* time of syscall entry */
> > +	struct timespec64   ctime;      /* time of syscall entry */
> >  	unsigned long	    argv[4];    /* syscall arguments */
> >  	long		    return_code;/* syscall return code */
> >  	u64		    prio;
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 62ab53d..ecebb3c 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1523,7 +1523,7 @@ void __audit_syscall_entry(int major, unsigned long
> > a1, unsigned long a2, return;
> > 
> >  	context->serial     = 0;
> > -	context->ctime      = CURRENT_TIME;
> > +	ktime_get_real_ts64(&context->ctime);
> >  	context->in_syscall = 1;
> >  	context->current_state  = state;
> >  	context->ppid       = 0;
> > @@ -1932,13 +1932,13 @@ EXPORT_SYMBOL_GPL(__audit_inode_child);
> >  /**
> >   * auditsc_get_stamp - get local copies of audit_context values
> >   * @ctx: audit_context for the task
> > - * @t: timespec to store time recorded in the audit_context
> > + * @t: timespec64 to store time recorded in the audit_context
> >   * @serial: serial value that is recorded in the audit_context
> >   *
> >   * Also sets the context as auditable.
> >   */
> >  int auditsc_get_stamp(struct audit_context *ctx,
> > -		       struct timespec *t, unsigned int *serial)
> > +		       struct timespec64 *t, unsigned int *serial)
> >  {
> >  	if (!ctx->in_syscall)
> >  		return 0;
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

Re: [PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[Steve Grubb]

On Thursday, June 09, 2016 07:59:43 PM Richard Guy Briggs wrote:
> On 16/06/09, Steve Grubb wrote:
> > On Wednesday, June 08, 2016 10:05:01 PM Deepa Dinamani wrote:
> > > struct timespec is not y2038 safe.
> > > Audit timestamps are recorded in string format into
> > > an audit buffer for a given context.
> > > These mark the entry timestamps for the syscalls.
> > > Use y2038 safe struct timespec64 to represent the times.
> > > The log strings can handle this transition as strings can
> > > hold upto 1024 characters.
> > 
> > Have you tested this with ausearch or any audit utilities? As an aside, a
> > time stamp that is up to 1024 characters long is terribly wasteful
> > considering how many events we get.
> 
> Steve,
> 
> I don't expect the size of the time stamp text to change since the
> format isn't being changed and I don't expect the date stamp text length
> to change until Y10K, but you never know what will happen in 8
> millenia...  (Who knows, maybe that damn Linux server in my basement
> will still be running then...)
> 
> Isn't the maximum message length MAX_AUDIT_MESSAGE_LENGTH (8970 octets)?

Bytes, yes. But I was thinking that if its going to get big we should consider 
switching from a base 10 representation to base 16. That would give us back a 
few bytes. We discuss this on the linux-audit list rather than the main list.

-Steve
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[Richard Guy Briggs]

On 16/06/09, Steve Grubb wrote:
> On Thursday, June 09, 2016 07:59:43 PM Richard Guy Briggs wrote:
> > On 16/06/09, Steve Grubb wrote:
> > > On Wednesday, June 08, 2016 10:05:01 PM Deepa Dinamani wrote:
> > > > struct timespec is not y2038 safe.
> > > > Audit timestamps are recorded in string format into
> > > > an audit buffer for a given context.
> > > > These mark the entry timestamps for the syscalls.
> > > > Use y2038 safe struct timespec64 to represent the times.
> > > > The log strings can handle this transition as strings can
> > > > hold upto 1024 characters.
> > > 
> > > Have you tested this with ausearch or any audit utilities? As an aside, a
> > > time stamp that is up to 1024 characters long is terribly wasteful
> > > considering how many events we get.
> > 
> > Steve,
> > 
> > I don't expect the size of the time stamp text to change since the
> > format isn't being changed and I don't expect the date stamp text length
> > to change until Y10K, but you never know what will happen in 8
> > millenia...  (Who knows, maybe that damn Linux server in my basement
> > will still be running then...)
> > 
> > Isn't the maximum message length MAX_AUDIT_MESSAGE_LENGTH (8970 octets)?
> 
> Bytes, yes. But I was thinking that if its going to get big we should consider 
> switching from a base 10 representation to base 16. That would give us back a 
> few bytes. We discuss this on the linux-audit list rather than the main list.

This seems like a false economy to me.  If I understand correctly, it
will be 285 years before we roll the next text digit.  The next binary
digit in the internal kernel format is in 22 years.

I know there have been discussions about changing to a binary format,
which seems to have a lot more to offer than breaking the current format
for a few bytes.

Is this not the linux-audit main list?  Is there another one I am
missing?

> -Steve

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[Paul Moore]

On Thu, Jun 9, 2016 at 7:59 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 16/06/09, Steve Grubb wrote:
>> On Wednesday, June 08, 2016 10:05:01 PM Deepa Dinamani wrote:
>> > struct timespec is not y2038 safe.
>> > Audit timestamps are recorded in string format into
>> > an audit buffer for a given context.
>> > These mark the entry timestamps for the syscalls.
>> > Use y2038 safe struct timespec64 to represent the times.
>> > The log strings can handle this transition as strings can
>> > hold upto 1024 characters.
>>
>> Have you tested this with ausearch or any audit utilities? As an aside, a time
>> stamp that is up to 1024 characters long is terribly wasteful considering how
>> many events we get.
>
> Steve,
>
> I don't expect the size of the time stamp text to change since the
> format isn't being changed and I don't expect the date stamp text length
> to change until Y10K, but you never know what will happen in 8
> millenia...  (Who knows, maybe that damn Linux server in my basement
> will still be running then...)

Yeah, I'm not really worried about the string date field growing; this
is more an internal implementation detail to make sure we can keep the
lights on in a few decades from now.

Deepa, I'm not going to merge your patchset because I'm guessing all
your timespec64 patches will likely go in at once, but you are free to
add my ack if you need to respin.

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul moore
www.paul-moore.com

Re: [PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[Deepa Dinamani]

On Thu, Jun 9, 2016 at 7:31 AM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Wednesday, June 08, 2016 10:05:01 PM Deepa Dinamani wrote:
>> Audit timestamps are recorded in string format into
>> an audit buffer for a given context.
>> These mark the entry timestamps for the syscalls.
>> Use y2038 safe struct timespec64 to represent the times.
>> The log strings can handle this transition as strings can
>> hold upto 1024 characters.
>
> Have you tested this with ausearch or any audit utilities? As an aside, a time
> stamp that is up to 1024 characters long is terribly wasteful considering how
> many events we get.

/* AUDIT_BUFSIZ is the size of the temporary buffer used for formatting
 * audit records.  Since printk uses a 1024 byte buffer, this buffer
 * should be at least that large. */
#define AUDIT_BUFSIZ 1024

The commit text is pointing out that the reserve space ensured in each
call to audit_log_vformat is already much more than is needed by this
call from audit_log_start.

Also, since struct timespec64 is already the same as struct timespec
on 64-bit systems, there is really no functional change except on
32-bit machines.

Let me know if you want me to try it out on a 32-bit system.

-Deepa
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038