From: Julius Werner <firstname.lastname@example.org> To: Julius Werner <email@example.com> Cc: firstname.lastname@example.org, Greg Kroah-Hartman <email@example.com>, LKML <firstname.lastname@example.org>, Wei-Ning Huang <email@example.com>, Brian Norris <firstname.lastname@example.org>, email@example.com Subject: Re: [PATCH v3 7/7] firmware: coreboot: Request table region for exclusive access Date: Thu, 9 Aug 2018 16:44:43 -0700 Message-ID: <CAODwPW8+QSittJdNLWpWKU=JNJa5bssMHQ4ER4CfnUAWaL1JGA@mail.gmail.com> (raw) In-Reply-To: <CAODwPW84FYK3k5=n4gQ4EPJExhQ59LCEi31ZYi2Bq2y9FH_E_A@mail.gmail.com> Actually, looking at what IO_STRICT_DEVMEM really does, would it really prevent userspace accesses to these areas? Because it seems that it only prevents accesses to areas marked as IORESOURCE_BUSY, and while I can't fully follow how the kernel assigns that, comments suggest that this is only set when "Driver has marked this resource busy". So after you make the change to the other patch where we immediately unmap the coreboot table again at the end of the probe() function, shouldn't it become available to userspace again even with IO_STRICT_DEVMEM set? On Thu, Aug 9, 2018 at 4:37 PM Julius Werner <firstname.lastname@example.org> wrote: > > > Furthermore, I see that my system RAM excludes this coreboot table so it > > doesn't fall into the bucket that CONFIG_STRICT_DEVMEM would find. > > Yes, that is intentional. We don't want the kernel to try to use that > memory for anything else (since we want those tables to survive), so > we mark them as reserved in the e820 map. > > > > (I guess an alternative would be to rewrite 'cbmem' to use > > > /sys/bus/coreboot/devices if available to get its coreboot table > > > information. But we'd still need to maintain the old path for > > > backwards compatibility anyway, so that would really just make it more > > > complicated.) > > > > This sounds like a good idea. Userspace reaching into /dev/mem is not > > good from a kernel hardening perspective. That's why those strict devmem > > configs exist. Can cbmem be updated to query information from device > > drivers instead, so that we can enable CONFIG_IO_STRICT_DEVMEM as well? > > Well... problem is that cbmem doesn't just access the coreboot tables, > it accesses more stuff. There is actually a larger memory region > called CBMEM (that's what the utility is named after) which contains > all sorts of random memory allocations that coreboot wanted to survive > for the lifetime of the system. The coreboot table is one section in > there, and it sort of serves as a directory for some of the others > (although there's also just a general CBMEM directory... there's some > redundancy there). But cbmem can also print some of the other CBMEM > sections which it finds by querying the coreboot table, such as the > firmware log or the boot timestamps. > > So the question is how we can get to that content if /dev/mem isn't > available anymore. One option would be to just write separate kernel > drivers to completely replace the cbmem utility (we already have one > for the log, for example), but I think Linux generally doesn't want to > have too much logic and parsing and stuff in kernel drivers. Another > option is to add a driver that just exposes a sysfs file through which > you could read (we don't need to write) the CBMEM area... but then > we'd essentially want that to take absolute addresses because that's > what the coreboot table pointers contain, so we would've just built > /dev/mem by another name (for a restricted range). > > The nicest thing, really, would be if there was a way for a kernel > driver to mark specific regions as "allowed" by /dev/mem. I don't > suppose we'd be willing to introduce a mechanism like that?
next prev parent reply index Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-09 17:17 [PATCH v3 0/7] firmware: coreboot: Fix probe and simplify code Stephen Boyd 2018-08-09 17:17 ` [PATCH v3 1/7] firmware: coreboot: Let OF core populate platform device Stephen Boyd 2018-08-09 17:31 ` Brian Norris 2018-08-09 17:17 ` [PATCH v3 2/7] firmware: coreboot: Unmap ioregion on failure Stephen Boyd 2018-08-09 17:49 ` Brian Norris 2018-08-09 19:40 ` Stephen Boyd 2018-08-09 19:52 ` Brian Norris 2018-08-09 23:25 ` Stephen Boyd 2018-08-09 17:17 ` [PATCH v3 3/7] firmware: coreboot: Make bus registration symmetric Stephen Boyd 2018-08-09 18:10 ` Julius Werner 2018-08-09 23:30 ` Stephen Boyd 2018-08-09 17:17 ` [PATCH v3 4/7] firmware: coreboot: Collapse platform drivers into bus core Stephen Boyd 2018-08-09 17:17 ` [PATCH v3 5/7] firmware: coreboot: Remap RAM with memremap() instead of ioremap() Stephen Boyd 2018-08-09 18:24 ` Julius Werner 2018-08-09 22:07 ` Stephen Boyd 2018-08-09 17:17 ` [PATCH v3 6/7] firmware: coreboot: Only populate devices in coreboot_table_init() Stephen Boyd 2018-08-09 21:02 ` Julius Werner 2018-08-09 23:43 ` Stephen Boyd 2018-08-09 17:17 ` [PATCH v3 7/7] firmware: coreboot: Request table region for exclusive access Stephen Boyd 2018-08-09 21:07 ` Julius Werner 2018-08-09 23:03 ` Stephen Boyd 2018-08-09 23:37 ` Julius Werner 2018-08-09 23:44 ` Julius Werner [this message] 2018-08-10 2:54 ` Stephen Boyd 2018-08-10 23:24 ` Stephen Boyd 2018-08-09 18:03 ` [PATCH v3 0/7] firmware: coreboot: Fix probe and simplify code Brian Norris
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAODwPW8+QSittJdNLWpWKU=JNJa5bssMHQ4ER4CfnUAWaL1JGA@mail.gmail.com' \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
LKML Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \ firstname.lastname@example.org public-inbox-index lkml Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git