linux-kernel - Re: [PATCH v4 1/2] x86: Add basic support for the Congatec CGEB BIOS interface

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: <Zr329S8995L0OsCu@freedom.csh.rit.edu>
Date: Thu, 15 Aug 2024 08:39:17 -0400
From: Mary Strodl <mstrodl@...edom.csh.rit.edu>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Mary Strodl <mstrodl@....rit.edu>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, urezki@...il.com, hch@...radead.org,
	linux-mm@...ck.org, lee@...nel.org, andi.shyti@...nel.org,
	linux-i2c@...r.kernel.org, s.hauer@...gutronix.de,
	christian.gmeiner@...il.com
Subject: Re: [PATCH v4 1/2] x86: Add basic support for the Congatec CGEB BIOS
 interface

On Thu, Aug 15, 2024 at 02:17:12PM +0200, Thomas Gleixner wrote:
> So this caught the attention of my mail filter because the subsystem
> prefix is 'x86:' while it obviously should be 'mfd:'

Sorry! Originally there was some debate as to where it belonged and the
commit message was never updated.

> This Signed-off-by chain is invalid. See Documentation/process/

My bad. I was following the example set by last time this was brought
up for review by Christian. Would you like only my Signed-off-by? Or
something else?

> Aisde of that. How is any of this supposed to work. What is the user
> mode helper doing and why does this need this netlink indirection?

The first time I posted the series I was asked to move the BIOS stuff
into userspace. Personally, I'm not convined that makes it any less
terrible, but I obliged.

> Seriously? RWX mappings?

Yeah. In my testing, it wouldn't work without writable mappings.

> So this copies a code blob out of the BIOS region into user space and
> lets user space use this blob to interact with the BIOS extension,
> right?

Yep.

> So an i2c transfer does a full round trip:
> 
>    i2c::xfer -> cgeb -> netlink -> user helper -> execute random code ->
>    user helper -> netlink -> cgeb -> ...

Looks right. I'm not thrilled about it either, but the vmalloc people
wanted it in userspace. For our usecase, we didn't intend to have
much throughput so it still worked okay for us.

> Has anyone tried to analyze what this BIOS provided code blob is
> actually doing?

I have no idea what the licensing on the BIOS blob is, I didn't want to
run afoul of any copyright stuff by disassembling it to find out what
it does. I suppose I could pull the blob off my hardware and watch what
it does in qemu?

> All it does is to poke at a range of IOPORTS with in*(), out*() and that
> poking mechanism depends on the generation of that CGEB implementation.
> 
> Congatec even provides the GPL2 licenced source for this pokery as a
> kernel driver.

The only driver I've found from them is the one that runs blobs in the BIOS.
Where did you find the IOPORT stuff?

> Thanks,
> 
>         tglx

Thanks for taking time to read through this!