SmmCommunicationCommunicate question?

SmmCommunicationCommunicate question?

[Anbazhagan, Baraneedharan]

Whether TPL needs to be raised before setting CommunicationBuffer and BufferSize in gSmmCorePrivate to avoid a callback overwriting those values before triggering SW SMI?

-Baranee

Re: SmmCommunicationCommunicate question?

[Laszlo Ersek]

On 10/13/16 05:40, Anbazhagan, Baraneedharan wrote:
> Whether TPL needs to be raised before setting CommunicationBuffer and
> BufferSize in gSmmCorePrivate to avoid a callback overwriting those
> values before triggering SW SMI?

I don't think so. The contents of the communication buffer is untrusted
(it is passed in from the unprivileged side to the privileged side), so
the SMI handler (running in SMM) has to do full validation /
verification anyway.

Another way to look at this is the following: assume that the runtime OS
calls a variable service on one processor. That variable service has an
unprivileged part (which formats the request in the communication
buffer, and triggers the SMI), and a privileged part (the code that
validates and serves the request in SMM). Now assume that the runtime OS
deliberately races, on another processor, with the unpriv variable
service code that runs on the first processor; i.e., the second CPU
tries to tamper with the communication buffer just in the time window
that you describe.

There's nothing that the unpriv variable code running on the first
processor can do against this.

Instead, once the first CPU enters SMM, it brings all the other CPUs
into SMM as well, where they will be executing known, secure code --
i.e., the first CPU to enter SMM forces the other CPUs to temporarily
abandon any (possibly malicious) code the runtime OS may have prepared.
Only *then* will the verification of the communication buffer commence.
If the malicious code managed to race the unpriv part of the service
successfully, now the privileged part will catch that, undisturbed.

Thanks
Laszlo

Re: SmmCommunicationCommunicate question?

[Paolo Bonzini]

On 13/10/2016 11:07, Laszlo Ersek wrote:
> 
> Instead, once the first CPU enters SMM, it brings all the other CPUs
> into SMM as well, where they will be executing known, secure code --
> i.e., the first CPU to enter SMM forces the other CPUs to temporarily
> abandon any (possibly malicious) code the runtime OS may have prepared.
> Only *then* will the verification of the communication buffer commence.
> If the malicious code managed to race the unpriv part of the service
> successfully, now the privileged part will catch that, undisturbed.

Even this is not strictly necessary if you can set aside some memory in
SMRAM for a copy the communication buffer.  Then you can do:

   tmp = comm buffer size
   if tmp > sizeof(privileged buffer)
       return error
   copy tmp bytes from comm buffer to privileged buffer

and not look anymore at the buffer provided by the user.

Of course, "bring all CPUs into SMM" can double as a poor man's mutex,
so there may be reasons to do that anyway.

Paolo

Re: SmmCommunicationCommunicate question?

[Anbazhagan, Baraneedharan]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> 
> On 13/10/2016 11:07, Laszlo Ersek wrote:
> >
> > Instead, once the first CPU enters SMM, it brings all the other CPUs
> > into SMM as well, where they will be executing known, secure code --
> > i.e., the first CPU to enter SMM forces the other CPUs to temporarily
> > abandon any (possibly malicious) code the runtime OS may have prepared.
> > Only *then* will the verification of the communication buffer commence.
> > If the malicious code managed to race the unpriv part of the service
> > successfully, now the privileged part will catch that, undisturbed.
> 
> Even this is not strictly necessary if you can set aside some memory in SMRAM for a
> copy the communication buffer.  Then you can do:
> 
>    tmp = comm buffer size
>    if tmp > sizeof(privileged buffer)
>        return error
>    copy tmp bytes from comm buffer to privileged buffer
> 
> and not look anymore at the buffer provided by the user.
> 
> Of course, "bring all CPUs into SMM" can double as a poor man's mutex, so there
> may be reasons to do that anyway.
> 
> Paolo

Am thinking in BDS phase - if a module have periodic callback and uses SmmCommunicate within the callback, then it could potentially overwrite those gSmmCorePrivate pointer while another module trying to use SmmCommunicate.

Re: SmmCommunicationCommunicate question?

[Ken Taylor]

I think there are a couple of assumptions here that should be reconsidered...

First, it is not always the case that entry into SMM on one CPU will always pull all CPUs into SMM.  There are methods to deliver targeted SMIs via the local APIC on some processors.  In addition, I have 2nd hand knowledge that some processors don't immediately return to SMM on RSM if other processors are still in SMM; this allows some processors to resume early and continue execution while execution on other cores continues in SMM.

