* Re: Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c
From: Ben Hutchings @ 2016-06-21 18:20 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: security, Pengfei Wang, Krinke, Jens, Oleg Nesterov, linux-audit
In-Reply-To: <20160621181431.GD25615@madcap2.tricolour.ca>
[-- Attachment #1.1: Type: text/plain, Size: 2652 bytes --]
On Tue, 2016-06-21 at 14:14 -0400, Richard Guy Briggs wrote:
> On 2016-06-21 10:51, Ben Hutchings wrote:
> > On Tue, 2016-06-21 at 10:37 +0100, Pengfei Wang wrote:
> > > >
> > > > 在 2016年6月20日，下午8:18，Oleg Nesterov <oleg@redhat.com> 写道：
> > > >
> > > > Not that I understand this report, but
> > > >
> > > > On 06/20, Richard Guy Briggs wrote:
> > > > >
> > > > > This function is only ever called by __audit_free(), which is only ever
> > > > > called on failure of task creation or on exit of the task, so in neither
> > > > > case can anything else change it.
> > > >
> > > > How so?
> > > >
> > > > Another thread or CLONE_VM task or /proc/pid/mem can change the user-space
> > > > memory in parallel.
> > > >
> > > > Oleg.
> > >
> > >
> > > Exactly, by saying “change the data”, I mean the modification from
> > > malicious users with crafted operations on the user space memory
> > > directly, rather than the normal operations within the audit
> > > subsystem in Linux. Moreover, since the copy operations from the user
> > > space are not protected by any locks or synchronization primitives,
> > > changing the data under race condition is feasible I think. Besides,
> > > there isn’t any visible checking step in the code to guarantee the
> > > consistency between the two copy operations.
> > >
> > > Here I would like to figure out what the consequences really are once
> > > the data is changed between the two copy operations, such as changing
> > > a none-control string to a control string but process it as a none-
> > > control string that has no control chars. I think problems will
> > > happen.
> >
> > So far as userland can see, kernel log lines are separated by newlines.
>
> Newlines are control characters that would be caught by that filter.
> That filter catches '"', < 0x21, > 0x7e.
>
> > If we fail to escape a newline, that makes it possible to inject
> > arbitrary log lines into the kernel log, which may be misleading to the
> > administrator or to software parsing the log.
>
> So, this is addressed, but I'm still trying to assess the danger of this
> repeated call to copy_from_user().
The problem is that newlines can be added to the strings by another
task between the first pass that checks for control characters and the
second pass that copies them to the log.
Ben.
--
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had
thought.
... I realized that a large part of my life from then on was going to
be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c
From: Richard Guy Briggs @ 2016-06-21 18:17 UTC (permalink / raw)
To: Pengfei Wang; +Cc: security, linux-audit, Krinke, Jens, Oleg Nesterov
In-Reply-To: <480FAE99-E4E1-42D0-ABD5-8DC24A7EC9BB@gmail.com>
On 2016-06-21 10:37, Pengfei Wang wrote:
>
> > 在 2016年6月20日，下午8:18，Oleg Nesterov <oleg@redhat.com> 写道：
> >
> > Not that I understand this report, but
> >
> > On 06/20, Richard Guy Briggs wrote:
> >>
> >> This function is only ever called by __audit_free(), which is only ever
> >> called on failure of task creation or on exit of the task, so in neither
> >> case can anything else change it.
> >
> > How so?
> >
> > Another thread or CLONE_VM task or /proc/pid/mem can change the user-space
> > memory in parallel.
> >
> > Oleg.
>
> Exactly, by saying “change the data”, I mean the modification from
> malicious users with crafted operations on the user space memory
> directly, rather than the normal operations within the audit subsystem
> in Linux. Moreover, since the copy operations from the user space are
> not protected by any locks or synchronization primitives, changing the
> data under race condition is feasible I think. Besides, there isn’t
> any visible checking step in the code to guarantee the consistency
> between the two copy operations.
Ok, fair enough. So if that is the case, then the iterative call to
copy_from_user() within each loop isn't safe either and needs a lock.
> Here I would like to figure out what the consequences really are once
> the data is changed between the two copy operations, such as changing
> a none-control string to a control string but process it as a
> none-control string that has no control chars. I think problems will
> happen.
If it is possible, I agree that it would cause problems.
So, next question, is each loop itself safe?
> Pengfei
- 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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Re: Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c
From: Richard Guy Briggs @ 2016-06-21 18:14 UTC (permalink / raw)
To: Ben Hutchings
Cc: security, Pengfei Wang, Krinke, Jens, Oleg Nesterov, linux-audit
In-Reply-To: <1466502671.27155.185.camel@decadent.org.uk>
On 2016-06-21 10:51, Ben Hutchings wrote:
> On Tue, 2016-06-21 at 10:37 +0100, Pengfei Wang wrote:
> > >
> > > 在 2016年6月20日，下午8:18，Oleg Nesterov <oleg@redhat.com> 写道：
> > >
> > > Not that I understand this report, but
> > >
> > > On 06/20, Richard Guy Briggs wrote:
> > > >
> > > > This function is only ever called by __audit_free(), which is only ever
> > > > called on failure of task creation or on exit of the task, so in neither
> > > > case can anything else change it.
> > >
> > > How so?
> > >
> > > Another thread or CLONE_VM task or /proc/pid/mem can change the user-space
> > > memory in parallel.
> > >
> > > Oleg.
> >
> >
> > Exactly, by saying “change the data”, I mean the modification from
> > malicious users with crafted operations on the user space memory
> > directly, rather than the normal operations within the audit
> > subsystem in Linux. Moreover, since the copy operations from the user
> > space are not protected by any locks or synchronization primitives,
> > changing the data under race condition is feasible I think. Besides,
> > there isn’t any visible checking step in the code to guarantee the
> > consistency between the two copy operations.
> >
> > Here I would like to figure out what the consequences really are once
> > the data is changed between the two copy operations, such as changing
> > a none-control string to a control string but process it as a none-
> > control string that has no control chars. I think problems will
> > happen.
>
> So far as userland can see, kernel log lines are separated by newlines.
Newlines are control characters that would be caught by that filter.
That filter catches '"', < 0x21, > 0x7e.
> If we fail to escape a newline, that makes it possible to inject
> arbitrary log lines into the kernel log, which may be misleading to the
> administrator or to software parsing the log.
So, this is addressed, but I'm still trying to assess the danger of this
repeated call to copy_from_user().
> Ben.
- 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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Re: [Y2038] [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
From: Arnd Bergmann @ 2016-06-21 15:00 UTC (permalink / raw)
To: y2038-cunTk1MwBs8s++Sfvej+rw
Cc: Linus Torvalds, Deepa Dinamani, Dave Kleikamp,
jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Trond Myklebust,
Adrian Hunter, Chris Mason,
adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org,
buchino-FYB4Gu1CFyUAvxtiuMwx3w, Thomas Gleixner, Yan, Zheng,
jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Paul Moore,
Linux SCSI List, Ilya Dryomov,
linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Changman Lee,
Mark Fasheh, sramars-FYB4Gu1CFyUAvxtiuMwx3w, John Stultz, Al Viro,
David Sterba, Jaegeuk Kim, ceph-devel
In-Reply-To: <CA+55aFzK2K-7UeQRsWR7ooNHMMbtMFRAF60byR1Gvmbf9XWhbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Monday, June 20, 2016 11:03:01 AM CEST you wrote:
> On Sun, Jun 19, 2016 at 5:26 PM, Deepa Dinamani <deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC macros.
> Gcc handles 8-byte structure returns (on most architectures) by
> returning them as two 32-bit registers (%edx:%eax on x86). But once it
> is timespec64, that will no longer be the case, and the calling
> convention will end up using a pointer to the local stack instead.
I guess we already have that today, as the implementation of
current_fs_time() is
static inline struct timespec64 tk_xtime(struct timekeeper *tk)
{
struct timespec64 ts;
ts.tv_sec = tk->xtime_sec;
ts.tv_nsec = (long)(tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift);
return ts;
}
extern struct timespec64 current_kernel_time64(void);
struct timespec64 current_kernel_time64(void)
{
struct timekeeper *tk = &tk_core.timekeeper;
struct timespec64 now;
unsigned long seq;
do {
seq = read_seqcount_begin(&tk_core.seq);
now = tk_xtime(tk);
} while (read_seqcount_retry(&tk_core.seq, seq));
return now;
}
static inline struct timespec current_kernel_time(void)
{
struct timespec64 now = current_kernel_time64();
return timespec64_to_timespec(now);
}
extern struct timespec current_fs_time(struct super_block *sb);
struct timespec current_fs_time(struct super_block *sb)
{
struct timespec now = current_kernel_time();
return timespec_trunc(now, sb->s_time_gran);
}
We can surely do a little better than this, independent of the
conversion in Deepa's patch set.
> So for 32-bit code generation, we *may* want to introduce a new model of doing
>
> set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);
>
> which basically just does
>
> inode->i_atime = inode->i_mtime = current_time(inode);
>
> but with a much easier calling convention on 32-bit architectures.
>
> But that is entirely orthogonal to this patch-set, and should be seen
> as a separate issue.
I've played around with that, but found it hard to avoid going
through memory other than going all the way to the tk_xtime()
access to copy both tk->xtime_sec and the nanoseconds into
the inode fields.
Without that, the set_inode_time() implementation ends up
being more expensive than
inode->i_atime = inode->i_ctime = inode->i_mtime = current_time(inode);
because we still copy through the stack but also have
a couple of conditional branches that we don't have at the
moment.
At the moment, the triple assignment becomes (here on ARM)
c: 4668 mov r0, sp
12: f7ff fffe bl 0 <current_kernel_time64>
3e: f107 0520 add.w r5, r7, #32
12: R_ARM_THM_CALL current_kernel_time64
16: f106 0410 add.w r4, r6, #16
1a: e89d 000f ldmia.w sp, {r0, r1, r2, r3} # load from stack
1e: e885 000f stmia.w r5, {r0, r1, r2, r3} # store into i_atime
22: e884 000f stmia.w r4, {r0, r1, r2, r3} # i_ctime
26: e886 000f stmia.w r6, {r0, r1, r2, r3} # i_mtime
and a slightly more verbose version of the same thing on x86
(storing only 12 bytes instead of 16 is cheaper there, while
ARM does a store-multiple to copy the entire structure).
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c
From: Ben Hutchings @ 2016-06-21 9:51 UTC (permalink / raw)
To: Pengfei Wang, Oleg Nesterov
Cc: security, Richard Guy Briggs, Krinke, Jens, linux-audit
In-Reply-To: <480FAE99-E4E1-42D0-ABD5-8DC24A7EC9BB@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1962 bytes --]
On Tue, 2016-06-21 at 10:37 +0100, Pengfei Wang wrote:
> >
> > 在 2016年6月20日，下午8:18，Oleg Nesterov <oleg@redhat.com> 写道：
> >
> > Not that I understand this report, but
> >
> > On 06/20, Richard Guy Briggs wrote:
> > >
> > > This function is only ever called by __audit_free(), which is only ever
> > > called on failure of task creation or on exit of the task, so in neither
> > > case can anything else change it.
> >
> > How so?
> >
> > Another thread or CLONE_VM task or /proc/pid/mem can change the user-space
> > memory in parallel.
> >
> > Oleg.
>
>
> Exactly, by saying “change the data”, I mean the modification from
> malicious users with crafted operations on the user space memory
> directly, rather than the normal operations within the audit
> subsystem in Linux. Moreover, since the copy operations from the user
> space are not protected by any locks or synchronization primitives,
> changing the data under race condition is feasible I think. Besides,
> there isn’t any visible checking step in the code to guarantee the
> consistency between the two copy operations.
>
> Here I would like to figure out what the consequences really are once
> the data is changed between the two copy operations, such as changing
> a none-control string to a control string but process it as a none-
> control string that has no control chars. I think problems will
> happen.
So far as userland can see, kernel log lines are separated by newlines.
If we fail to escape a newline, that makes it possible to inject
arbitrary log lines into the kernel log, which may be misleading to the
administrator or to software parsing the log.
Ben.
--
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had
thought.
... I realized that a large part of my life from then on was going to
be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c
From: Pengfei Wang @ 2016-06-21 9:37 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: security, Richard Guy Briggs, Krinke, Jens, linux-audit
In-Reply-To: <20160620191814.GA2942@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 1301 bytes --]
> 在 2016年6月20日，下午8:18，Oleg Nesterov <oleg@redhat.com> 写道：
>
> Not that I understand this report, but
>
> On 06/20, Richard Guy Briggs wrote:
>>
>> This function is only ever called by __audit_free(), which is only ever
>> called on failure of task creation or on exit of the task, so in neither
>> case can anything else change it.
>
> How so?
>
> Another thread or CLONE_VM task or /proc/pid/mem can change the user-space
> memory in parallel.
>
> Oleg.
Exactly, by saying “change the data”, I mean the modification from malicious users with crafted operations on the user space memory directly, rather than the normal operations within the audit subsystem in Linux. Moreover, since the copy operations from the user space are not protected by any locks or synchronization primitives, changing the data under race condition is feasible I think. Besides, there isn’t any visible checking step in the code to guarantee the consistency between the two copy operations.
Here I would like to figure out what the consequences really are once the data is changed between the two copy operations, such as changing a none-control string to a control string but process it as a none-control string that has no control chars. I think problems will happen.
Pengfei
[-- Attachment #1.2: Type: text/html, Size: 2701 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c
From: Oleg Nesterov @ 2016-06-20 19:18 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: security, Pengfei Wang, Krinke, Jens, linux-audit
In-Reply-To: <20160620182215.GC25615@madcap2.tricolour.ca>
Not that I understand this report, but
On 06/20, Richard Guy Briggs wrote:
>
> This function is only ever called by __audit_free(), which is only ever
> called on failure of task creation or on exit of the task, so in neither
> case can anything else change it.
How so?
Another thread or CLONE_VM task or /proc/pid/mem can change the user-space
memory in parallel.
Oleg.
^ permalink raw reply
* Re: [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
From: Deepa Dinamani @ 2016-06-20 18:58 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dave Kleikamp, jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
Trond Myklebust, Adrian Hunter, Chris Mason,
adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org,
Brian Uchino, Thomas Gleixner, Yan, Zheng, James E.J. Bottomley,
Paul Moore, Linux SCSI List, y2038 Mailman List, Ilya Dryomov,
linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Changman Lee,
Arnd Bergmann, Mark Fasheh, Suma Ramars, John Stultz, Al Viro,
David Sterba, Jaegeuk Kim <jaegeuk@
In-Reply-To: <CA+55aFzK2K-7UeQRsWR7ooNHMMbtMFRAF60byR1Gvmbf9XWhbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> This version now looks ok to me.
>
> I do have a comment (or maybe just a RFD) for future work.
>
> It does strike me that once we actually change over the inode times to
> use timespec64, the calling conventions are going to be fairly
> horrendous on most 32-bit architectures.
>
> Gcc handles 8-byte structure returns (on most architectures) by
> returning them as two 32-bit registers (%edx:%eax on x86). But once it
> is timespec64, that will no longer be the case, and the calling
> convention will end up using a pointer to the local stack instead.
>
> So for 32-bit code generation, we *may* want to introduce a new model of doing
>
> set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);
>
> which basically just does
>
> inode->i_atime = inode->i_mtime = current_time(inode);
>
> but with a much easier calling convention on 32-bit architectures.
Arnd and I had discussed something like this before.
But, for entirely different reasons:
Having the set_inode_time() like you suggest will also help switching
of vfs inode times to timespec64.
We were suggesting all the accesses to inode time be abstracted
through something like inode_set_time().
Arnd also had suggested a split representation of fields in the struct
inode as well which led to space savings
as well. And, having the split representation also meant no more
direct assignments:
https://lkml.org/lkml/2016/1/7/20
This in general will be similar to setattr_copy(), but only sets times
rather than other attributes as well.
If this is what is preferred, then the patches to change vfs to use
timespec64 could make use of this and will
need to be refactored. So maybe it would be good to discuss before I
post those patches.
-Deepa
^ permalink raw reply
* Re: Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c
From: Richard Guy Briggs @ 2016-06-20 18:22 UTC (permalink / raw)
To: Pengfei Wang; +Cc: security, linux-audit, Krinke, Jens
In-Reply-To: <CACxtibRq2ddMeLh5V5tV5A2zKDHfZX1EcNaKPaKZZvxYP1ntGA@mail.gmail.com>
On 2016-06-20 14:50, Pengfei Wang wrote:
> Hello,
>
> I found this Double-Fetch issue in Linux-4.6.1/kernel/auditsc.c when I
> was examining the source code, which I think is a bug.
>
> In function audit_log_single_execve_arg(), the whole argument is
> fetched from user space twice via copy_from_user(). In the first loop,
> it is firstly fetched (line 1038) to verify, aka looking for non-ascii
> chars. While in the second loop, the whole argument is fetched again
> (line 1105) from user space and used at line 1121 and line 1123
> respectively depends on the previous verification.
>
> However, a double fetch problem happens when the user space fetched
> data is changed by a concurrently running user thread under race
> condition during the verification and the usage, and the data
> inconsistency will cause serious problems. In this case, the verified
> non-ascii argument from the first loop is likely to be changed to an
> ascii one (i.e. containing ‘ “ ’) which will be used in the second
> loop. Then the argument is passed to audit_log_string() as none-ascii,
> then move forward in audit_log_n_string() of file audit.c, the string
> is enclosed with quote marks as well. Since the string contains
> another quote mark in the middle, problems will happen when processing
> the string based on quote mark, e.g. the string will be recognized as
> a shorter one based on the middle quote mark. I believe other
> consequences are also likely to be caused once the none control string
> is treated as a control string, or vice versa, which is very likely to
> happen under double fetch situations.
This function is only ever called by __audit_free(), which is only ever
called on failure of task creation or on exit of the task, so in neither
case can anything else change it.
I don't think what you describe will ever happen.
> I am looking forward to a reply to confirm this, thank you!
>
> Kind regards
>
> Pengfei
- 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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Re: [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
From: Linus Torvalds @ 2016-06-20 18:03 UTC (permalink / raw)
To: Deepa Dinamani
Cc: Dave Kleikamp, jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
Trond Myklebust, Adrian Hunter, Chris Mason,
adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org,
buchino-FYB4Gu1CFyUAvxtiuMwx3w, Thomas Gleixner, Yan, Zheng,
jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Paul Moore,
Linux SCSI List, y2038-cunTk1MwBs8s++Sfvej+rw, Ilya Dryomov,
linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Changman Lee,
Arnd Bergmann, Mark Fasheh, sramars-FYB4Gu1CFyUAvxtiuMwx3w,
John Stultz, Al Viro, David Sterba, Jaegeuk Kim, ceph-devel
In-Reply-To: <1466382443-11063-1-git-send-email-deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Sun, Jun 19, 2016 at 5:26 PM, Deepa Dinamani <deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC macros.
This version now looks ok to me.
I do have a comment (or maybe just a RFD) for future work.
It does strike me that once we actually change over the inode times to
use timespec64, the calling conventions are going to be fairly
horrendous on most 32-bit architectures.
Gcc handles 8-byte structure returns (on most architectures) by
returning them as two 32-bit registers (%edx:%eax on x86). But once it
is timespec64, that will no longer be the case, and the calling
convention will end up using a pointer to the local stack instead.
So for 32-bit code generation, we *may* want to introduce a new model of doing
set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);
which basically just does
inode->i_atime = inode->i_mtime = current_time(inode);
but with a much easier calling convention on 32-bit architectures.
But that is entirely orthogonal to this patch-set, and should be seen
as a separate issue.
And maybe it doesn't end up helping anyway, but I do think those
"simple" direct assignments will really generate pretty disgusting
code on 32-bit architectures.
That whole
inode->i_atime = inode->i_mtime = CURRENT_TIME;
model really made a lot more sense back in the ancient days when inode
times were just simply 32-bit seconds (not even timeval structures).
Linus
^ permalink raw reply
* Re: Logging from where user connected?
From: Steve Grubb @ 2016-06-20 15:32 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <01baeee4-2b49-2dbe-0c6d-895783271173@everyware.ch>
On Monday, June 20, 2016 03:54:02 PM Skwar Alexander wrote:
> On certain servers (Ubuntu 14.04 and Ubuntu 16.04, with auditd 2.3.2
> and v2.4.5), we'd like to log all the commands that root has run, or
> that were run as root.
>
> For that, I added the following rules:
>
> # Log all commands run as (or by) root
> -a exit,always -F arch=b64 -F euid=0 -S execve -k exec_root
> -a exit,always -F arch=b32 -F euid=0 -S execve -k exec_root
That will also get daemon child processes. Normally you would want to separate
routine system activity from user initiated activity.
> When I now do an "ausearch -k exec_root -i", I get:
>
> …
<snip>
> Now I'd like to know, from where that user connected. That user is
> on tty=pts1, so do I have to use last?
Nope. This was thought about long ago.
> local@app01-test ~ % last pts/1
> local pts/1 10.8.0.1 Mon Jun 20 13:26 still logged in
> …
>
>
>
> That's fine, as long as /var/log/wtmp* exists. But is there maybe a
> way to get that information right away, without having to consult a
> different logfile (eg. /var/log/wtmp)?
This has been long considered a user space post processing issue. When someone
logs in, a series of events occur. You can find the description here:
https://github.com/linux-audit/audit-documentation/wiki/SPEC-User-Login-Lifecycle-Events
Near the beginning you get USER_AUTH which is recorded by pam and it has the
IP address or terminal if it were a console.
There is a program, aulast, which tracks the sessions. It does show the origin
of the user session. Also, if you give it the --proof commandline option, it
will give you the ausearch command to examine the whole session.
> Additionally, if I'd like auditd to do remote logging (ie. send
> logs off of the system), I'd have to use audispd, wouldn't I?
Yes.
> How would I then get hold of the right wtmp file?
You don't need it.
-Steve
> I've got the feeling, that this might become quite complicated, if numerous
> servers would do remote logging to one central system...
>
> Would be quite thankful, if somebody could help :)
>
> Thanks a lot,
> Alexander
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Re: [PATCH v2 17/24] audit: Use timespec64 to represent audit timestamps
From: Richard Guy Briggs @ 2016-06-20 15:16 UTC (permalink / raw)
To: Deepa Dinamani
Cc: tytso, arnd, y2038, linux-kernel, linux-audit, viro,
linux-fsdevel, tglx, torvalds
In-Reply-To: <1466382443-11063-18-git-send-email-deepa.kernel@gmail.com>
On 2016-06-19 17:27, 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.
>
> 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
> Acked-by: Paul Moore <paul@paul-moore.com>
Acked-by: Richard Guy Briggs <rgb@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 fb1a3df..591c726 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1527,7 +1527,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;
> @@ -1936,13 +1936,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
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
- 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
^ permalink raw reply
* Logging from where user connected?
From: Skwar Alexander @ 2016-06-20 13:54 UTC (permalink / raw)
To: Linux Auditd Mailing Liste
Hello
On certain servers (Ubuntu 14.04 and Ubuntu 16.04, with auditd 2.3.2
and v2.4.5), we'd like to log all the commands that root has run, or
that were run as root.
For that, I added the following rules:
# Log all commands run as (or by) root
-a exit,always -F arch=b64 -F euid=0 -S execve -k exec_root
-a exit,always -F arch=b32 -F euid=0 -S execve -k exec_root
When I now do an "ausearch -k exec_root -i", I get:
…
----
type=PATH msg=audit(20.06.2016 15:28:06.976:65023) : item=1
name=/lib64/ld-linux-x86-64.so.2 inode=2952 dev=fc:01 mode=file,755
ouid=root ogid=root rdev=00:00 nametype=NORMAL
type=PATH msg=audit(20.06.2016 15:28:06.976:65023) : item=0
name=/usr/bin/sudo inode=396945 dev=fc:01 mode=file,suid,755 ouid=root
ogid=root rdev=00:00 nametype=NORMAL
type=CWD msg=audit(20.06.2016 15:28:06.976:65023) : cwd=/home/local
type=EXECVE msg=audit(20.06.2016 15:28:06.976:65023) : argc=5 a0=sudo
a1=ausearch a2=-k a3=exec_root a4=-i
type=BPRM_FCAPS msg=audit(20.06.2016 15:28:06.976:65023) : fver=0
fp=none fi=none fe=none old_pp=none old_pi=none old_pe=none
new_pp=chown,dac_override,dac_read_search,fowner,fsetid,kill,setgid,setuid,setpcap,linux_immutable,net_bind_service,net_broadcast,net_admin,net_raw,ipc_lock,ipc_owner,sys_module,sys_rawio,sys_chroot,sys_ptrace,sys_pacct,sys_admin,sys_boot,sys_nice,sys_resource,sys_time,sys_tty_config,mknod,lease,audit_write,audit_control,setfcap,mac_override,mac_admin,syslog,wake_alarm,block_suspend
new_pi=none
new_pe=chown,dac_override,dac_read_search,fowner,fsetid,kill,setgid,setuid,setpcap,linux_immutable,net_bind_service,net_broadcast,net_admin,net_raw,ipc_lock,ipc_owner,sys_module,sys_rawio,sys_chroot,sys_ptrace,sys_pacct,sys_admin,sys_boot,sys_nice,sys_resource,sys_time,sys_tty_config,mknod,lease,audit_write,audit_control,setfcap,mac_override,mac_admin,syslog,wake_alarm,block_suspend
type=SYSCALL msg=audit(20.06.2016 15:28:06.976:65023) : arch=x86_64
syscall=execve success=yes exit=0 a0=0x7fff4981a280 a1=0x7f7482187bd8
a2=0x1bfcf40 a3=0x7fff49819e80 items=2 ppid=11261 pid=14093 auid=local
uid=local gid=local euid=root suid=root fsuid=root egid=local sgid=local
fsgid=local tty=pts1 ses=15 comm=sudo exe=/usr/bin/sudo key=exec_root
----
type=PATH msg=audit(20.06.2016 15:28:06.980:65025) : item=1
name=/lib64/ld-linux-x86-64.so.2 inode=2952 dev=fc:01 mode=file,755
ouid=root ogid=root rdev=00:00 nametype=NORMAL
type=PATH msg=audit(20.06.2016 15:28:06.980:65025) : item=0
name=/sbin/ausearch inode=618 dev=fc:01 mode=file,755 ouid=root
ogid=root rdev=00:00 nametype=NORMAL
type=CWD msg=audit(20.06.2016 15:28:06.980:65025) : cwd=/home/local
type=EXECVE msg=audit(20.06.2016 15:28:06.980:65025) : argc=4
a0=ausearch a1=-k a2=exec_root a3=-i
type=SYSCALL msg=audit(20.06.2016 15:28:06.980:65025) : arch=x86_64
syscall=execve success=yes exit=0 a0=0x7fc01c0e0618 a1=0x7fc01c0e0638
a2=0x7fc01c0e5cd0 a3=0x7fff84d454c0 items=2 ppid=14093 pid=14094
auid=local uid=root gid=root euid=root suid=root fsuid=root egid=root
sgid=root fsgid=root tty=pts1 ses=15 comm=ausearch exe=/sbin/ausearch
key=exec_root
Now I'd like to know, from where that user connected. That user is
on tty=pts1, so do I have to use last?
local@app01-test ~ % last pts/1
local pts/1 10.8.0.1 Mon Jun 20 13:26 still logged in
…
That's fine, as long as /var/log/wtmp* exists. But is there maybe a
way to get that information right away, without having to consult a
different logfile (eg. /var/log/wtmp)?
Additionally, if I'd like auditd to do remote logging (ie. send
logs off of the system), I'd have to use audispd, wouldn't I? How
would I then get hold of the right wtmp file? I've got the feeling,
that this might become quite complicated, if numerous servers would
do remote logging to one central system...
Would be quite thankful, if somebody could help :)
Thanks a lot,
Alexander
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c
From: Pengfei Wang @ 2016-06-20 13:50 UTC (permalink / raw)
To: paul, eparis; +Cc: security, linux-audit, Krinke, Jens
[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]
Hello,
I found this Double-Fetch issue in Linux-4.6.1/kernel/auditsc.c when I
was examining the source code, which I think is a bug.
In function audit_log_single_execve_arg(), the whole argument is
fetched from user space twice via copy_from_user(). In the first loop,
it is firstly fetched (line 1038) to verify, aka looking for non-ascii
chars. While in the second loop, the whole argument is fetched again
(line 1105) from user space and used at line 1121 and line 1123
respectively depends on the previous verification.
However, a double fetch problem happens when the user space fetched
data is changed by a concurrently running user thread under race
condition during the verification and the usage, and the data
inconsistency will cause serious problems. In this case, the verified
non-ascii argument from the first loop is likely to be changed to an
ascii one (i.e. containing ‘ “ ’) which will be used in the second
loop. Then the argument is passed to audit_log_string() as none-ascii,
then move forward in audit_log_n_string() of file audit.c, the string
is enclosed with quote marks as well. Since the string contains
another quote mark in the middle, problems will happen when processing
the string based on quote mark, e.g. the string will be recognized as
a shorter one based on the middle quote mark. I believe other
consequences are also likely to be caused once the none control string
is treated as a control string, or vice versa, which is very likely to
happen under double fetch situations.
I am looking forward to a reply to confirm this, thank you!
Kind regards
Pengfei
[-- Attachment #2: audit.zip --]
[-- Type: application/zip, Size: 34545 bytes --]
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* [PATCH v2 17/24] audit: Use timespec64 to represent audit timestamps
From: Deepa Dinamani @ 2016-06-20 0:27 UTC (permalink / raw)
To: linux-fsdevel, linux-kernel
Cc: tytso, Paul Moore, arnd, y2038, Eric Paris, linux-audit, viro,
tglx, torvalds
In-Reply-To: <1466382443-11063-1-git-send-email-deepa.kernel@gmail.com>
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
Acked-by: Paul Moore <paul@paul-moore.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 fb1a3df..591c726 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1527,7 +1527,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;
@@ -1936,13 +1936,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
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply related
* [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
From: Deepa Dinamani @ 2016-06-20 0:26 UTC (permalink / raw)
To: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: shaggy-DgEjT+Ai2ygdnm+yROfE0A,
jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
trond.myklebust-7I+n7zu2hftEKMMhf/gKZA, clm-b10kYP2dOMg,
adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q,
buchino-FYB4Gu1CFyUAvxtiuMwx3w, tglx-hfZtesqFncYOwBW4kG4KsQ,
zyan-H+wXaHxf7aLQT0dZR+AlfA,
jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
paul-r2n+y4ga6xFZroRs9YW3xA, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
y2038-cunTk1MwBs8s++Sfvej+rw, idryomov-Re5JQEeQqe8AvxtiuMwx3w,
linux-ext4-u79uwXL29TY76Z2rM5mHXA,
cm224.lee-Sze3O3UU22JBDgjK7y7TUQ, arnd-r2nGTMty4D4,
mfasheh-IBi9RG/b67k, sramars-FYB4Gu1CFyUAvxtiuMwx3w,
john.stultz-QSEj5FYQhm4dnm+yROfE0A,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, dsterba-IBi9RG/b67k,
jaegeuk-DgEjT+Ai2ygdnm+yROfE0A, ceph-devel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, elder-DgEjT+Ai2ygdnm+yROfE0A,
tytso-3s7WtUTddSA, sage-H+wXaHxf7aLQT0dZR+AlfA,
martin.petersen-QHcLZuEGTsvQT0dZR+AlfA,
dedekind1-Re5JQEeQqe8AvxtiuMwx3w, jbacik-b10kYP2dOMg,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
hiralpat-FYB4Gu1CFyUAvxtiuMwx3w,
adrian.hunter-ral2JQCrhuEAvxtiuMwx3w,
eparis-H+wXaHxf7aLQT0dZR+AlfA,
linux-f2fs-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
sfrench-eUNUBHrolfbYtjvyW6yDsg,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA,
ocfs2-devel-N0ozoZBvEnrZJqsBc5GL+g, jack-IBi9RG/b67k
The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC macros.
The macros are not y2038 safe. There is no plan to transition them into being
y2038 safe.
ktime_get_* api's can be used in their place. And, these are y2038 safe.
Thanks to Arnd Bergmann for all the guidance and discussions.
Patches 2-4 were mostly generated using coccinelle scripts.
All filesystem timestamps use current_fs_time() for right granularity as
mentioned in the respective commit texts of patches. This has a changed
signature, renamed to current_time() and moved to the fs/inode.c.
This series also serves as a preparatory series to transition vfs to 64 bit
timestamps as outlined here: https://lkml.org/lkml/2016/2/12/104 .
As per Linus's suggestion in https://lkml.org/lkml/2016/5/24/663 , all the
inode timestamp changes have been squashed into a single patch. Also,
current_time() now is used as a single generic vfs filesystem timestamp api.
It also takes struct inode* as argument instead of struct super_block*.
Posting all patches together in a bigger series so that the big picture is
clear.
As per the suggestion in https://lwn.net/Articles/672598/, CURRENT_TIME macro
bug fixes are being handled in a series separate from transitioning vfs to use.
Changes from v1:
* Change current_fs_time(struct super_block *) to current_time(struct inode *)
* Note that change to add time64_to_tm() is already part of John's
kernel tree: https://lkml.org/lkml/2016/6/17/875 .
Deepa Dinamani (24):
vfs: Add current_time() api
fs: Replace CURRENT_TIME with current_time() for inode timestamps
fs: Replace CURRENT_TIME_SEC with current_time() for inode timestamps
fs: Replace current_fs_time() with current_time()
fs: jfs: Replace CURRENT_TIME_SEC by current_time()
fs: ext4: Use current_time() for inode timestamps
fs: ubifs: Replace CURRENT_TIME_SEC with current_time
fs: btrfs: Use ktime_get_real_ts for root ctime
fs: udf: Replace CURRENT_TIME with current_time()
fs: cifs: Replace CURRENT_TIME by current_time()
fs: cifs: Replace CURRENT_TIME with ktime_get_real_ts()
fs: cifs: Replace CURRENT_TIME by get_seconds
fs: f2fs: Use ktime_get_real_seconds for sit_info times
drivers: staging: lustre: Replace CURRENT_TIME with current_time()
fs: ocfs2: Use time64_t to represent orphan scan times
fs: ocfs2: Replace CURRENT_TIME with ktime_get_real_seconds()
audit: Use timespec64 to represent audit timestamps
fs: nfs: Make nfs boot time y2038 safe
fnic: Use time64_t to represent trace timestamps
block: Replace CURRENT_TIME with ktime_get_real_ts
libceph: Replace CURRENT_TIME with ktime_get_real_ts
fs: ceph: Replace current_fs_time for request stamp
time: Delete CURRENT_TIME_SEC and CURRENT_TIME macro
time: Delete current_fs_time() function
arch/powerpc/platforms/cell/spufs/inode.c | 2 +-
arch/s390/hypfs/inode.c | 4 ++--
drivers/block/rbd.c | 2 +-
drivers/char/sonypi.c | 2 +-
drivers/infiniband/hw/qib/qib_fs.c | 2 +-
drivers/misc/ibmasm/ibmasmfs.c | 2 +-
drivers/oprofile/oprofilefs.c | 2 +-
drivers/platform/x86/sony-laptop.c | 2 +-
drivers/scsi/fnic/fnic_trace.c | 4 ++--
drivers/scsi/fnic/fnic_trace.h | 2 +-
drivers/staging/lustre/lustre/llite/llite_lib.c | 16 ++++++-------
drivers/staging/lustre/lustre/llite/namei.c | 4 ++--
drivers/staging/lustre/lustre/mdc/mdc_reint.c | 6 ++---
.../lustre/lustre/obdclass/linux/linux-obdo.c | 6 ++---
drivers/staging/lustre/lustre/obdclass/obdo.c | 6 ++---
drivers/staging/lustre/lustre/osc/osc_io.c | 2 +-
drivers/usb/core/devio.c | 18 +++++++-------
drivers/usb/gadget/function/f_fs.c | 8 +++----
drivers/usb/gadget/legacy/inode.c | 2 +-
fs/9p/vfs_inode.c | 2 +-
fs/adfs/inode.c | 2 +-
fs/affs/amigaffs.c | 6 ++---
fs/affs/inode.c | 2 +-
fs/attr.c | 2 +-
fs/autofs4/inode.c | 2 +-
fs/autofs4/root.c | 6 ++---
fs/bad_inode.c | 2 +-
fs/bfs/dir.c | 14 +++++------
fs/binfmt_misc.c | 2 +-
fs/btrfs/file.c | 6 ++---
fs/btrfs/inode.c | 22 ++++++++---------
fs/btrfs/ioctl.c | 8 +++----
fs/btrfs/root-tree.c | 3 ++-
fs/btrfs/transaction.c | 4 ++--
fs/btrfs/xattr.c | 2 +-
fs/ceph/file.c | 4 ++--
fs/ceph/inode.c | 2 +-
fs/ceph/mds_client.c | 4 +++-
fs/ceph/xattr.c | 2 +-
fs/cifs/cifsencrypt.c | 4 +++-
fs/cifs/cifssmb.c | 10 ++++----
fs/cifs/file.c | 4 ++--
fs/cifs/inode.c | 28 ++++++++++++----------
fs/coda/dir.c | 2 +-
fs/coda/file.c | 2 +-
fs/coda/inode.c | 2 +-
fs/configfs/inode.c | 6 ++---
fs/debugfs/inode.c | 2 +-
fs/devpts/inode.c | 6 ++---
fs/efivarfs/inode.c | 2 +-
fs/exofs/dir.c | 6 ++---
fs/exofs/inode.c | 4 ++--
fs/exofs/namei.c | 6 ++---
fs/ext2/acl.c | 2 +-
fs/ext2/dir.c | 6 ++---
fs/ext2/ialloc.c | 2 +-
fs/ext2/inode.c | 4 ++--
fs/ext2/ioctl.c | 4 ++--
fs/ext2/namei.c | 6 ++---
fs/ext2/super.c | 2 +-
fs/ext2/xattr.c | 2 +-
fs/ext4/acl.c | 2 +-
fs/ext4/ext4.h | 6 -----
fs/ext4/extents.c | 10 ++++----
fs/ext4/ialloc.c | 2 +-
fs/ext4/inline.c | 4 ++--
fs/ext4/inode.c | 6 ++---
fs/ext4/ioctl.c | 8 +++----
fs/ext4/namei.c | 24 ++++++++++---------
fs/ext4/super.c | 2 +-
fs/ext4/xattr.c | 2 +-
fs/f2fs/dir.c | 8 +++----
fs/f2fs/file.c | 8 +++----
fs/f2fs/inline.c | 2 +-
fs/f2fs/namei.c | 12 +++++-----
fs/f2fs/segment.c | 2 +-
fs/f2fs/segment.h | 5 ++--
fs/f2fs/xattr.c | 2 +-
fs/fat/dir.c | 2 +-
fs/fat/file.c | 6 ++---
fs/fat/inode.c | 2 +-
fs/fat/namei_msdos.c | 12 +++++-----
fs/fat/namei_vfat.c | 10 ++++----
fs/fuse/control.c | 2 +-
fs/fuse/dir.c | 2 +-
fs/gfs2/bmap.c | 8 +++----
fs/gfs2/dir.c | 12 +++++-----
fs/gfs2/inode.c | 8 +++----
fs/gfs2/quota.c | 2 +-
fs/gfs2/xattr.c | 8 +++----
fs/hfs/catalog.c | 8 +++----
fs/hfs/dir.c | 2 +-
fs/hfs/inode.c | 2 +-
fs/hfsplus/catalog.c | 8 +++----
fs/hfsplus/dir.c | 6 ++---
fs/hfsplus/inode.c | 2 +-
fs/hfsplus/ioctl.c | 2 +-
fs/hugetlbfs/inode.c | 10 ++++----
fs/inode.c | 21 +++++++++++++---
fs/jffs2/acl.c | 2 +-
fs/jffs2/fs.c | 2 +-
fs/jfs/acl.c | 2 +-
fs/jfs/inode.c | 2 +-
fs/jfs/ioctl.c | 2 +-
fs/jfs/jfs_inode.c | 2 +-
fs/jfs/namei.c | 24 +++++++++----------
fs/jfs/super.c | 2 +-
fs/jfs/xattr.c | 2 +-
fs/kernfs/inode.c | 2 +-
fs/libfs.c | 14 +++++------
fs/locks.c | 2 +-
fs/logfs/dir.c | 6 ++---
fs/logfs/file.c | 2 +-
fs/logfs/inode.c | 4 ++--
fs/logfs/readwrite.c | 4 ++--
fs/minix/bitmap.c | 2 +-
fs/minix/dir.c | 6 ++---
fs/minix/itree_common.c | 4 ++--
fs/minix/namei.c | 4 ++--
fs/nfs/client.c | 2 +-
fs/nfs/netns.h | 2 +-
fs/nfs/nfs4proc.c | 10 ++++----
fs/nfs/nfs4xdr.c | 2 +-
fs/nfsd/blocklayout.c | 2 +-
fs/nilfs2/dir.c | 6 ++---
fs/nilfs2/inode.c | 4 ++--
fs/nilfs2/ioctl.c | 2 +-
fs/nilfs2/namei.c | 6 ++---
fs/nsfs.c | 2 +-
fs/ntfs/inode.c | 2 +-
fs/ntfs/mft.c | 2 +-
fs/ocfs2/acl.c | 2 +-
fs/ocfs2/alloc.c | 2 +-
fs/ocfs2/aops.c | 2 +-
fs/ocfs2/cluster/heartbeat.c | 2 +-
fs/ocfs2/dir.c | 4 ++--
fs/ocfs2/dlmfs/dlmfs.c | 4 ++--
fs/ocfs2/file.c | 12 +++++-----
fs/ocfs2/inode.c | 2 +-
fs/ocfs2/journal.c | 4 ++--
fs/ocfs2/move_extents.c | 2 +-
fs/ocfs2/namei.c | 16 +++++++------
fs/ocfs2/ocfs2.h | 2 +-
fs/ocfs2/refcounttree.c | 4 ++--
fs/ocfs2/super.c | 2 +-
fs/ocfs2/xattr.c | 2 +-
fs/omfs/dir.c | 4 ++--
fs/omfs/inode.c | 2 +-
fs/openpromfs/inode.c | 2 +-
fs/orangefs/file.c | 2 +-
fs/orangefs/inode.c | 2 +-
fs/orangefs/namei.c | 10 ++++----
fs/pipe.c | 2 +-
fs/posix_acl.c | 2 +-
fs/proc/base.c | 2 +-
fs/proc/inode.c | 4 ++--
fs/proc/proc_sysctl.c | 2 +-
fs/proc/self.c | 2 +-
fs/proc/thread_self.c | 2 +-
fs/pstore/inode.c | 2 +-
fs/ramfs/inode.c | 6 ++---
fs/reiserfs/inode.c | 2 +-
fs/reiserfs/ioctl.c | 4 ++--
fs/reiserfs/namei.c | 12 +++++-----
fs/reiserfs/stree.c | 8 +++----
fs/reiserfs/super.c | 2 +-
fs/reiserfs/xattr.c | 6 ++---
fs/reiserfs/xattr_acl.c | 2 +-
fs/sysv/dir.c | 6 ++---
fs/sysv/ialloc.c | 2 +-
fs/sysv/itree.c | 4 ++--
fs/sysv/namei.c | 4 ++--
fs/tracefs/inode.c | 2 +-
fs/ubifs/dir.c | 10 ++++----
fs/ubifs/file.c | 12 +++++-----
fs/ubifs/ioctl.c | 2 +-
fs/ubifs/misc.h | 10 --------
fs/ubifs/sb.c | 14 +++++++----
fs/ubifs/xattr.c | 6 ++---
fs/udf/ialloc.c | 2 +-
fs/udf/inode.c | 4 ++--
fs/udf/namei.c | 20 ++++++++--------
fs/udf/super.c | 9 +++++--
fs/ufs/dir.c | 6 ++---
fs/ufs/ialloc.c | 8 ++++---
fs/ufs/inode.c | 6 ++---
fs/ufs/namei.c | 6 ++---
fs/xfs/xfs_acl.c | 2 +-
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_iops.c | 2 +-
fs/xfs/xfs_trans_inode.c | 2 +-
include/linux/audit.h | 4 ++--
include/linux/fs.h | 2 +-
include/linux/time.h | 3 ---
ipc/mqueue.c | 18 +++++++-------
kernel/audit.c | 10 ++++----
kernel/audit.h | 2 +-
kernel/auditsc.c | 6 ++---
kernel/bpf/inode.c | 2 +-
kernel/time/time.c | 14 -----------
mm/shmem.c | 20 ++++++++--------
net/ceph/messenger.c | 6 +++--
net/ceph/osd_client.c | 4 ++--
net/sunrpc/rpc_pipe.c | 2 +-
security/inode.c | 2 +-
security/selinux/selinuxfs.c | 2 +-
206 files changed, 533 insertions(+), 522 deletions(-)
--
1.9.1
Cc: adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org
Cc: adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Cc: anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
Cc: buchino-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org
Cc: ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: clm-b10kYP2dOMg@public.gmane.org
Cc: cm224.lee-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org
Cc: dedekind1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: dsterba-IBi9RG/b67k@public.gmane.org
Cc: elder-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org
Cc: hiralpat-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org
Cc: idryomov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: jack-IBi9RG/b67k@public.gmane.org
Cc: jaegeuk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: jbacik-b10kYP2dOMg@public.gmane.org
Cc: jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Cc: jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc: jlbec-aKy9MeLSZ9dg9hUCZPvPmw@public.gmane.org
Cc: john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
Cc: linux-audit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: linux-btrfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-f2fs-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org
Cc: martin.petersen-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org
Cc: mfasheh-IBi9RG/b67k@public.gmane.org
Cc: ocfs2-devel-N0ozoZBvEnrZJqsBc5GL+g@public.gmane.org
Cc: paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org
Cc: sage-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org
Cc: shaggy-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: sramars-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org
Cc: trond.myklebust-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org
Cc: zyan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
^ permalink raw reply
* [PATCH] audit: fix some horrible switch statement style crimes
From: Paul Moore @ 2016-06-16 21:31 UTC (permalink / raw)
To: linux-audit
From: Paul Moore <paul@paul-moore.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
kernel/auditfilter.c | 8 ++++++--
kernel/auditsc.c | 8 ++++++--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 8a8aa3fb..ff59a5e 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1343,8 +1343,12 @@ static int audit_filter_user_rules(struct audit_krule *rule, int type,
return result;
}
switch (rule->action) {
- case AUDIT_NEVER: *state = AUDIT_DISABLED; break;
- case AUDIT_ALWAYS: *state = AUDIT_RECORD_CONTEXT; break;
+ case AUDIT_NEVER:
+ *state = AUDIT_DISABLED;
+ break;
+ case AUDIT_ALWAYS:
+ *state = AUDIT_RECORD_CONTEXT;
+ break;
}
return 1;
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 7d0e3cf..ec4c552 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -695,8 +695,12 @@ static int audit_filter_rules(struct task_struct *tsk,
ctx->prio = rule->prio;
}
switch (rule->action) {
- case AUDIT_NEVER: *state = AUDIT_DISABLED; break;
- case AUDIT_ALWAYS: *state = AUDIT_RECORD_CONTEXT; break;
+ case AUDIT_NEVER:
+ *state = AUDIT_DISABLED;
+ break;
+ case AUDIT_ALWAYS:
+ *state = AUDIT_RECORD_CONTEXT;
+ break;
}
return 1;
}
^ permalink raw reply related
* Re: [PATCH] audit: catch errors from audit_filter_rules field checks
From: Paul Moore @ 2016-06-16 21:07 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel
In-Reply-To: <6c97e8eec7aee6bade82dd0c7933908098846a90.1465934130.git.rgb@redhat.com>
On Tue, Jun 14, 2016 at 5:03 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> In the case of an error returned from a field check in an audit filter
> syscall rule, it is treated as a match and the rule action is honoured.
>
> This could cause a rule with a default of NEVER and an selinux field
> check error to avoid logging.
>
> Recommend matching with an action of ALWAYS to catch malicious abuse of
> this bug. The downside of this approach is it could DoS the audit
> subsystem.
I understand your concern about the DoS, but in reality it is no worse
than if no audit filter rules were configured, yes?
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> kernel/auditsc.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 71e14d8..6123672 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -683,6 +683,10 @@ static int audit_filter_rules(struct task_struct *tsk,
> }
> if (!result)
> return 0;
> + if (result < 0) {
> + *state = AUDIT_RECORD_CONTEXT;
> + return 1;
> + }
> }
>
> if (ctx) {
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH v2] audit: add fields to exclude filter by reusing user filter
From: Paul Moore @ 2016-06-16 20:54 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel
In-Reply-To: <d7c489a8f2a8ee344c697999cc4ee70236036b64.1465938080.git.rgb@redhat.com>
On Tue, Jun 14, 2016 at 5:04 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> RFE: add additional fields for use in audit filter exclude rules
> https://github.com/linux-audit/audit-kernel/issues/5
>
> Re-factor and combine audit_filter_type() with audit_filter_user() to
> use audit_filter_user_rules() to enable the exclude filter to
> additionally filter on PID, UID, GID, AUID, LOGINUID_SET, SUBJ_*.
>
> The process of combining the similar audit_filter_user() and
> audit_filter_type() functions, required inverting the meaning and
> including the ALWAYS action of the latter.
>
> Keep the check to quit early if the list is empty.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> v2: combine audit_filter_user() and audit_filter_type() into
> audit_filter().
> ---
>
> include/linux/audit.h | 2 --
> kernel/audit.c | 4 ++--
> kernel/audit.h | 2 ++
> kernel/auditfilter.c | 46 ++++++++++------------------------------------
> 4 files changed, 14 insertions(+), 40 deletions(-)
Patches that remove more code than the add always make me happy :)
Comment below ...
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 32cdafb..539c1d9 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -160,8 +160,6 @@ extern void audit_log_task_info(struct audit_buffer *ab,
> extern int audit_update_lsm_rules(void);
>
> /* Private API (for audit.c only) */
> -extern int audit_filter_user(int type);
> -extern int audit_filter_type(int type);
> extern int audit_rule_change(int type, __u32 portid, int seq,
> void *data, size_t datasz);
> extern int audit_list_rules_send(struct sk_buff *request_skb, int seq);
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 384374a..2dfaa19 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -914,7 +914,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> if (!audit_enabled && msg_type != AUDIT_USER_AVC)
> return 0;
>
> - err = audit_filter_user(msg_type);
> + err = audit_filter(msg_type, AUDIT_FILTER_USER);
> if (err == 1) { /* match or error */
> err = 0;
> if (msg_type == AUDIT_USER_TTY) {
> @@ -1362,7 +1362,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
> if (audit_initialized != AUDIT_INITIALIZED)
> return NULL;
>
> - if (unlikely(audit_filter_type(type)))
> + if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE)))
> return NULL;
>
> if (gfp_mask & __GFP_DIRECT_RECLAIM) {
> diff --git a/kernel/audit.h b/kernel/audit.h
> index cbbe6bb..1879f02 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -327,6 +327,8 @@ extern pid_t audit_sig_pid;
> extern kuid_t audit_sig_uid;
> extern u32 audit_sig_sid;
>
> +extern int audit_filter(int msgtype, unsigned int listtype);
> +
> #ifdef CONFIG_AUDITSYSCALL
> extern int __audit_signal_info(int sig, struct task_struct *t);
> static inline int audit_signal_info(int sig, struct task_struct *t)
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 96c9a1b..f90c042 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1349,50 +1349,24 @@ static int audit_filter_user_rules(struct audit_krule *rule, int type,
> return 1;
> }
>
> -int audit_filter_user(int type)
> +int audit_filter(int msgtype, unsigned int listtype)
> {
> enum audit_state state = AUDIT_DISABLED;
> struct audit_entry *e;
> - int rc, ret;
> -
> - ret = 1; /* Audit by default */
> + int rc, result = 1; /* Audit by default */
>
> rcu_read_lock();
> - list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
> - rc = audit_filter_user_rules(&e->rule, type, &state);
> - if (rc) {
> - if (rc > 0 && state == AUDIT_DISABLED)
> - ret = 0;
> - break;
> - }
> - }
> - rcu_read_unlock();
> -
> - return ret;
> -}
> -
> -int audit_filter_type(int type)
> -{
> - struct audit_entry *e;
> - int result = 0;
> -
> - rcu_read_lock();
> - if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE]))
> + if (list_empty(&audit_filter_list[listtype]))
> goto unlock_and_return;
>
> - list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE],
> - list) {
> - int i;
> - for (i = 0; i < e->rule.field_count; i++) {
> - struct audit_field *f = &e->rule.fields[i];
> - if (f->type == AUDIT_MSGTYPE) {
> - result = audit_comparator(type, f->op, f->val);
> - if (!result)
> - break;
> - }
> + list_for_each_entry_rcu(e, &audit_filter_list[listtype], list) {
> + rc = audit_filter_user_rules(&e->rule, msgtype, &state);
Any reason not to do away with audit_filter_user_rules() and just
insert the code here? As far as I can tell there are no other callers
...
> + if (rc) {
> + if (rc > 0 && ((state == AUDIT_DISABLED) ||
> + (listtype == AUDIT_FILTER_TYPE)))
> + result = 0;
> + break;
> }
> - if (result)
> - goto unlock_and_return;
> }
> unlock_and_return:
> rcu_read_unlock();
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 17/21] audit: Use timespec64 to represent audit timestamps
From: Paul Moore @ 2016-06-15 21:23 UTC (permalink / raw)
To: Deepa Dinamani, Richard Guy Briggs, Steve Grubb
Cc: Arnd Bergmann, y2038, linux-kernel, linux-audit, Al Viro,
linux-fsdevel, Thomas Gleixner, Linus Torvalds
In-Reply-To: <20160609235943.GL18488@madcap2.tricolour.ca>
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
^ permalink raw reply
* Re: [RFC PATCH 3/7] audit: allow systemd to use queue reserves
From: Richard Guy Briggs @ 2016-06-15 17:35 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-audit
In-Reply-To: <1945739.qBIBrYXbVh@sifl>
On 15/11/05, Paul Moore wrote:
> On Thursday, October 22, 2015 03:51:59 PM Richard Guy Briggs wrote:
> > On 15/10/22, Steve Grubb wrote:
> > > On Thursday, October 22, 2015 02:53:16 PM Richard Guy Briggs wrote:
> > > > Treat systemd the same way as auditd, allowing it to overrun the queue
> > > > to avoid blocking.
> > >
> > > Do you mind explaining this a little more? I'm having a hard time
> > > understanding how systemd is involved.
> >
> > systemd should only have CAP_AUDIT_READ for the multicast socket and
> > otherwise behaves as a user client, sending AUDIT_USER_* messages. It
> > starts and stops auditd and we don't want it blocking trying to allocate
> > a buffer on the standard queue in audit_log_start() while it is tasked
> > with telling auditd to start or stop.
>
> Is this something we are hearing reports about? Starting and stopping auditd
> should be rare in normal use, and by rare I mean start it at boot and don't
> touch it again ... although I suspect you might update/patch it at some point
> if your system is long running.
(Sorry, this message has been on my queue for a while...)
Well, it is touched again, when shutting down the machine, or upgrading
auditd. There also seem to be issues when the logs are rotated. This
first case is the one I've been working on that caused all this
examination and instrumentation. systemd was chucking in a couple dozen
messages onto the queue on startup
> If this is a common problem we can look at doing something like this, but if
> it isn't - and I don't think it is - I'd like to avoid special casing init
> (it's even more specialized since we are basically talking about just systemd,
> although others could have similar prblems).
>
> > > -Steve
> > >
> > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > ---
> > >
> > > > kernel/audit.c | 2 +-
> > > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > index 3917aad..384a1a1 100644
> > > > --- a/kernel/audit.c
> > > > +++ b/kernel/audit.c
> > > > @@ -1375,7 +1375,7 @@ struct audit_buffer *audit_log_start(struct
> > > > audit_context *ctx, gfp_t gfp_mask, return NULL;
> > > >
> > > > if (gfp_mask & __GFP_WAIT) {
> > > >
> > > > - if (audit_pid && audit_pid == current->tgid)
> > > > + if (current->tgid == 1 || (audit_pid && audit_pid == current-
> >tgid))
> > > >
> > > > gfp_mask &= ~__GFP_WAIT;
> > > >
> > > > else
> > > >
> > > > reserve = 0;
> >
> > - RGB
>
> paul moore
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* Re: [PATCH] audit: add fields to exclude filter by reusing user filter
From: Richard Guy Briggs @ 2016-06-15 2:09 UTC (permalink / raw)
To: Paul Moore; +Cc: Paul Moore, linux-audit, linux-kernel
In-Reply-To: <CAGH-KgtcN0U=ak1rAx-puRa9f7xwaFxwh9VJGE4fwa0RD0jKRQ@mail.gmail.com>
On 16/06/03, Paul Moore wrote:
> On Fri, Jun 3, 2016 at 4:24 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 16/06/03, Paul Moore wrote:
> >> On Wed, Jun 1, 2016 at 6:50 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >> > RFE: add additional fields for use in audit filter exclude rules
> >> > https://github.com/linux-audit/audit-kernel/issues/5
> >> >
> >> > Re-factor audit_filter_type() to use audit_filter_user_rules() to enable
> >> > exclude filter to additionally filter on PID, UID, GID, AUID,
> >> > LOGINUID_SET, SUBJ_*.
> >> >
> >> > Add check in audit_filter_user() to quit early if list is empty.
> >> >
> >> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> >> > ---
> >> > kernel/auditfilter.c | 22 +++++++++-------------
> >> > 1 files changed, 9 insertions(+), 13 deletions(-)
> >>
> >> I like the consolidation between audit_filter_type() and
> >> audit_filter_user(), I like it so much I think we should take it
> >> further. Let's consolidate both functions into a single function (say
> >> audit_filter()?) and update the callers to use the new function. This
> >> shouldn't be hard as the only callers are audit_receive_msg() and
> >> audit_log_start(); you'll need to be careful as the return values of
> >> the current functions are opposite of each other, but it should be
> >> easy enough to update one of the callers.
> >>
> >> Sound reasonable?
> >
> > Potentially... I was even eyeing kernel/auditsc.c::audit_filter_rules()
> > for re-factoring...
>
> It's possible that we may be able to do some work on eliminating
> duplication between the audit_filter_user()/audit_filter_type() and
> audit_filter_rules(), but we'll always need audit_filter_rules()
> simply because it can filter on more things, e.g. it has access to
> object information.
Yes, I was thinking that audit_filter_type/user() could be a sub-call of
audit_filter_rules(), but not sure the clearest way to implement that.
> I would suggest working on just audit_filter_user() and
> audit_filter_type() right now so we can get something ready for
> upstream before -rc5 or so, and then look into possible refactoring
> with audit_filter_rules().
I had a first crack at it last week and it wasn't just a simple
inversion, but had rule action and error differences too, so I had some
doubts about how worthwhile the refactor would be in terms of reducing
duplicaiton and making things clearer, but had another go today and am
satisfied with the resulting patch posted today.
> paul moore
- 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
^ permalink raw reply
* [PATCH v2] audit: add fields to exclude filter by reusing user filter
From: Richard Guy Briggs @ 2016-06-14 21:04 UTC (permalink / raw)
To: linux-audit, linux-kernel; +Cc: Richard Guy Briggs, sgrubb, pmoore, eparis
RFE: add additional fields for use in audit filter exclude rules
https://github.com/linux-audit/audit-kernel/issues/5
Re-factor and combine audit_filter_type() with audit_filter_user() to
use audit_filter_user_rules() to enable the exclude filter to
additionally filter on PID, UID, GID, AUID, LOGINUID_SET, SUBJ_*.
The process of combining the similar audit_filter_user() and
audit_filter_type() functions, required inverting the meaning and
including the ALWAYS action of the latter.
Keep the check to quit early if the list is empty.
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
v2: combine audit_filter_user() and audit_filter_type() into
audit_filter().
---
include/linux/audit.h | 2 --
kernel/audit.c | 4 ++--
kernel/audit.h | 2 ++
kernel/auditfilter.c | 46 ++++++++++------------------------------------
4 files changed, 14 insertions(+), 40 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 32cdafb..539c1d9 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -160,8 +160,6 @@ extern void audit_log_task_info(struct audit_buffer *ab,
extern int audit_update_lsm_rules(void);
/* Private API (for audit.c only) */
-extern int audit_filter_user(int type);
-extern int audit_filter_type(int type);
extern int audit_rule_change(int type, __u32 portid, int seq,
void *data, size_t datasz);
extern int audit_list_rules_send(struct sk_buff *request_skb, int seq);
diff --git a/kernel/audit.c b/kernel/audit.c
index 384374a..2dfaa19 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -914,7 +914,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
if (!audit_enabled && msg_type != AUDIT_USER_AVC)
return 0;
- err = audit_filter_user(msg_type);
+ err = audit_filter(msg_type, AUDIT_FILTER_USER);
if (err == 1) { /* match or error */
err = 0;
if (msg_type == AUDIT_USER_TTY) {
@@ -1362,7 +1362,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
if (audit_initialized != AUDIT_INITIALIZED)
return NULL;
- if (unlikely(audit_filter_type(type)))
+ if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE)))
return NULL;
if (gfp_mask & __GFP_DIRECT_RECLAIM) {
diff --git a/kernel/audit.h b/kernel/audit.h
index cbbe6bb..1879f02 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -327,6 +327,8 @@ extern pid_t audit_sig_pid;
extern kuid_t audit_sig_uid;
extern u32 audit_sig_sid;
+extern int audit_filter(int msgtype, unsigned int listtype);
+
#ifdef CONFIG_AUDITSYSCALL
extern int __audit_signal_info(int sig, struct task_struct *t);
static inline int audit_signal_info(int sig, struct task_struct *t)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 96c9a1b..f90c042 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1349,50 +1349,24 @@ static int audit_filter_user_rules(struct audit_krule *rule, int type,
return 1;
}
-int audit_filter_user(int type)
+int audit_filter(int msgtype, unsigned int listtype)
{
enum audit_state state = AUDIT_DISABLED;
struct audit_entry *e;
- int rc, ret;
-
- ret = 1; /* Audit by default */
+ int rc, result = 1; /* Audit by default */
rcu_read_lock();
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
- rc = audit_filter_user_rules(&e->rule, type, &state);
- if (rc) {
- if (rc > 0 && state == AUDIT_DISABLED)
- ret = 0;
- break;
- }
- }
- rcu_read_unlock();
-
- return ret;
-}
-
-int audit_filter_type(int type)
-{
- struct audit_entry *e;
- int result = 0;
-
- rcu_read_lock();
- if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE]))
+ if (list_empty(&audit_filter_list[listtype]))
goto unlock_and_return;
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE],
- list) {
- int i;
- for (i = 0; i < e->rule.field_count; i++) {
- struct audit_field *f = &e->rule.fields[i];
- if (f->type == AUDIT_MSGTYPE) {
- result = audit_comparator(type, f->op, f->val);
- if (!result)
- break;
- }
+ list_for_each_entry_rcu(e, &audit_filter_list[listtype], list) {
+ rc = audit_filter_user_rules(&e->rule, msgtype, &state);
+ if (rc) {
+ if (rc > 0 && ((state == AUDIT_DISABLED) ||
+ (listtype == AUDIT_FILTER_TYPE)))
+ result = 0;
+ break;
}
- if (result)
- goto unlock_and_return;
}
unlock_and_return:
rcu_read_unlock();
--
1.7.1
^ permalink raw reply related
* [PATCH] audit: catch errors from audit_filter_rules field checks
From: Richard Guy Briggs @ 2016-06-14 21:03 UTC (permalink / raw)
To: linux-audit, linux-kernel; +Cc: Richard Guy Briggs, sgrubb, pmoore, eparis
In the case of an error returned from a field check in an audit filter
syscall rule, it is treated as a match and the rule action is honoured.
This could cause a rule with a default of NEVER and an selinux field
check error to avoid logging.
Recommend matching with an action of ALWAYS to catch malicious abuse of
this bug. The downside of this approach is it could DoS the audit
subsystem.
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
kernel/auditsc.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 71e14d8..6123672 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -683,6 +683,10 @@ static int audit_filter_rules(struct task_struct *tsk,
}
if (!result)
return 0;
+ if (result < 0) {
+ *state = AUDIT_RECORD_CONTEXT;
+ return 1;
+ }
}
if (ctx) {
--
1.7.1
^ permalink raw reply related
* RE: Audit reporting Invalid argument
From: Bhagwat, Shriniketan Manjunath @ 2016-06-14 13:44 UTC (permalink / raw)
To: Steve Grubb; +Cc: linux-audit@redhat.com
In-Reply-To: <3850158.qFrFSIuG4S@x2>
Hi Steve,
>> The plugin will be restarted when the next event arrives to audispd.
I do not want my plug-in to be running unnecessarily all the time until the auditd is running.
I can accomplish my requirement by sending SIGHUP to audispd and changing the plug-in configuration's option active=yes/no.
Regards,
Ketan
-----Original Message-----
From: Steve Grubb [mailto:sgrubb@redhat.com]
Sent: Monday, June 13, 2016 8:31 PM
To: Bhagwat, Shriniketan Manjunath <shriniketan.bhagwat@hpe.com>
Cc: linux-audit@redhat.com
Subject: Re: Audit reporting Invalid argument
On Monday, June 13, 2016 08:15:36 AM Bhagwat, Shriniketan Manjunath wrote:
> Hi,
>
> Is it possible to start and stop the user written audit plug-in while
> auditd and audispd running? As I understand, audispd is started by
> auditd. Audispd starts the user plug-in program using their
> configuration files present in /etc/audisp/plugins.d directory. Auditd
> and user plug-in are started and stopped as part of auditd startup and
> stop. Is it possible to start the user plug-in after the auditd is
> started and stop the user plug-in before the auditd is stopped?
There is nothing that prevents you from sending a SIGTERM to the plugin if you are root. The plugin will be restarted when the next event arrives to audispd.
-Steve
> -----Original Message-----
> From: Steve Grubb [mailto:sgrubb@redhat.com]
> Sent: Monday, May 16, 2016 6:24 PM
> To: Bhagwat, Shriniketan Manjunath <shriniketan.bhagwat@hpe.com>
> Cc: linux-audit@redhat.com
> Subject: Re: Audit reporting Invalid argument
>
> On Saturday, May 14, 2016 09:40:05 AM Bhagwat, Shriniketan Manjunath wrote:
> > > Not today. The check for uid 0 is a poor man's check for
> > > CAP_AUDIT_CONTROL
> >
> > Are there any future plans to support enabling audit from non root
> > user using CAP_AUDIT_CONTROL?
>
> You are the only person who has asked for it. I suppose it can be done
> in a couple lines of code. But you still have the permissions of the
> directories that hold the rules to correct. Easy to fix, but I think
> you might be fighting the distribution's package manager which would
> set things back to root every update.
> > Regarding suppression of events, I will do some testing and let you
> > know later.
> >
> > Is there a way I can avoid default logging of the audit events to
> > /var/log/audit/audit.log?
>
> If you have an old copy old the audit system (2.5.1 or earlier) then
> use log_format = NOLOG. If you have a current copy, then use write_logs = no.
>
> -Steve
>
> > I do not want audit to log audit events to audit.log, however I will
> > capture them using my plug-in. Is there a way I can accomplish this?
> > I tried to commenting the log_file filed from auditd.conf, however
> > the events are still written to audit.log. I think below code from
> > auditd-config.c is causing audit to write to audit.log
> >
> > config->log_file = strdup("/var/log/audit/audit.log");
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox