Re: [Y2038] [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 
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: [Y2038] [PATCH 17/21] audit: Use timespec64 to represent audit timestamps

[Deepa Dinamani]

On Thu, Jun 9, 2016 at 7:31 AM, Steve Grubb  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

Re: [Y2038] [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: [Y2038] [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 
> Cc: Paul Moore 
> Cc: Eric Paris 
> Cc: linux-au...@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 intuninitialized_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, , );
> 
> - audit_log_format(ab, "audit(%lu.%03lu:%u): ",
> -  t.tv_sec, t.tv_nsec/100, serial);
> + audit_log_format(ab, "audit(%llu.%03lu:%u): ",
> +  (unsigned long long)t.tv_sec, t.tv_nsec/100, 
> 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_statestate, current_state;
>   unsigned intserial; /* 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 */
>   longreturn_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(>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