kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Kees Cook <keescook@chromium.org>
Cc: alex.williamson@redhat.com, jmorris@namei.org,
	dhowells@redhat.com, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org, kvm@vger.kernel.org,
	mjg59@srcf.ucam.org, cohuck@redhat.com
Subject: Re: [PATCH] vfio: Lock down no-IOMMU mode when kernel is locked down
Date: Thu, 20 May 2021 10:38:52 +0200	[thread overview]
Message-ID: <d9138fab-4420-8c80-047d-b83c04eeed8e@redhat.com> (raw)
In-Reply-To: <202105101955.933F66A@keescook>



On 5/11/21 4:58 AM, Kees Cook wrote:
> On Thu, May 06, 2021 at 11:18:59AM +0200, Maxime Coquelin wrote:
>> When no-IOMMU mode is enabled, VFIO is as unsafe as accessing
>> the PCI BARs via the device's sysfs, which is locked down when
>> the kernel is locked down.
>>
>> Indeed, it is possible for an attacker to craft DMA requests
>> to modify kernel's code or leak secrets stored in the kernel,
>> since the device is not isolated by an IOMMU.
>>
>> This patch introduces a new integrity lockdown reason for the
>> unsafe VFIO no-iommu mode.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  drivers/vfio/vfio.c      | 13 +++++++++----
>>  include/linux/security.h |  1 +
>>  security/security.c      |  1 +
>>  3 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>> index 5e631c359ef2..fe466d6ea5d8 100644
>> --- a/drivers/vfio/vfio.c
>> +++ b/drivers/vfio/vfio.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/pci.h>
>>  #include <linux/rwsem.h>
>>  #include <linux/sched.h>
>> +#include <linux/security.h>
>>  #include <linux/slab.h>
>>  #include <linux/stat.h>
>>  #include <linux/string.h>
>> @@ -165,7 +166,8 @@ static void *vfio_noiommu_open(unsigned long arg)
>>  {
>>  	if (arg != VFIO_NOIOMMU_IOMMU)
>>  		return ERR_PTR(-EINVAL);
>> -	if (!capable(CAP_SYS_RAWIO))
>> +	if (!capable(CAP_SYS_RAWIO) ||
>> +			security_locked_down(LOCKDOWN_VFIO_NOIOMMU))
> 
> The LSM hook check should come before the capable() check to avoid
> setting PF_SUPERPRIV if capable() passes and the LSM doesn't.

OK, good to know, I'll swap in next revision.

BTW, it seems other places are doing the capable check before the LSM
hook check, for example in ioport [0].

>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 06f7c50ce77f..f29388180fab 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -120,6 +120,7 @@ enum lockdown_reason {
>>  	LOCKDOWN_MMIOTRACE,
>>  	LOCKDOWN_DEBUGFS,
>>  	LOCKDOWN_XMON_WR,
>> +	LOCKDOWN_VFIO_NOIOMMU,
>>  	LOCKDOWN_INTEGRITY_MAX,
>>  	LOCKDOWN_KCORE,
>>  	LOCKDOWN_KPROBES,
> 
> Is the security threat specific to VFIO? (i.e. could other interfaces
> want a similar thing, such that naming this VFIO doesn't make sense?

It could possibly in theory, maybe something like
"LOCKDOWN_UNRESTRICTED_DMA" would be a better fit?

Maxime

[0]:
https://elixir.bootlin.com/linux/v5.13-rc2/source/arch/x86/kernel/ioport.c#L73


      reply	other threads:[~2021-05-20  8:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-06  9:18 Maxime Coquelin
2021-05-06 21:50 ` Alex Williamson
2021-05-07  8:37   ` Ondrej Mosnacek
2021-05-07  9:11     ` Maxime Coquelin
2021-05-07 12:31       ` Ondrej Mosnacek
2021-05-11  2:58 ` Kees Cook
2021-05-20  8:38   ` Maxime Coquelin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d9138fab-4420-8c80-047d-b83c04eeed8e@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mjg59@srcf.ucam.org \
    --subject='Re: [PATCH] vfio: Lock down no-IOMMU mode when kernel is locked down' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).