From: Stephen Warren <swarren@wwwdotorg.org>
To: Doug Anderson <dianders@chromium.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>,
Tomasz Figa <t.figa@samsung.com>,
Vikas Sajjan <vikas.sajjan@samsung.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
Kukjin Kim <kgene.kim@samsung.com>,
sunil joshi <joshi@samsung.com>,
Vikas Sajjan <sajjan.linux@gmail.com>,
Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH] ARM: dts: Add mask-tpm-reset to the device tree
Date: Fri, 27 Jun 2014 13:56:46 -0600 [thread overview]
Message-ID: <53ADCC7E.4000703@wwwdotorg.org> (raw)
In-Reply-To: <CAD=FV=XYFy_QpBtFPc-EURXmV9qMMyOkk0Ybhoy5E6r+hXGywA@mail.gmail.com>
On 06/27/2014 12:30 PM, Doug Anderson wrote:
> Stephen,
>
> On Fri, Jun 27, 2014 at 11:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 06/27/2014 10:45 AM, Doug Anderson wrote:
>>> Stephen,
>>>
>>> On Fri, Jun 27, 2014 at 9:10 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> Surely there's a driver (or could be a driver) for the TPM chip, and
>>>> that driver should have a reset-mask-gpios property, so the driver can
>>>> call gpio_get() and gpio_set_output() on the GPIO?
>>>>
>>>> Faking this out via a not-really-a-regulator or pinctrl hogs seems like
>>>> an abuse of those features to me.
...
>> As an aside, why do we even care about this? The kernel clearly can
>> unlock the TPM simply by failing to set the GPIO up correctly, so at
>> best this is security through obscurity. If someone actively wanted to
>> do something bad to the TPM, they'd simply comment out this piece of
>> code in the kernel. At least that's true with usptream kernels which
>> aren't validated at all during boot. For a downstream signed kernel,
>> perhaps this patch makes sense (although I haven't thought about all the
>> security angles), but then it'd make sense to keep this patch downstream.
>
> Check out the bullet point about the firmware checking for kernel
> cheats. At resume time the chip actually re-loads read only firmware
> out of SPI flash (no choice about this). This read only firmware can
> check the state of the mask pin (which is preserved across sleep wake)
> to see if the kernel cheated.
Ah, that covers the security issues then.
I'd argue that the RO firmware should be setting up GPIOs like this
myself (and the pinmux, from a nice big table), and the kernel simply
not touch it all since it has no direct use for the pin. But I suppose
if the firmware is already baked and read only, that's not possible.
WARNING: multiple messages have this Message-ID (diff)
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: dts: Add mask-tpm-reset to the device tree
Date: Fri, 27 Jun 2014 13:56:46 -0600 [thread overview]
Message-ID: <53ADCC7E.4000703@wwwdotorg.org> (raw)
In-Reply-To: <CAD=FV=XYFy_QpBtFPc-EURXmV9qMMyOkk0Ybhoy5E6r+hXGywA@mail.gmail.com>
On 06/27/2014 12:30 PM, Doug Anderson wrote:
> Stephen,
>
> On Fri, Jun 27, 2014 at 11:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 06/27/2014 10:45 AM, Doug Anderson wrote:
>>> Stephen,
>>>
>>> On Fri, Jun 27, 2014 at 9:10 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> Surely there's a driver (or could be a driver) for the TPM chip, and
>>>> that driver should have a reset-mask-gpios property, so the driver can
>>>> call gpio_get() and gpio_set_output() on the GPIO?
>>>>
>>>> Faking this out via a not-really-a-regulator or pinctrl hogs seems like
>>>> an abuse of those features to me.
...
>> As an aside, why do we even care about this? The kernel clearly can
>> unlock the TPM simply by failing to set the GPIO up correctly, so at
>> best this is security through obscurity. If someone actively wanted to
>> do something bad to the TPM, they'd simply comment out this piece of
>> code in the kernel. At least that's true with usptream kernels which
>> aren't validated at all during boot. For a downstream signed kernel,
>> perhaps this patch makes sense (although I haven't thought about all the
>> security angles), but then it'd make sense to keep this patch downstream.
>
> Check out the bullet point about the firmware checking for kernel
> cheats. At resume time the chip actually re-loads read only firmware
> out of SPI flash (no choice about this). This read only firmware can
> check the state of the mask pin (which is preserved across sleep wake)
> to see if the kernel cheated.
Ah, that covers the security issues then.
I'd argue that the RO firmware should be setting up GPIOs like this
myself (and the pinmux, from a nice big table), and the kernel simply
not touch it all since it has no direct use for the pin. But I suppose
if the firmware is already baked and read only, that's not possible.
next prev parent reply other threads:[~2014-06-27 19:56 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-26 9:15 [PATCH] ARM: dts: Add mask-tpm-reset to the device tree Vikas Sajjan
2014-06-26 9:15 ` Vikas Sajjan
2014-06-26 9:52 ` Tomasz Figa
2014-06-26 9:52 ` Tomasz Figa
2014-06-26 15:25 ` Doug Anderson
2014-06-26 15:25 ` Doug Anderson
2014-06-27 12:17 ` Tomasz Figa
2014-06-27 12:17 ` Tomasz Figa
2014-06-27 15:10 ` Doug Anderson
2014-06-27 15:10 ` Doug Anderson
2014-06-27 15:14 ` Tomasz Figa
2014-06-27 15:14 ` Tomasz Figa
2014-06-27 15:22 ` Doug Anderson
2014-06-27 15:22 ` Doug Anderson
2014-06-27 15:49 ` Vikas Sajjan
2014-06-27 15:49 ` Vikas Sajjan
2014-06-27 16:10 ` Stephen Warren
2014-06-27 16:10 ` Stephen Warren
2014-06-27 16:45 ` Doug Anderson
2014-06-27 16:45 ` Doug Anderson
2014-06-27 18:20 ` Stephen Warren
2014-06-27 18:20 ` Stephen Warren
2014-06-27 18:30 ` Doug Anderson
2014-06-27 18:30 ` Doug Anderson
2014-06-27 19:56 ` Stephen Warren [this message]
2014-06-27 19:56 ` Stephen Warren
2014-06-27 19:58 ` Doug Anderson
2014-06-27 19:58 ` Doug Anderson
2014-07-08 7:46 ` Linus Walleij
2014-07-08 7:46 ` Linus Walleij
2014-07-08 15:27 ` Doug Anderson
2014-07-08 15:27 ` Doug Anderson
2014-07-08 16:20 ` Tomasz Figa
2014-07-08 16:20 ` Tomasz Figa
2014-07-09 15:22 ` Doug Anderson
2014-07-09 15:22 ` Doug Anderson
2014-07-10 4:35 ` Vikas Sajjan
2014-07-10 4:35 ` Vikas Sajjan
2014-07-10 15:25 ` Doug Anderson
2014-07-10 15:25 ` Doug Anderson
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=53ADCC7E.4000703@wwwdotorg.org \
--to=swarren@wwwdotorg.org \
--cc=dianders@chromium.org \
--cc=joshi@samsung.com \
--cc=kgene.kim@samsung.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=sajjan.linux@gmail.com \
--cc=t.figa@samsung.com \
--cc=tomasz.figa@gmail.com \
--cc=vikas.sajjan@samsung.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.