From: Mimi Zohar <zohar@linux.ibm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Kees Cook <keescook@chromium.org>,
Stephen Boyd <sboyd@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Luis R . Rodriguez" <mcgrof@suse.com>,
Kexec Mailing List <kexec@lists.infradead.org>,
linux-security-module <linux-security-module@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
David Howells <dhowells@redhat.com>,
"Luis R . Rodriguez" <mcgrof@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Eric Biederman <ebiederm@xmission.com>,
linux-integrity <linux-integrity@vger.kernel.org>,
"Serge E . Hallyn" <serge@hallyn.com>,
Andres Rodriguez <andresx7@gmail.com>
Subject: Re: [PATCH v5 7/8] ima: based on policy warn about loading firmware (pre-allocated buffer)
Date: Mon, 09 Jul 2018 15:41:34 -0400 [thread overview]
Message-ID: <1531165294.3332.40.camel@linux.ibm.com> (raw)
In-Reply-To: <CAKv+Gu-nky1yX59TL2FNQ-0q7Sy399t2WpVWSy7jqYpQY4Fxjw@mail.gmail.com>
On Mon, 2018-07-02 at 17:30 +0200, Ard Biesheuvel wrote:
> On 2 July 2018 at 16:38, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Some systems are memory constrained but they need to load very large
> > firmwares. The firmware subsystem allows drivers to request this
> > firmware be loaded from the filesystem, but this requires that the
> > entire firmware be loaded into kernel memory first before it's provided
> > to the driver. This can lead to a situation where we map the firmware
> > twice, once to load the firmware into kernel memory and once to copy the
> > firmware into the final resting place.
> >
> > To resolve this problem, commit a098ecd2fa7d ("firmware: support loading
> > into a pre-allocated buffer") introduced request_firmware_into_buf() API
> > that allows drivers to request firmware be loaded directly into a
> > pre-allocated buffer. (Based on the mailing list discussions, calling
> > dma_alloc_coherent() is unnecessary and confusing.)
> >
> > (Very broken/buggy) devices using pre-allocated memory run the risk of
> > the firmware being accessible to the device prior to the completion of
> > IMA's signature verification. For the time being, this patch emits a
> > warning, but does not prevent the loading of the firmware.
> >
>
> As I attempted to explain in the exchange with Luis, this has nothing
> to do with broken or buggy devices, but is simply the reality we have
> to deal with on platforms that lack IOMMUs.
> Even if you load into one buffer, carry out the signature verification
> and *only then* copy it to another buffer, a bus master could
> potentially read it from the first buffer as well. Mapping for DMA
> does *not* mean 'making the memory readable by the device' unless
> IOMMUs are being used. Otherwise, a bus master can read it from the
> first buffer, or even patch the code that performs the security check
> in the first place. For such platforms, copying the data around to
> prevent the device from reading it is simply pointless, as well as any
> other mitigation in software to protect yourself from misbehaving bus
> masters.
Thank you for taking the time to explain this again.
> So issuing a warning in this particular case is rather arbitrary. On
> these platforms, all bus masters can read (and modify) all of your
> memory all of the time, and as long as the firmware loader code takes
> care not to provide the DMA address to the device until after the
> verification is complete, it really has done all it reasonably can in
> the environment that it is expected to operate in.
So for the non-IOMMU system case, differentiating between pre-
allocated buffers vs. using two buffers doesn't make sense.
>
> (The use of dma_alloc_coherent() is a bit of a red herring here, as it
> incorporates the DMA map operation. However, DMA map is a no-op on
> systems with cache coherent 1:1 DMA [iow, all PCs and most arm64
> platforms unless they have IOMMUs], and so there is not much
> difference between memory allocated with kmalloc() or with
> dma_alloc_coherent() in terms of whether the device can access it
> freely)
What about systems with an IOMMU?
Mimi
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: Mimi Zohar <zohar@linux.ibm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: linux-integrity <linux-integrity@vger.kernel.org>,
linux-security-module <linux-security-module@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
David Howells <dhowells@redhat.com>,
"Luis R . Rodriguez" <mcgrof@kernel.org>,
Eric Biederman <ebiederm@xmission.com>,
Kexec Mailing List <kexec@lists.infradead.org>,
Andres Rodriguez <andresx7@gmail.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Luis R . Rodriguez" <mcgrof@suse.com>,
Kees Cook <keescook@chromium.org>,
"Serge E . Hallyn" <serge@hallyn.com>,
Stephen Boyd <sboyd@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>
Subject: Re: [PATCH v5 7/8] ima: based on policy warn about loading firmware (pre-allocated buffer)
Date: Mon, 09 Jul 2018 15:41:34 -0400 [thread overview]
Message-ID: <1531165294.3332.40.camel@linux.ibm.com> (raw)
In-Reply-To: <CAKv+Gu-nky1yX59TL2FNQ-0q7Sy399t2WpVWSy7jqYpQY4Fxjw@mail.gmail.com>
On Mon, 2018-07-02 at 17:30 +0200, Ard Biesheuvel wrote:
> On 2 July 2018 at 16:38, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Some systems are memory constrained but they need to load very large
> > firmwares. The firmware subsystem allows drivers to request this
> > firmware be loaded from the filesystem, but this requires that the
> > entire firmware be loaded into kernel memory first before it's provided
> > to the driver. This can lead to a situation where we map the firmware
> > twice, once to load the firmware into kernel memory and once to copy the
> > firmware into the final resting place.
> >
> > To resolve this problem, commit a098ecd2fa7d ("firmware: support loading
> > into a pre-allocated buffer") introduced request_firmware_into_buf() API
> > that allows drivers to request firmware be loaded directly into a
> > pre-allocated buffer. (Based on the mailing list discussions, calling
> > dma_alloc_coherent() is unnecessary and confusing.)
> >
> > (Very broken/buggy) devices using pre-allocated memory run the risk of
> > the firmware being accessible to the device prior to the completion of
> > IMA's signature verification. For the time being, this patch emits a
> > warning, but does not prevent the loading of the firmware.
> >
>
> As I attempted to explain in the exchange with Luis, this has nothing
> to do with broken or buggy devices, but is simply the reality we have
> to deal with on platforms that lack IOMMUs.
> Even if you load into one buffer, carry out the signature verification
> and *only then* copy it to another buffer, a bus master could
> potentially read it from the first buffer as well. Mapping for DMA
> does *not* mean 'making the memory readable by the device' unless
> IOMMUs are being used. Otherwise, a bus master can read it from the
> first buffer, or even patch the code that performs the security check
> in the first place. For such platforms, copying the data around to
> prevent the device from reading it is simply pointless, as well as any
> other mitigation in software to protect yourself from misbehaving bus
> masters.
Thank you for taking the time to explain this again.
> So issuing a warning in this particular case is rather arbitrary. On
> these platforms, all bus masters can read (and modify) all of your
> memory all of the time, and as long as the firmware loader code takes
> care not to provide the DMA address to the device until after the
> verification is complete, it really has done all it reasonably can in
> the environment that it is expected to operate in.
So for the non-IOMMU system case, differentiating between pre-
allocated buffers vs. using two buffers doesn't make sense.
>
> (The use of dma_alloc_coherent() is a bit of a red herring here, as it
> incorporates the DMA map operation. However, DMA map is a no-op on
> systems with cache coherent 1:1 DMA [iow, all PCs and most arm64
> platforms unless they have IOMMUs], and so there is not much
> difference between memory allocated with kmalloc() or with
> dma_alloc_coherent() in terms of whether the device can access it
> freely)
What about systems with an IOMMU?
Mimi
WARNING: multiple messages have this Message-ID (diff)
From: zohar@linux.ibm.com (Mimi Zohar)
To: linux-security-module@vger.kernel.org
Subject: [PATCH v5 7/8] ima: based on policy warn about loading firmware (pre-allocated buffer)
Date: Mon, 09 Jul 2018 15:41:34 -0400 [thread overview]
Message-ID: <1531165294.3332.40.camel@linux.ibm.com> (raw)
In-Reply-To: <CAKv+Gu-nky1yX59TL2FNQ-0q7Sy399t2WpVWSy7jqYpQY4Fxjw@mail.gmail.com>
On Mon, 2018-07-02 at 17:30 +0200, Ard Biesheuvel wrote:
> On 2 July 2018 at 16:38, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Some systems are memory constrained but they need to load very large
> > firmwares. The firmware subsystem allows drivers to request this
> > firmware be loaded from the filesystem, but this requires that the
> > entire firmware be loaded into kernel memory first before it's provided
> > to the driver. This can lead to a situation where we map the firmware
> > twice, once to load the firmware into kernel memory and once to copy the
> > firmware into the final resting place.
> >
> > To resolve this problem, commit a098ecd2fa7d ("firmware: support loading
> > into a pre-allocated buffer") introduced request_firmware_into_buf() API
> > that allows drivers to request firmware be loaded directly into a
> > pre-allocated buffer. (Based on the mailing list discussions, calling
> > dma_alloc_coherent() is unnecessary and confusing.)
> >
> > (Very broken/buggy) devices using pre-allocated memory run the risk of
> > the firmware being accessible to the device prior to the completion of
> > IMA's signature verification. For the time being, this patch emits a
> > warning, but does not prevent the loading of the firmware.
> >
>
> As I attempted to explain in the exchange with Luis, this has nothing
> to do with broken or buggy devices, but is simply the reality we have
> to deal with on platforms that lack IOMMUs.
> Even if you load into one buffer, carry out the signature verification
> and *only then* copy it to another buffer, a bus master could
> potentially read it from the first buffer as well. Mapping for DMA
> does *not* mean 'making the memory readable by the device' unless
> IOMMUs are being used. Otherwise, a bus master can read it from the
> first buffer, or even patch the code that performs the security check
> in the first place. For such platforms, copying the data around to
> prevent the device from reading it is simply pointless, as well as any
> other mitigation in software to protect yourself from misbehaving bus
> masters.
Thank you for taking the time to explain this again.
> So issuing a warning in this particular case is rather arbitrary. On
> these platforms, all bus masters can read (and modify) all of your
> memory all of the time, and as long as the firmware loader code takes
> care not to provide the DMA address to the device until after the
> verification is complete, it really has done all it reasonably can in
> the environment that it is expected to operate in.
So for the non-IOMMU system case, differentiating between pre-
allocated buffers vs. using two buffers doesn't make sense.
>
> (The use of dma_alloc_coherent() is a bit of a red herring here, as it
> incorporates the DMA map operation. However, DMA map is a no-op on
> systems with cache coherent 1:1 DMA [iow, all PCs and most arm64
> platforms unless they have IOMMUs], and so there is not much
> difference between memory allocated with kmalloc() or with
> dma_alloc_coherent() in terms of whether the device can access it
> freely)
?
What about systems with an IOMMU?
Mimi
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Mimi Zohar <zohar@linux.ibm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: linux-integrity <linux-integrity@vger.kernel.org>,
linux-security-module <linux-security-module@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
David Howells <dhowells@redhat.com>,
"Luis R . Rodriguez" <mcgrof@kernel.org>,
Eric Biederman <ebiederm@xmission.com>,
Kexec Mailing List <kexec@lists.infradead.org>,
Andres Rodriguez <andresx7@gmail.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Luis R . Rodriguez" <mcgrof@suse.com>,
Kees Cook <keescook@chromium.org>,
"Serge E . Hallyn" <serge@hallyn.com>,
Stephen Boyd <sboyd@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>
Subject: Re: [PATCH v5 7/8] ima: based on policy warn about loading firmware (pre-allocated buffer)
Date: Mon, 09 Jul 2018 15:41:34 -0400 [thread overview]
Message-ID: <1531165294.3332.40.camel@linux.ibm.com> (raw)
In-Reply-To: <CAKv+Gu-nky1yX59TL2FNQ-0q7Sy399t2WpVWSy7jqYpQY4Fxjw@mail.gmail.com>
On Mon, 2018-07-02 at 17:30 +0200, Ard Biesheuvel wrote:
> On 2 July 2018 at 16:38, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Some systems are memory constrained but they need to load very large
> > firmwares. The firmware subsystem allows drivers to request this
> > firmware be loaded from the filesystem, but this requires that the
> > entire firmware be loaded into kernel memory first before it's provided
> > to the driver. This can lead to a situation where we map the firmware
> > twice, once to load the firmware into kernel memory and once to copy the
> > firmware into the final resting place.
> >
> > To resolve this problem, commit a098ecd2fa7d ("firmware: support loading
> > into a pre-allocated buffer") introduced request_firmware_into_buf() API
> > that allows drivers to request firmware be loaded directly into a
> > pre-allocated buffer. (Based on the mailing list discussions, calling
> > dma_alloc_coherent() is unnecessary and confusing.)
> >
> > (Very broken/buggy) devices using pre-allocated memory run the risk of
> > the firmware being accessible to the device prior to the completion of
> > IMA's signature verification. For the time being, this patch emits a
> > warning, but does not prevent the loading of the firmware.
> >
>
> As I attempted to explain in the exchange with Luis, this has nothing
> to do with broken or buggy devices, but is simply the reality we have
> to deal with on platforms that lack IOMMUs.
> Even if you load into one buffer, carry out the signature verification
> and *only then* copy it to another buffer, a bus master could
> potentially read it from the first buffer as well. Mapping for DMA
> does *not* mean 'making the memory readable by the device' unless
> IOMMUs are being used. Otherwise, a bus master can read it from the
> first buffer, or even patch the code that performs the security check
> in the first place. For such platforms, copying the data around to
> prevent the device from reading it is simply pointless, as well as any
> other mitigation in software to protect yourself from misbehaving bus
> masters.
Thank you for taking the time to explain this again.
> So issuing a warning in this particular case is rather arbitrary. On
> these platforms, all bus masters can read (and modify) all of your
> memory all of the time, and as long as the firmware loader code takes
> care not to provide the DMA address to the device until after the
> verification is complete, it really has done all it reasonably can in
> the environment that it is expected to operate in.
So for the non-IOMMU system case, differentiating between pre-
allocated buffers vs. using two buffers doesn't make sense.
>
> (The use of dma_alloc_coherent() is a bit of a red herring here, as it
> incorporates the DMA map operation. However, DMA map is a no-op on
> systems with cache coherent 1:1 DMA [iow, all PCs and most arm64
> platforms unless they have IOMMUs], and so there is not much
> difference between memory allocated with kmalloc() or with
> dma_alloc_coherent() in terms of whether the device can access it
> freely)
What about systems with an IOMMU?
Mimi
next prev parent reply other threads:[~2018-07-09 19:41 UTC|newest]
Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-02 14:37 [PATCH v5 0/8] kexec/firmware: support system wide policy requiring signatures Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-02 14:37 ` [PATCH v5 1/8] security: define new LSM hook named security_kernel_load_data Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-02 18:45 ` J Freyensee
2018-07-02 18:45 ` J Freyensee
2018-07-02 18:45 ` J Freyensee
2018-07-02 18:45 ` J Freyensee
2018-07-03 12:35 ` Mimi Zohar
2018-07-03 12:35 ` Mimi Zohar
2018-07-03 12:35 ` Mimi Zohar
2018-07-03 12:35 ` Mimi Zohar
2018-07-02 14:37 ` [PATCH v5 2/8] kexec: add call to LSM hook in original kexec_load syscall Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-10 20:26 ` Mimi Zohar
2018-07-10 20:26 ` Mimi Zohar
2018-07-10 20:26 ` Mimi Zohar
2018-07-02 14:37 ` [PATCH v5 3/8] ima: based on policy require signed kexec kernel images Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-02 18:31 ` J Freyensee
2018-07-02 18:31 ` J Freyensee
2018-07-02 18:31 ` J Freyensee
2018-07-02 18:31 ` J Freyensee
2018-07-03 13:07 ` Mimi Zohar
2018-07-03 13:07 ` Mimi Zohar
2018-07-03 13:07 ` Mimi Zohar
2018-07-03 13:07 ` Mimi Zohar
2018-07-02 14:37 ` [PATCH v5 4/8] firmware: add call to LSM hook before firmware sysfs fallback Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-02 14:37 ` Mimi Zohar
2018-07-03 12:04 ` kbuild test robot
2018-07-03 12:04 ` kbuild test robot
2018-07-03 12:04 ` kbuild test robot
2018-07-03 12:04 ` kbuild test robot
2018-07-02 14:38 ` [PATCH v5 5/8] ima: based on policy require signed firmware (sysfs fallback) Mimi Zohar
2018-07-02 14:38 ` Mimi Zohar
2018-07-02 14:38 ` Mimi Zohar
2018-07-02 14:38 ` [PATCH v5 6/8] ima: add build time policy Mimi Zohar
2018-07-02 14:38 ` Mimi Zohar
2018-07-02 14:38 ` Mimi Zohar
2018-07-02 14:38 ` [PATCH v5 7/8] ima: based on policy warn about loading firmware (pre-allocated buffer) Mimi Zohar
2018-07-02 14:38 ` Mimi Zohar
2018-07-02 14:38 ` Mimi Zohar
2018-07-02 15:30 ` Ard Biesheuvel
2018-07-02 15:30 ` Ard Biesheuvel
2018-07-02 15:30 ` Ard Biesheuvel
2018-07-09 19:41 ` Mimi Zohar [this message]
2018-07-09 19:41 ` Mimi Zohar
2018-07-09 19:41 ` Mimi Zohar
2018-07-09 19:41 ` Mimi Zohar
2018-07-10 6:51 ` Ard Biesheuvel
2018-07-10 6:51 ` Ard Biesheuvel
2018-07-10 6:51 ` Ard Biesheuvel
2018-07-10 6:56 ` Ard Biesheuvel
2018-07-10 6:56 ` Ard Biesheuvel
2018-07-10 6:56 ` Ard Biesheuvel
2018-07-10 18:47 ` Mimi Zohar
2018-07-10 18:47 ` Mimi Zohar
2018-07-10 18:47 ` Mimi Zohar
2018-07-10 18:47 ` Mimi Zohar
2018-07-10 19:19 ` Bjorn Andersson
2018-07-10 19:19 ` Bjorn Andersson
2018-07-10 19:19 ` Bjorn Andersson
2018-07-11 6:24 ` Ard Biesheuvel
2018-07-11 6:24 ` Ard Biesheuvel
2018-07-11 6:24 ` Ard Biesheuvel
2018-07-12 20:03 ` Mimi Zohar
2018-07-12 20:03 ` Mimi Zohar
2018-07-12 20:03 ` Mimi Zohar
2018-07-12 20:03 ` Mimi Zohar
2018-07-12 20:37 ` Bjorn Andersson
2018-07-12 20:37 ` Bjorn Andersson
2018-07-12 20:37 ` Bjorn Andersson
2018-07-12 20:37 ` Bjorn Andersson
2018-07-02 14:38 ` [PATCH v5 8/8] module: replace the existing LSM hook in init_module Mimi Zohar
2018-07-02 14:38 ` Mimi Zohar
2018-07-02 14:38 ` Mimi Zohar
2018-07-03 9:35 ` kbuild test robot
2018-07-03 9:35 ` kbuild test robot
2018-07-03 9:35 ` kbuild test robot
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=1531165294.3332.40.camel@linux.ibm.com \
--to=zohar@linux.ibm.com \
--cc=andresx7@gmail.com \
--cc=ard.biesheuvel@linaro.org \
--cc=bjorn.andersson@linaro.org \
--cc=dhowells@redhat.com \
--cc=ebiederm@xmission.com \
--cc=gregkh@linuxfoundation.org \
--cc=keescook@chromium.org \
--cc=kexec@lists.infradead.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=mcgrof@suse.com \
--cc=sboyd@kernel.org \
--cc=serge@hallyn.com \
--cc=zohar@linux.vnet.ibm.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.