Re: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c

Re: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c

[Mark Salyzyn]

ToT commit 97f3c0a4b0579b646b6b10ae5a3d59f0441cc12c

(ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c)

was assigned CVE-2017-13695 
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-13695
and has been public since August 25 2017

Please apply to 3.18, 4.4 and 4.9 stable kernels for the reasons 
outlined in the body of the patch:

"This cache leak causes a security threat because an old kernel (<= 4.9) 
shows memory locations of kernel functions in stack dump. Some malicious 
users could use this information to neutralize kernel ASLR."

Bonus Points: Since the patch is ToT upstream, relieving the bug that 
results in the memory leak, even despite the non-CVE security status for 
<=4.12 kernels, it may be advised to also include this patch in 4.14.y 
stable as well.

Sincerely -- Mark Salyzyn

Re: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c

[Greg KH]

On Wed, May 09, 2018 at 02:30:14PM -0700, Mark Salyzyn wrote:
> ToT commit 97f3c0a4b0579b646b6b10ae5a3d59f0441cc12c

"ToT"?  What does that mean?

> 
> (ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c)
> 
> was assigned CVE-2017-13695
> https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-13695
> and has been public since August 25 2017
> 
> Please apply to 3.18, 4.4 and 4.9 stable kernels for the reasons outlined in
> the body of the patch:
> 
> "This cache leak causes a security threat because an old kernel (<= 4.9)
> shows memory locations of kernel functions in stack dump. Some malicious
> users could use this information to neutralize kernel ASLR."
> 
> Bonus Points: Since the patch is ToT upstream, relieving the bug that
> results in the memory leak, even despite the non-CVE security status for
> <=4.12 kernels, it may be advised to also include this patch in 4.14.y
> stable as well.

Well, I wouldn't apply a patch to just older kernels and not newer ones,
that just causes confusion.

But I'm going to push back on this.  The kernel security team said
something like "this is crazy, if you control ACPI tables you have
bigger problems" when this bug was reported and told the developer to
just submit this as a normal code cleanup.

Granting this a CVE was, in my opinion, a total mistake as well.  This
doesn't fix any "real" problem that anyone can hit in the wild from what
I can tell.  And again, if you can modify ACPI tables, there are much
bigger problems you can cause on the hardware.

Because of this, why would you need/want this in the stable kernel
releases?  It doesn't fix any real bug, only a theoretical one, right?

ACPI developers, do you think this should be backported?

thanks,

greg k-h

RE: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c

[Schmauss, Erik]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Wednesday, May 9, 2018 11:55 PM
> To: Mark Salyzyn <salyzyn@android.com>
> Cc: stable <stable@vger.kernel.org>; Seunghun Han <kkamagui@gmail.com>;
> Schmauss, Erik <erik.schmauss@intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki@intel.com>; kernel-team <kernel-team@android.com>
> Subject: Re: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c
> 
> On Wed, May 09, 2018 at 02:30:14PM -0700, Mark Salyzyn wrote:
> > ToT commit 97f3c0a4b0579b646b6b10ae5a3d59f0441cc12c
> 
> "ToT"?  What does that mean?
> 
> >
> > (ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c)
> >
> > was assigned CVE-2017-13695
> > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-13695
> > and has been public since August 25 2017
> >
> > Please apply to 3.18, 4.4 and 4.9 stable kernels for the reasons
> > outlined in the body of the patch:
> >
> > "This cache leak causes a security threat because an old kernel (<=
> > 4.9) shows memory locations of kernel functions in stack dump. Some
> > malicious users could use this information to neutralize kernel ASLR."
> >
> > Bonus Points: Since the patch is ToT upstream, relieving the bug that
> > results in the memory leak, even despite the non-CVE security status
> > for
> > <=4.12 kernels, it may be advised to also include this patch in 4.14.y
> > stable as well.
> 
> Well, I wouldn't apply a patch to just older kernels and not newer ones, that just
> causes confusion.
> 
> But I'm going to push back on this.  The kernel security team said something like
> "this is crazy, if you control ACPI tables you have bigger problems" when this bug
> was reported and told the developer to just submit this as a normal code
> cleanup.
> 
> Granting this a CVE was, in my opinion, a total mistake as well.  This doesn't fix
> any "real" problem that anyone can hit in the wild from what I can tell.  And
> again, if you can modify ACPI tables, there are much bigger problems you can
> cause on the hardware.

Agreed. Could we somehow close this CVE?

> 
> Because of this, why would you need/want this in the stable kernel releases?  It
> doesn't fix any real bug, only a theoretical one, right?

