netdev - Re: [PATCH V3 0/3] Add OP-TEE based bnxt f/w manager

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <183f68bc-8145-ef98-07ca-8d3f85d66a17@gmail.com>
Date:   Thu, 24 Oct 2019 11:39:42 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Sheetal Tigadoli <sheetal.tigadoli@...adcom.com>,
        Rafał Miłecki <zajec5@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Michal Simek <michal.simek@...inx.com>,
        Rajan Vaja <rajan.vaja@...inx.com>,
        Scott Branden <scott.branden@...adcom.com>,
        Ray Jui <ray.jui@...adcom.com>,
        Vikram Prakash <vikram.prakash@...adcom.com>,
        Jens Wiklander <jens.wiklander@...aro.org>,
        Michael Chan <michael.chan@...adcom.com>,
        "David S. Miller" <davem@...emloft.net>,
        Vikas Gupta <vikas.gupta@...adcom.com>,
        Vasundhara Volam <vasundhara-v.volam@...adcom.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        tee-dev@...ts.linaro.org, bcm-kernel-feedback-list@...adcom.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH V3 0/3] Add OP-TEE based bnxt f/w manager

On 10/23/19 10:32 PM, Sheetal Tigadoli wrote:
> This patch series adds support for TEE based BNXT firmware
> management module and the driver changes to invoke OP-TEE
> APIs to fastboot firmware and to collect crash dump.

Sorry for chiming on this so late, the more I look into this and the
more it seems like you have built a custom TEE firmware loading solution
rather than thinking about extending the firmware API to load a firmware
opaque handle from somewhere other than the filesystem.

The TEE integration appears okay to me in that you leverage the TEE bus
to advertise your driver. What seems to violating layers is that you
have bnxt directly tap into your TEE driver's services and that looks
not ideal to say the least. That approach does not scale well over
multiple drivers (bnxt or otherwise), but also does not really scale
over trusted components providers. TEE is one of them, but conceptually
the same thing could exist with ACPI/UEFI or any platform that has
services that offer some sort of secure/non-secured world differentiation.

The way I would imagine you to integrate this is to basically register a
TEE firmware provider through the firmware API, continue using the
firmware API from within bnxt, possibly with using a specific file
handle/flag that designates whether you want to favor loading from
disk/file system or TEE. It should not matter to bnxt how the firmware
is obtained basically.

> 
> changes from v2:
>  - address review comments from Jakub
> 
> Vasundhara Volam (2):
>   bnxt_en: Add support to invoke OP-TEE API to reset firmware
>   bnxt_en: Add support to collect crash dump via ethtool
> 
> Vikas Gupta (1):
>   firmware: broadcom: add OP-TEE based BNXT f/w manager
> 
>  drivers/firmware/broadcom/Kconfig                 |   8 +
>  drivers/firmware/broadcom/Makefile                |   1 +
>  drivers/firmware/broadcom/tee_bnxt_fw.c           | 277 ++++++++++++++++++++++
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c         |  13 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h         |   6 +
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  37 ++-
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h |   2 +
>  include/linux/firmware/broadcom/tee_bnxt_fw.h     |  14 ++
>  8 files changed, 354 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/firmware/broadcom/tee_bnxt_fw.c
>  create mode 100644 include/linux/firmware/broadcom/tee_bnxt_fw.h
> 

-- 
Florian