[PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Eric Paris]

From: Andy Lutomirski <luto@amacapital.net>

Fixes an easy DoS and possible information disclosure.

This does nothing about the broken state of x32 auditing.

eparis: If the admin has enabled auditd and has specifically loaded audit
rules.  This bug has been around since before git.  Wow...

Cc: stable@vger.kernel.org
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
 kernel/auditsc.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 254ce20..842f58a 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -728,6 +728,22 @@ static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)
 	return AUDIT_BUILD_CONTEXT;
 }
 
+static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
+{
+	int word, bit;
+
+	if (val > 0xffffffff)
+		return false;
+
+	word = AUDIT_WORD(val);
+	if (word >= AUDIT_BITMASK_SIZE)
+		return false;
+
+	bit = AUDIT_BIT(val);
+
+	return rule->mask[word] & bit;
+}
+
 /* At syscall entry and exit time, this filter is called if the
  * audit_state is not low enough that auditing cannot take place, but is
  * also not high enough that we already know we have to write an audit
@@ -745,11 +761,8 @@ static enum audit_state audit_filter_syscall(struct task_struct *tsk,
 
 	rcu_read_lock();
 	if (!list_empty(list)) {
-		int word = AUDIT_WORD(ctx->major);
-		int bit  = AUDIT_BIT(ctx->major);
-
 		list_for_each_entry_rcu(e, list, list) {
-			if ((e->rule.mask[word] & bit) == bit &&
+			if (audit_in_mask(&e->rule, ctx->major) &&
 			    audit_filter_rules(tsk, &e->rule, ctx, NULL,
 					       &state, false)) {
 				rcu_read_unlock();
@@ -769,20 +782,16 @@ static enum audit_state audit_filter_syscall(struct task_struct *tsk,
 static int audit_filter_inode_name(struct task_struct *tsk,
 				   struct audit_names *n,
 				   struct audit_context *ctx) {
-	int word, bit;
 	int h = audit_hash_ino((u32)n->ino);
 	struct list_head *list = &audit_inode_hash[h];
 	struct audit_entry *e;
 	enum audit_state state;
 
-	word = AUDIT_WORD(ctx->major);
-	bit  = AUDIT_BIT(ctx->major);
-
 	if (list_empty(list))
 		return 0;
 
 	list_for_each_entry_rcu(e, list, list) {
-		if ((e->rule.mask[word] & bit) == bit &&
+		if (audit_in_mask(&e->rule, ctx->major) &&
 		    audit_filter_rules(tsk, &e->rule, ctx, n, &state, false)) {
 			ctx->current_state = state;
 			return 1;
-- 
1.9.0

[PATCH 2/2] audit: do not select HAVE_ARCH_AUDITSYSCALL on x32

[Eric Paris]

When x32 was introduced it assumed that it would get syscall audit for
free (since it works for x86 and x86_64).  However, the audit system
assumed that the syscall table has less that 2048 entries.  This is not
the case for x32 (or it is, kinda sorta)

Since audit syscall does not work on x32 stop selecting it.

Signed-off-by: Eric Paris <eparis@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 56f47ca..e11c4da 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -125,7 +125,7 @@ config X86
 	select RTC_LIB
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
-	select HAVE_ARCH_AUDITSYSCALL
+	select HAVE_ARCH_AUDITSYSCALL if !X86_X32
 
 config INSTRUCTION_DECODER
 	def_bool y
-- 
1.9.0

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Greg KH]

On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
> From: Andy Lutomirski <luto@amacapital.net>
> 
> Fixes an easy DoS and possible information disclosure.
> 
> This does nothing about the broken state of x32 auditing.
> 
> eparis: If the admin has enabled auditd and has specifically loaded audit
> rules.  This bug has been around since before git.  Wow...
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
>  kernel/auditsc.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)

Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
miss a later conversation about this?

thanks,

greg k-h

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Andy Lutomirski]

On Mon, Jun 9, 2014 at 3:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
>> From: Andy Lutomirski <luto@amacapital.net>
>>
>> Fixes an easy DoS and possible information disclosure.
>>
>> This does nothing about the broken state of x32 auditing.
>>
>> eparis: If the admin has enabled auditd and has specifically loaded audit
>> rules.  This bug has been around since before git.  Wow...
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> Signed-off-by: Eric Paris <eparis@redhat.com>
>> ---
>>  kernel/auditsc.c | 27 ++++++++++++++++++---------
>>  1 file changed, 18 insertions(+), 9 deletions(-)
>
> Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
> miss a later conversation about this?

Hmm.  It seems that it didn't make it into Linus' tree.  Crap.

IMO we need some kind of real tracking system for issues reported to
security@.  This shouldn't have been possible (and if I'd realized
that the patch got dropped, I wouldn't have publicly disclosed it).

For whoever applies this: it's CVE-2014-3917.

--Andy

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Greg KH]

On Mon, Jun 09, 2014 at 03:35:02PM -0700, Andy Lutomirski wrote:
> On Mon, Jun 9, 2014 at 3:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
> >> From: Andy Lutomirski <luto@amacapital.net>
> >>
> >> Fixes an easy DoS and possible information disclosure.
> >>
> >> This does nothing about the broken state of x32 auditing.
> >>
> >> eparis: If the admin has enabled auditd and has specifically loaded audit
> >> rules.  This bug has been around since before git.  Wow...
> >>
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> >> Signed-off-by: Eric Paris <eparis@redhat.com>
> >> ---
> >>  kernel/auditsc.c | 27 ++++++++++++++++++---------
> >>  1 file changed, 18 insertions(+), 9 deletions(-)
> >
> > Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
> > miss a later conversation about this?
> 
> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.
> 
> IMO we need some kind of real tracking system for issues reported to
> security@.

That seems to be my mbox at times :)

But yes, having something "real" might be good if the load gets higher,
right now it's so low that my "sweep pending security patches" task
usually catches anything pending, which is rare.

thanks,

greg k-h

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Linus Torvalds]

On Mon, Jun 9, 2014 at 3:35 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.

I assume that if there is a maintainer who normally sends me stuff by
git, when I see patches in emails they are just informational
heads-ups about stuff that is being discussed or pending, and that
I'll see it later in a pull request. So I just ignore them unless I
have specific comments, since clearly the emailed patch is just
informational and/or for comments/acks from others.

The exception is unless it *VERY CLEARLY* says otherwise (as in
"Linus, can you please take this directly due to xyz").

Because why would somebody send me a patch series sometimes, and git
trees at other times? That would just be stupid.

              Linus

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Andy Lutomirski]

On Mon, Jun 9, 2014 at 3:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 09, 2014 at 03:35:02PM -0700, Andy Lutomirski wrote:
>> On Mon, Jun 9, 2014 at 3:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
>> >> From: Andy Lutomirski <luto@amacapital.net>
>> >>
>> >> Fixes an easy DoS and possible information disclosure.
>> >>
>> >> This does nothing about the broken state of x32 auditing.
>> >>
>> >> eparis: If the admin has enabled auditd and has specifically loaded audit
>> >> rules.  This bug has been around since before git.  Wow...
>> >>
>> >> Cc: stable@vger.kernel.org
>> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> >> Signed-off-by: Eric Paris <eparis@redhat.com>
>> >> ---
>> >>  kernel/auditsc.c | 27 ++++++++++++++++++---------
>> >>  1 file changed, 18 insertions(+), 9 deletions(-)
>> >
>> > Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
>> > miss a later conversation about this?
>>
>> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.
>>
>> IMO we need some kind of real tracking system for issues reported to
>> security@.
>
> That seems to be my mbox at times :)
>
> But yes, having something "real" might be good if the load gets higher,
> right now it's so low that my "sweep pending security patches" task
> usually catches anything pending, which is rare.
>

There are currently at least two issues that I reported that are stuck
in limbo: this one and the (not-yet-public) vfs thing.  And there's
the CVE-2014-0181 regression fix that almost got forgotten, but that
isn't really a security issue.

And I can't read your mbox :-/

--Andy

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Andy Lutomirski]

On Mon, Jun 9, 2014 at 3:53 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Jun 9, 2014 at 3:35 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>
>> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.
>
> I assume that if there is a maintainer who normally sends me stuff by
> git, when I see patches in emails they are just informational
> heads-ups about stuff that is being discussed or pending, and that
> I'll see it later in a pull request. So I just ignore them unless I
> have specific comments, since clearly the emailed patch is just
> informational and/or for comments/acks from others.
>
> The exception is unless it *VERY CLEARLY* says otherwise (as in
> "Linus, can you please take this directly due to xyz").
>
> Because why would somebody send me a patch series sometimes, and git
> trees at other times? That would just be stupid.

In this particular case, it's my patch, and I've never sent you a pull
request.  I sort of assumed that security@kernel.org magically caused
acknowledged fixes to end up in your tree.  I'm not sure what I'm
supposed to do here.

Maybe the confusion is because Eric resent the patch?

--Andy

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Josh Boyer]

On Mon, Jun 9, 2014 at 6:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 09, 2014 at 03:35:02PM -0700, Andy Lutomirski wrote:
>> On Mon, Jun 9, 2014 at 3:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
>> >> From: Andy Lutomirski <luto@amacapital.net>
>> >>
>> >> Fixes an easy DoS and possible information disclosure.
>> >>
>> >> This does nothing about the broken state of x32 auditing.
>> >>
>> >> eparis: If the admin has enabled auditd and has specifically loaded audit
>> >> rules.  This bug has been around since before git.  Wow...
>> >>
>> >> Cc: stable@vger.kernel.org
>> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> >> Signed-off-by: Eric Paris <eparis@redhat.com>
>> >> ---
>> >>  kernel/auditsc.c | 27 ++++++++++++++++++---------
>> >>  1 file changed, 18 insertions(+), 9 deletions(-)
>> >
>> > Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
>> > miss a later conversation about this?
>>
>> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.
>>
>> IMO we need some kind of real tracking system for issues reported to
>> security@.
>
> That seems to be my mbox at times :)
>
> But yes, having something "real" might be good if the load gets higher,
> right now it's so low that my "sweep pending security patches" task
> usually catches anything pending, which is rare.

How does one get added to the security@ alias?  We've been carrying
this patch in Fedora for a bit now.  I'd be happy to help track things
given we get distro security bug reports and such.

josh

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Linus Torvalds]

On Mon, Jun 9, 2014 at 3:56 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> In this particular case, it's my patch, and I've never sent you a pull
> request.  I sort of assumed that security@kernel.org magically caused
> acknowledged fixes to end up in your tree.  I'm not sure what I'm
> supposed to do here.
>
> Maybe the confusion is because Eric resent the patch?

So I saw the patch twice in email , but neither time did I get the
feeling that I should apply it. The first time Eric responded to it,
so the maintainer clearly knew about it and was reacting to it, so I
ignored it. The second time Eric resent it as email to various people
and lists, and I didn't react to it because I expected that was again
just for discussion.

So I'm not blaming you as much as Eric. If a maintainer expects me to
pick it up from the email (rather than his usual git pulls), I want
that maintainer to *say* so. Because otherwise, as mentioned, I expect
it to come through the maintainer tree as usual.

              Linus

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Andy Lutomirski]

On Mon, Jun 9, 2014 at 5:32 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 09, 2014 at 03:55:20PM -0700, Andy Lutomirski wrote:
>> On Mon, Jun 9, 2014 at 3:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Mon, Jun 09, 2014 at 03:35:02PM -0700, Andy Lutomirski wrote:
>> >> On Mon, Jun 9, 2014 at 3:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> >> > On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
>> >> >> From: Andy Lutomirski <luto@amacapital.net>
>> >> >>
>> >> >> Fixes an easy DoS and possible information disclosure.
>> >> >>
>> >> >> This does nothing about the broken state of x32 auditing.
>> >> >>
>> >> >> eparis: If the admin has enabled auditd and has specifically loaded audit
>> >> >> rules.  This bug has been around since before git.  Wow...
>> >> >>
>> >> >> Cc: stable@vger.kernel.org
>> >> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> >> >> Signed-off-by: Eric Paris <eparis@redhat.com>
>> >> >> ---
>> >> >>  kernel/auditsc.c | 27 ++++++++++++++++++---------
>> >> >>  1 file changed, 18 insertions(+), 9 deletions(-)
>> >> >
>> >> > Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
>> >> > miss a later conversation about this?
>> >>
>> >> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.
>> >>
>> >> IMO we need some kind of real tracking system for issues reported to
>> >> security@.
>> >
>> > That seems to be my mbox at times :)
>> >
>> > But yes, having something "real" might be good if the load gets higher,
>> > right now it's so low that my "sweep pending security patches" task
>> > usually catches anything pending, which is rare.
>> >
>>
>> There are currently at least two issues that I reported that are stuck
>> in limbo: this one and the (not-yet-public) vfs thing.
>
> That was next on my list to poke people about...
>
>> And there's the CVE-2014-0181 regression fix that almost got
>> forgotten, but that isn't really a security issue.
>
> What is that, where was that reported?

commit 2d7a85f4b06e9c27ff629f07a524c48074f07f81
Author: Eric W. Biederman <ebiederm@xmission.com>
Date:   Fri May 30 11:04:00 2014 -0700

    netlink: Only check file credentials for implicit destinations


The security issue got fixed quickly, but the fix turned out to be problematic.

--Andy

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Greg KH]

On Mon, Jun 09, 2014 at 07:35:57PM -0400, Josh Boyer wrote:
> On Mon, Jun 9, 2014 at 6:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Mon, Jun 09, 2014 at 03:35:02PM -0700, Andy Lutomirski wrote:
> >> On Mon, Jun 9, 2014 at 3:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
> >> >> From: Andy Lutomirski <luto@amacapital.net>
> >> >>
> >> >> Fixes an easy DoS and possible information disclosure.
> >> >>
> >> >> This does nothing about the broken state of x32 auditing.
> >> >>
> >> >> eparis: If the admin has enabled auditd and has specifically loaded audit
> >> >> rules.  This bug has been around since before git.  Wow...
> >> >>
> >> >> Cc: stable@vger.kernel.org
> >> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> >> >> Signed-off-by: Eric Paris <eparis@redhat.com>
> >> >> ---
> >> >>  kernel/auditsc.c | 27 ++++++++++++++++++---------
> >> >>  1 file changed, 18 insertions(+), 9 deletions(-)
> >> >
> >> > Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
> >> > miss a later conversation about this?
> >>
> >> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.
> >>
> >> IMO we need some kind of real tracking system for issues reported to
> >> security@.
> >
> > That seems to be my mbox at times :)
> >
> > But yes, having something "real" might be good if the load gets higher,
> > right now it's so low that my "sweep pending security patches" task
> > usually catches anything pending, which is rare.
> 
> How does one get added to the security@ alias?  We've been carrying
> this patch in Fedora for a bit now.  I'd be happy to help track things
> given we get distro security bug reports and such.

Just ask on the security@ alias to be added and we can take it from
there.

thanks,

greg k-h

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Greg KH]

On Mon, Jun 09, 2014 at 03:55:20PM -0700, Andy Lutomirski wrote:
> On Mon, Jun 9, 2014 at 3:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Mon, Jun 09, 2014 at 03:35:02PM -0700, Andy Lutomirski wrote:
> >> On Mon, Jun 9, 2014 at 3:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
> >> >> From: Andy Lutomirski <luto@amacapital.net>
> >> >>
> >> >> Fixes an easy DoS and possible information disclosure.
> >> >>
> >> >> This does nothing about the broken state of x32 auditing.
> >> >>
> >> >> eparis: If the admin has enabled auditd and has specifically loaded audit
> >> >> rules.  This bug has been around since before git.  Wow...
> >> >>
> >> >> Cc: stable@vger.kernel.org
> >> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> >> >> Signed-off-by: Eric Paris <eparis@redhat.com>
> >> >> ---
> >> >>  kernel/auditsc.c | 27 ++++++++++++++++++---------
> >> >>  1 file changed, 18 insertions(+), 9 deletions(-)
> >> >
> >> > Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
> >> > miss a later conversation about this?
> >>
> >> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.
> >>
> >> IMO we need some kind of real tracking system for issues reported to
> >> security@.
> >
> > That seems to be my mbox at times :)
> >
> > But yes, having something "real" might be good if the load gets higher,
> > right now it's so low that my "sweep pending security patches" task
> > usually catches anything pending, which is rare.
> >
> 
> There are currently at least two issues that I reported that are stuck
> in limbo: this one and the (not-yet-public) vfs thing.

That was next on my list to poke people about...

> And there's the CVE-2014-0181 regression fix that almost got
> forgotten, but that isn't really a security issue.

What is that, where was that reported?

thanks,

greg k-h

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Greg KH]

On Mon, Jun 09, 2014 at 05:30:19PM -0700, Andy Lutomirski wrote:
> On Mon, Jun 9, 2014 at 5:32 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Mon, Jun 09, 2014 at 03:55:20PM -0700, Andy Lutomirski wrote:
> >> On Mon, Jun 9, 2014 at 3:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > On Mon, Jun 09, 2014 at 03:35:02PM -0700, Andy Lutomirski wrote:
> >> >> On Mon, Jun 9, 2014 at 3:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> >> > On Wed, May 28, 2014 at 11:09:58PM -0400, Eric Paris wrote:
> >> >> >> From: Andy Lutomirski <luto@amacapital.net>
> >> >> >>
> >> >> >> Fixes an easy DoS and possible information disclosure.
> >> >> >>
> >> >> >> This does nothing about the broken state of x32 auditing.
> >> >> >>
> >> >> >> eparis: If the admin has enabled auditd and has specifically loaded audit
> >> >> >> rules.  This bug has been around since before git.  Wow...
> >> >> >>
> >> >> >> Cc: stable@vger.kernel.org
> >> >> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> >> >> >> Signed-off-by: Eric Paris <eparis@redhat.com>
> >> >> >> ---
> >> >> >>  kernel/auditsc.c | 27 ++++++++++++++++++---------
> >> >> >>  1 file changed, 18 insertions(+), 9 deletions(-)
> >> >> >
> >> >> > Did this patch get dropped somewhere?  Isn't it a valid bugfix, or did I
> >> >> > miss a later conversation about this?
> >> >>
> >> >> Hmm.  It seems that it didn't make it into Linus' tree.  Crap.
> >> >>
> >> >> IMO we need some kind of real tracking system for issues reported to
> >> >> security@.
> >> >
> >> > That seems to be my mbox at times :)
> >> >
> >> > But yes, having something "real" might be good if the load gets higher,
> >> > right now it's so low that my "sweep pending security patches" task
> >> > usually catches anything pending, which is rare.
> >> >
> >>
> >> There are currently at least two issues that I reported that are stuck
> >> in limbo: this one and the (not-yet-public) vfs thing.
> >
> > That was next on my list to poke people about...
> >
> >> And there's the CVE-2014-0181 regression fix that almost got
> >> forgotten, but that isn't really a security issue.
> >
> > What is that, where was that reported?
> 
> commit 2d7a85f4b06e9c27ff629f07a524c48074f07f81
> Author: Eric W. Biederman <ebiederm@xmission.com>
> Date:   Fri May 30 11:04:00 2014 -0700
> 
>     netlink: Only check file credentials for implicit destinations
> 
> 
> The security issue got fixed quickly, but the fix turned out to be problematic.

Ah, thanks, I rely on Dave to send me networking stable patches, I'm
sure he's on this...

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Andy Lutomirski]

[cc list trimmed, security@ added]

On Mon, Jun 9, 2014 at 5:31 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 09, 2014 at 07:35:57PM -0400, Josh Boyer wrote:
>> On Mon, Jun 9, 2014 at 6:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> >
>> > But yes, having something "real" might be good if the load gets higher,
>> > right now it's so low that my "sweep pending security patches" task
>> > usually catches anything pending, which is rare.
>>
>> How does one get added to the security@ alias?  We've been carrying
>> this patch in Fedora for a bit now.  I'd be happy to help track things
>> given we get distro security bug reports and such.
>
> Just ask on the security@ alias to be added and we can take it from
> there.
>

Would it make sense for there to be someone on the security list who
can assign CVE numbers?

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Greg KH]

On Mon, Jun 09, 2014 at 05:51:37PM -0700, Andy Lutomirski wrote:
> [cc list trimmed, security@ added]
> 
> On Mon, Jun 9, 2014 at 5:31 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Mon, Jun 09, 2014 at 07:35:57PM -0400, Josh Boyer wrote:
> >> On Mon, Jun 9, 2014 at 6:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> >
> >> > But yes, having something "real" might be good if the load gets higher,
> >> > right now it's so low that my "sweep pending security patches" task
> >> > usually catches anything pending, which is rare.
> >>
> >> How does one get added to the security@ alias?  We've been carrying
> >> this patch in Fedora for a bit now.  I'd be happy to help track things
> >> given we get distro security bug reports and such.
> >
> > Just ask on the security@ alias to be added and we can take it from
> > there.
> >
> 
> Would it make sense for there to be someone on the security list who
> can assign CVE numbers?

I'm pretty sure we have that already.

thanks,

greg k-h

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Andy Lutomirski]

On Mon, Jun 9, 2014 at 7:57 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 09, 2014 at 05:51:37PM -0700, Andy Lutomirski wrote:
>> [cc list trimmed, security@ added]
>>
>> On Mon, Jun 9, 2014 at 5:31 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Mon, Jun 09, 2014 at 07:35:57PM -0400, Josh Boyer wrote:
>> >> On Mon, Jun 9, 2014 at 6:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> >> >
>> >> > But yes, having something "real" might be good if the load gets higher,
>> >> > right now it's so low that my "sweep pending security patches" task
>> >> > usually catches anything pending, which is rare.
>> >>
>> >> How does one get added to the security@ alias?  We've been carrying
>> >> this patch in Fedora for a bit now.  I'd be happy to help track things
>> >> given we get distro security bug reports and such.
>> >
>> > Just ask on the security@ alias to be added and we can take it from
>> > there.
>> >
>>
>> Would it make sense for there to be someone on the security list who
>> can assign CVE numbers?
>
> I'm pretty sure we have that already.

Let me rephrase the question:

Would it make sense for someone on the security list to assign CVE numbers?

--Andy

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Greg KH]

On Mon, Jun 09, 2014 at 09:04:16PM -0700, Andy Lutomirski wrote:
> On Mon, Jun 9, 2014 at 7:57 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Mon, Jun 09, 2014 at 05:51:37PM -0700, Andy Lutomirski wrote:
> >> [cc list trimmed, security@ added]
> >>
> >> On Mon, Jun 9, 2014 at 5:31 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > On Mon, Jun 09, 2014 at 07:35:57PM -0400, Josh Boyer wrote:
> >> >> On Mon, Jun 9, 2014 at 6:46 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> >> >
> >> >> > But yes, having something "real" might be good if the load gets higher,
> >> >> > right now it's so low that my "sweep pending security patches" task
> >> >> > usually catches anything pending, which is rare.
> >> >>
> >> >> How does one get added to the security@ alias?  We've been carrying
> >> >> this patch in Fedora for a bit now.  I'd be happy to help track things
> >> >> given we get distro security bug reports and such.
> >> >
> >> > Just ask on the security@ alias to be added and we can take it from
> >> > there.
> >> >
> >>
> >> Would it make sense for there to be someone on the security list who
> >> can assign CVE numbers?
> >
> > I'm pretty sure we have that already.
> 
> Let me rephrase the question:
> 
> Would it make sense for someone on the security list to assign CVE numbers?

If we cared about CVE numbers, maybe :)

Seriously, there are people on the security alias that can get CVE
numbers assigned if needed, so that should not be an issue.  It's
happened in the past from what I can recall.

thanks,

greg k-h

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Eric Paris]

On Mon, 2014-06-09 at 16:36 -0700, Linus Torvalds wrote:
> On Mon, Jun 9, 2014 at 3:56 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> >
> > In this particular case, it's my patch, and I've never sent you a pull
> > request.  I sort of assumed that security@kernel.org magically caused
> > acknowledged fixes to end up in your tree.  I'm not sure what I'm
> > supposed to do here.
> >
> > Maybe the confusion is because Eric resent the patch?
> 
> So I saw the patch twice in email , but neither time did I get the
> feeling that I should apply it. The first time Eric responded to it,
> so the maintainer clearly knew about it and was reacting to it, so I
> ignored it. The second time Eric resent it as email to various people
> and lists, and I didn't react to it because I expected that was again
> just for discussion.
> 
> So I'm not blaming you as much as Eric.

No, it's good to blame me.  I was trying to deal with it as fast as I
could since I was already trying to ignore my computer before I got
married last weekend and took the last week off.  I realized when I got
back yesterday you hadn't picked it up and it was on my list of things
to try to handle today.  I think both 1 and 2 are good to be applied to
your tree.  Although only #1 is really an absolutely critical issue.

>  If a maintainer expects me to
> pick it up from the email (rather than his usual git pulls), I want
> that maintainer to *say* so. Because otherwise, as mentioned, I expect
> it to come through the maintainer tree as usual.
> 
>               Linus

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Eric Paris]

On Mon, 2014-06-09 at 16:36 -0700, Linus Torvalds wrote:
> On Mon, Jun 9, 2014 at 3:56 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> >
> > In this particular case, it's my patch, and I've never sent you a pull
> > request.  I sort of assumed that security@kernel.org magically caused
> > acknowledged fixes to end up in your tree.  I'm not sure what I'm
> > supposed to do here.
> >
> > Maybe the confusion is because Eric resent the patch?
> 
> So I saw the patch twice in email , but neither time did I get the
> feeling that I should apply it. The first time Eric responded to it,
> so the maintainer clearly knew about it and was reacting to it, so I
> ignored it. The second time Eric resent it as email to various people
> and lists, and I didn't react to it because I expected that was again
> just for discussion.
> 
> So I'm not blaming you as much as Eric.

No, it's good to blame me.  I was trying to deal with it as fast as I
could since I was already trying to ignore my computer before I got
married last weekend and took the last week off.  I realized when I got
back yesterday you hadn't picked it up and it was on my list of things
to try to handle today.  I think both 1 and 2 are good to be applied to
your tree.  Although only #1 is really an absolutely critical issue.

>  If a maintainer expects me to
> pick it up from the email (rather than his usual git pulls), I want
> that maintainer to *say* so. Because otherwise, as mentioned, I expect
> it to come through the maintainer tree as usual.
> 
>               Linus

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Linus Torvalds]

On Tue, Jun 10, 2014 at 5:50 AM, Eric Paris <eparis@redhat.com> wrote:
>
> No, it's good to blame me.  I was trying to deal with it as fast as I
> could since I was already trying to ignore my computer before I got
> married last weekend and took the last week off.  I realized when I got
> back yesterday you hadn't picked it up and it was on my list of things
> to try to handle today.  I think both 1 and 2 are good to be applied to
> your tree.  Although only #1 is really an absolutely critical issue.

Ok, I'll take your patch-series rather than the recent pull from Andy,
and pick up #2 that way too.

I'll just take them from emails - it's not like I have to wait for a
pull from you. It's just that I don't want to take them from emails
_and_ then get them in a pull from you, which is why I tend to want to
get explicit "please apply these directly" notification.

                Linus

Re: [PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Linus Torvalds]

On Tue, Jun 10, 2014 at 8:42 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Ok, I'll take your patch-series rather than the recent pull from Andy,
> and pick up #2 that way too.

Hmm. In fact, #2 doesn't apply cleanly. It's trivial to fix up, but
rather than do that, the reject made me go "I'll just forward this to
Peter Anvin" instead, so that he's aware of the x32 issue.

            Linus

[PATCH 1/2] auditsc: audit_krule mask accesses need bounds checking

[Andy Lutomirski]

Fixes an easy DoS and possible information disclosure.

This does nothing about the broken state of x32 auditing.

Cc: stable@vger.kernel.org
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 kernel/auditsc.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index f251a5e..7ccd9db 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -728,6 +728,22 @@ static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)
 	return AUDIT_BUILD_CONTEXT;
 }
 
+static bool audit_in_mask(const struct audit_krule *rule, unsigned long val)
+{
+	int word, bit;
+
+	if (val > 0xffffffff)
+		return false;
+
+	word = AUDIT_WORD(val);
+	if (word >= AUDIT_BITMASK_SIZE)
+		return false;
+
+	bit = AUDIT_BIT(val);
+
+	return rule->mask[word] & bit;
+}
+
 /* At syscall entry and exit time, this filter is called if the
  * audit_state is not low enough that auditing cannot take place, but is
  * also not high enough that we already know we have to write an audit
@@ -745,11 +761,8 @@ static enum audit_state audit_filter_syscall(struct task_struct *tsk,
 
 	rcu_read_lock();
 	if (!list_empty(list)) {
-		int word = AUDIT_WORD(ctx->major);
-		int bit  = AUDIT_BIT(ctx->major);
-
 		list_for_each_entry_rcu(e, list, list) {
-			if ((e->rule.mask[word] & bit) == bit &&
+			if (audit_in_mask(&e->rule, ctx->major) &&
 			    audit_filter_rules(tsk, &e->rule, ctx, NULL,
 					       &state, false)) {
 				rcu_read_unlock();
@@ -769,20 +782,16 @@ static enum audit_state audit_filter_syscall(struct task_struct *tsk,
 static int audit_filter_inode_name(struct task_struct *tsk,
 				   struct audit_names *n,
 				   struct audit_context *ctx) {
-	int word, bit;
 	int h = audit_hash_ino((u32)n->ino);
 	struct list_head *list = &audit_inode_hash[h];
 	struct audit_entry *e;
 	enum audit_state state;
 
-	word = AUDIT_WORD(ctx->major);
-	bit  = AUDIT_BIT(ctx->major);
-
 	if (list_empty(list))
 		return 0;
 
 	list_for_each_entry_rcu(e, list, list) {
-		if ((e->rule.mask[word] & bit) == bit &&
+		if (audit_in_mask(&e->rule, ctx->major) &&
 		    audit_filter_rules(tsk, &e->rule, ctx, n, &state, false)) {
 			ctx->current_state = state;
 			return 1;
-- 
1.9.3