The AML would need to be carefully crafted. So yes, this could happen in theory.

> 
> ACPI developers, do you think this should be backported?

One reason to backport this patch is that it performs memory reclamation for
certain code paths. So no, not necessary but it might be a nice-to-have.

Erik

> 
> thanks,
> 
> greg k-h

Re: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c

[Greg KH]

On Thu, May 10, 2018 at 06:45:42PM +0000, Schmauss, Erik wrote:
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Wednesday, May 9, 2018 11:55 PM
> > To: Mark Salyzyn <salyzyn@android.com>
> > Cc: stable <stable@vger.kernel.org>; Seunghun Han <kkamagui@gmail.com>;
> > Schmauss, Erik <erik.schmauss@intel.com>; Wysocki, Rafael J
> > <rafael.j.wysocki@intel.com>; kernel-team <kernel-team@android.com>
> > Subject: Re: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c
> > 
> > On Wed, May 09, 2018 at 02:30:14PM -0700, Mark Salyzyn wrote:
> > > ToT commit 97f3c0a4b0579b646b6b10ae5a3d59f0441cc12c
> > 
> > "ToT"?  What does that mean?
> > 
> > >
> > > (ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c)
> > >
> > > was assigned CVE-2017-13695
> > > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-13695
> > > and has been public since August 25 2017
> > >
> > > Please apply to 3.18, 4.4 and 4.9 stable kernels for the reasons
> > > outlined in the body of the patch:
> > >
> > > "This cache leak causes a security threat because an old kernel (<=
> > > 4.9) shows memory locations of kernel functions in stack dump. Some
> > > malicious users could use this information to neutralize kernel ASLR."
> > >
> > > Bonus Points: Since the patch is ToT upstream, relieving the bug that
> > > results in the memory leak, even despite the non-CVE security status
> > > for
> > > <=4.12 kernels, it may be advised to also include this patch in 4.14.y
> > > stable as well.
> > 
> > Well, I wouldn't apply a patch to just older kernels and not newer ones, that just
> > causes confusion.
> > 
> > But I'm going to push back on this.  The kernel security team said something like
> > "this is crazy, if you control ACPI tables you have bigger problems" when this bug
> > was reported and told the developer to just submit this as a normal code
> > cleanup.
> > 
> > Granting this a CVE was, in my opinion, a total mistake as well.  This doesn't fix
> > any "real" problem that anyone can hit in the wild from what I can tell.  And
> > again, if you can modify ACPI tables, there are much bigger problems you can
> > cause on the hardware.
> 
> Agreed. Could we somehow close this CVE?

Please do, you can submit a request for it to be rejected on the main
CVE site somewhere.  I've done it once in the past.

> > Because of this, why would you need/want this in the stable kernel releases?  It
> > doesn't fix any real bug, only a theoretical one, right?
> 
> The AML would need to be carefully crafted. So yes, this could happen in theory.
> 
> > 
> > ACPI developers, do you think this should be backported?
> 
> One reason to backport this patch is that it performs memory reclamation for
> certain code paths. So no, not necessary but it might be a nice-to-have.

Nice to have for what?  If the AML is correct (as all devices have), all
should be fine, right?

thanks,

greg k-h

RE: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c

