linux-kernel - Re: [PATCH v8] firmware: google: Implement cbmem in sysfs driver

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] [day] [month] [year] [list]

Message-ID: <CAODwPW-B80WBvhOvQ96PGwoCSa=fpnYbHv+DM3EMEzTr+1v1sg@mail.gmail.com>
Date:   Mon, 15 Aug 2022 15:03:58 -0700
From:   Julius Werner <jwerner@...omium.org>
To:     Jack Rosenthal <jrosenth@...omium.org>
Cc:     Stephen Boyd <swboyd@...omium.org>,
        chrome-platform@...ts.linux.dev,
        LKML <linux-kernel@...r.kernel.org>,
        Tzung-Bi Shih <tzungbi@...nel.org>,
        Guenter Roeck <groeck@...omium.org>,
        Julius Werner <jwerner@...omium.org>
Subject: Re: [PATCH v8] firmware: google: Implement cbmem in sysfs driver

> > Are all entries supposed to be writeable? Are there entries that are
> > read-only?
>
> The idea here was that we could use crossystem to update the cbmem entry
> when it makes changes, however, I do see this as an odd usage of the
> ABI, and in v9, I removed writing entirely and all entries are now
> read-only.
>
> If tools like crossystem need to maintain changes to this buffer, they
> should either maintain that copy themselves in /run or something, or
> daemonize and keep it in their process' memory.

Sorry, but I really don't like that approach, I feel like this is
swinging too far in the other direction for no good reason. There
should only be one source of truth for these things, not every process
for themselves.

If people are worried about allowing (access-controlled) writes to all
CBMEM buffers, we could restrict this to just a hardcoded allowlist of
CBMEM IDs. It would only need to be the vboot workbuffer for now, and
if we ever find another use case it would be easy to add (unlikely
that this would happen often). Or if you really don't want this at all
then ditch the `mem` node completely and just have crossystem use
/dev/mem on that address and size (although this would run into the
memory type uncertainties again that Stephen mentioned). But we
shouldn't keep a dozen redundant caches of the same thing.