linux-kernel - Re: [PATCH 2/3] syscon: add support for "syscon-smc" compatible

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]

Date:   Fri, 23 Jul 2021 18:07:44 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Clément Léger <clement.leger@...tlin.com>
Cc:     Lee Jones <lee.jones@...aro.org>, Rob Herring <robh+dt@...nel.org>,
        Mark Brown <broonie@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        DTML <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peng Fan <peng.fan@....com>,
        Sudeep Holla <sudeep.holla@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>
Subject: Re: [PATCH 2/3] syscon: add support for "syscon-smc" compatible

On Fri, Jul 23, 2021 at 3:52 PM Clément Léger <clement.leger@...tlin.com> wrote:
>
> System controllers can be placed under secure monitor control when running
> under them. In order to keep existing code which accesses such system
> controllers using a syscon, add support for "syscon-smc" compatible.
>
> When enable, the syscon will handle this new compatible and look for an
> "arm,smc-id" property to execute the appropriate SMC. A SMC regmap is then
> created to forward register access to the secure monitor.
>
> Signed-off-by: Clément Léger <clement.leger@...tlin.com>

I don't see anything wrong with the implementation, but this worries
me conceptually, because of the ways this might get abused:

- this creates one more way to keep device drivers hidden away
  behind firmware when they should be in the kernel. You can already
  do that with separate SMC calls, but adding an indirection makes it
  sneakier. If the 'registers' in here are purely

- This may be seen as an easy way out for firmware writers that just
   expose a bare register-level interface when the correct solution would
   be to create a high-level interface.

There is also a problem with locking: In the case that both firmware and
kernel have to access registers within a syscon area, you may need to
have a semaphore to protect an atomic sequence of accesses, but since
the interface only provides a single register load/store, there is no way for
a kernel driver to serialize against a firmware-internal driver.

        Arnd