[Schmauss, Erik]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, May 10, 2018 10:24 PM
> To: Schmauss, Erik <erik.schmauss@intel.com>
> Cc: Mark Salyzyn <salyzyn@android.com>; Moore, Robert
> <robert.moore@intel.com>; stable <stable@vger.kernel.org>; Seunghun Han
> <kkamagui@gmail.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>;
> kernel-team <kernel-team@android.com>
> Subject: Re: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c
> 
> On Thu, May 10, 2018 at 06:45:42PM +0000, Schmauss, Erik wrote:
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Wednesday, May 9, 2018 11:55 PM
> > > To: Mark Salyzyn <salyzyn@android.com>
> > > Cc: stable <stable@vger.kernel.org>; Seunghun Han
> > > <kkamagui@gmail.com>; Schmauss, Erik <erik.schmauss@intel.com>;
> > > Wysocki, Rafael J <rafael.j.wysocki@intel.com>; kernel-team
> > > <kernel-team@android.com>
> > > Subject: Re: ACPICA: acpi: acpica: fix acpi operand cache leak in
> > > nseval.c
> > >
> > > On Wed, May 09, 2018 at 02:30:14PM -0700, Mark Salyzyn wrote:
> > > > ToT commit 97f3c0a4b0579b646b6b10ae5a3d59f0441cc12c
> > >
> > > "ToT"?  What does that mean?
> > >
> > > >
> > > > (ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c)
> > > >
> > > > was assigned CVE-2017-13695
> > > > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-13695
> > > > and has been public since August 25 2017
> > > >
> > > > Please apply to 3.18, 4.4 and 4.9 stable kernels for the reasons
> > > > outlined in the body of the patch:
> > > >
> > > > "This cache leak causes a security threat because an old kernel
> > > > (<=
> > > > 4.9) shows memory locations of kernel functions in stack dump.
> > > > Some malicious users could use this information to neutralize kernel ASLR."
> > > >
> > > > Bonus Points: Since the patch is ToT upstream, relieving the bug
> > > > that results in the memory leak, even despite the non-CVE security
> > > > status for
> > > > <=4.12 kernels, it may be advised to also include this patch in
> > > > 4.14.y stable as well.
> > >
> > > Well, I wouldn't apply a patch to just older kernels and not newer
> > > ones, that just causes confusion.
> > >
> > > But I'm going to push back on this.  The kernel security team said
> > > something like "this is crazy, if you control ACPI tables you have
> > > bigger problems" when this bug was reported and told the developer
> > > to just submit this as a normal code cleanup.
> > >
> > > Granting this a CVE was, in my opinion, a total mistake as well.
> > > This doesn't fix any "real" problem that anyone can hit in the wild
> > > from what I can tell.  And again, if you can modify ACPI tables,
> > > there are much bigger problems you can cause on the hardware.
> >
> > Agreed. Could we somehow close this CVE?
> 
> Please do, you can submit a request for it to be rejected on the main CVE site
> somewhere.  I've done it once in the past.
Ok. I'll do this.
> 
> > > Because of this, why would you need/want this in the stable kernel
> > > releases?  It doesn't fix any real bug, only a theoretical one, right?
> >
> > The AML would need to be carefully crafted. So yes, this could happen in
> theory.
> >
> > >
> > > ACPI developers, do you think this should be backported?
> >
> > One reason to backport this patch is that it performs memory
> > reclamation for certain code paths. So no, not necessary but it might be a
> nice-to-have.
> 
> Nice to have for what?  If the AML is correct (as all devices have), all should be
> fine, right?

If the AML is correct, it's fine. Almost all OEMs use ASL compilers like iASL to ensure
correctness of ASL/AML.

This patch might be nice to have for when users wish to alter their ACPI tables by hand
and those altered ACPI tables cause this memory leak. If you wish to account for
memory leaks that result from these hand-crafted AML files, then you should add this
patch. Otherwise, it's not necessary.

Erik
> 
> thanks,
> 
> greg k-h

Re: ACPICA: acpi: acpica: fix acpi operand cache leak in nseval.c

[Mark Salyzyn]

On 05/15/2018 10:36 AM, Schmauss, Erik wrote:
>>>> But I'm going to push back on this.  The kernel security team said
>>>> something like "this is crazy, if you control ACPI tables you have
>>>> bigger problems" when this bug was reported and told the developer
>>>> to just submit this as a normal code cleanup.
>>>>
>>>> Granting this a CVE was, in my opinion, a total mistake as well.
>>>> This doesn't fix any "real" problem that anyone can hit in the wild
>>>> from what I can tell.  And again, if you can modify ACPI tables,
>>>> there are much bigger problems you can cause on the hardware.
>>> Agreed. Could we somehow close this CVE?
>> Please do, you can submit a request for it to be rejected on the main CVE site
>> somewhere.  I've done it once in the past.
> Ok. I'll do this.

Thanks!

Please do the same for CVE-2017-13694 (not in Linus' tree) as well as 
this one CVE-2017-13695 (in Linus' tree) as they are both associated 
with crafted ACPI tables.

I am rescinding my request to have these in stable for security concerns.

> If the AML is correct, it's fine. Almost all OEMs use ASL compilers like iASL to ensure
> correctness of ASL/AML.
That probably is enough to push back on stable, really an academic 
defence in depth measure.
>
> This patch might be nice to have for when users wish to alter their ACPI tables by hand
> and those altered ACPI tables cause this memory leak. If you wish to account for
> memory leaks that result from these hand-crafted AML files, then you should add this
> patch. Otherwise, it's not necessary.
Linus' tree has this, should deal with those advanced developers/users 
that wish to alter their ACPI tables by hand? The leak is probably a 
smaller issue than what can happen if someone decides to adjust them by 
hand ;-}


-- Mark