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
| ||
|
Date: Mon, 15 Aug 2022 14:53:07 -0600 From: Jack Rosenthal <jrosenth@...omium.org> To: Stephen Boyd <swboyd@...omium.org> Cc: chrome-platform@...ts.linux.dev, 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 Thanks for the detailed review! On 2022-08-09 at 20:49 -0500, Stephen Boyd wrote: > /sys/firmware/coreboot/cbmem? Fixed in v9. > > +#include <linux/ctype.h> > > What is used from this header? Not used, fixed in v9. > 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. > > + return cbmem_entry_setup(entry); > > Is there a reason this isn't inlined here? cbmem_entry_setup was already a little long, so I just did this for reading clarity. If you think it would be clearer just inlined here, I can change it (let me know). > Need to set .owner = THIS_MODULE unless you use > module_coreboot_driver(). Done. > > + if (!coreboot_kobj) > > cbmem_kobj? Done.
Powered by blists - more mailing lists