Second, CPUs are not the only bus master capable of changing the contents of a CommBuffer that is passed to an SMI handler.  I could, for example, schedule a USB or a SATA transaction that will clobber CommBuffer contents some arbitrary amount of time after I've triggered an SMI, and CommBuffer would change on the fly even if all my processors are executing known good code in SMM.

If you want your SMI handler code to be safe, as a first step, either copy CommBuffer to a local buffer in SMM, or copy all critical parameters such as pointers, BARs, object lengths and commands to local variables. Operate only on local copies from that point forward.

 Regards,
-Ken.

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Anbazhagan, Baraneedharan
Sent: Thursday, October 13, 2016 6:20 AM
To: Paolo Bonzini; Laszlo Ersek
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] SmmCommunicationCommunicate question?

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> 
> On 13/10/2016 11:07, Laszlo Ersek wrote:
> >
> > Instead, once the first CPU enters SMM, it brings all the other CPUs
> > into SMM as well, where they will be executing known, secure code --
> > i.e., the first CPU to enter SMM forces the other CPUs to temporarily
> > abandon any (possibly malicious) code the runtime OS may have prepared.
> > Only *then* will the verification of the communication buffer commence.
> > If the malicious code managed to race the unpriv part of the service
> > successfully, now the privileged part will catch that, undisturbed.
> 
> Even this is not strictly necessary if you can set aside some memory in SMRAM for a
> copy the communication buffer.  Then you can do:
> 
>    tmp = comm buffer size
>    if tmp > sizeof(privileged buffer)
>        return error
>    copy tmp bytes from comm buffer to privileged buffer
> 
> and not look anymore at the buffer provided by the user.
> 
> Of course, "bring all CPUs into SMM" can double as a poor man's mutex, so there
> may be reasons to do that anyway.
> 
> Paolo

Am thinking in BDS phase - if a module have periodic callback and uses SmmCommunicate within the callback, then it could potentially overwrite those gSmmCorePrivate pointer while another module trying to use SmmCommunicate.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: SmmCommunicationCommunicate question?

[Laszlo Ersek]

On 10/14/16 01:52, Ken Taylor wrote:
> I think there are a couple of assumptions here that should be
> reconsidered...
> 
> First, it is not always the case that entry into SMM on one CPU will
> always pull all CPUs into SMM.  There are methods to deliver targeted
> SMIs via the local APIC on some processors.  In addition, I have 2nd
> hand knowledge that some processors don't immediately return to SMM
> on RSM if other processors are still in SMM; this allows some
> processors to resume early and continue execution while execution on
> other cores continues in SMM.
> 
> Second, CPUs are not the only bus master capable of changing the
> contents of a CommBuffer that is passed to an SMI handler.  I could,
> for example, schedule a USB or a SATA transaction that will clobber
> CommBuffer contents some arbitrary amount of time after I've
> triggered an SMI, and CommBuffer would change on the fly even if all
> my processors are executing known good code in SMM.
> 
> If you want your SMI handler code to be safe, as a first step, either
> copy CommBuffer to a local buffer in SMM, or copy all critical
> parameters such as pointers, BARs, object lengths and commands to
> local variables. Operate only on local copies from that point
> forward.

Good points, thank you! (Practically elaborating on what Paolo said as
well.) I completely missed PCI DMA here.

Thanks!
Laszlo

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Anbazhagan, Baraneedharan
> Sent: Thursday, October 13, 2016 6:20 AM
> To: Paolo Bonzini; Laszlo Ersek
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] SmmCommunicationCommunicate question?
> 
>> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
>>
>> On 13/10/2016 11:07, Laszlo Ersek wrote:
>>>
>>> Instead, once the first CPU enters SMM, it brings all the other CPUs
>>> into SMM as well, where they will be executing known, secure code --
>>> i.e., the first CPU to enter SMM forces the other CPUs to temporarily
>>> abandon any (possibly malicious) code the runtime OS may have prepared.
>>> Only *then* will the verification of the communication buffer commence.
>>> If the malicious code managed to race the unpriv part of the service
>>> successfully, now the privileged part will catch that, undisturbed.
>>
>> Even this is not strictly necessary if you can set aside some memory in SMRAM for a
>> copy the communication buffer.  Then you can do:
>>
>>    tmp = comm buffer size
>>    if tmp > sizeof(privileged buffer)
>>        return error
>>    copy tmp bytes from comm buffer to privileged buffer
>>
>> and not look anymore at the buffer provided by the user.
>>
>> Of course, "bring all CPUs into SMM" can double as a poor man's mutex, so there
>> may be reasons to do that anyway.
>>
>> Paolo
> 
> Am thinking in BDS phase - if a module have periodic callback and uses SmmCommunicate within the callback, then it could potentially overwrite those gSmmCorePrivate pointer while another module trying to use SmmCommunicate.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>