On Tue, Nov 9, 2021 at 11:09 AM Mauro Lima <mauro.lima@xxxxxxxxxxxxx> wrote: > > On Tue, Nov 9, 2021 at 9:42 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > On Tue, Nov 09, 2021 at 01:32:48PM +0100, Hans-Gert Dahmen wrote: > > > On Tue, 9 Nov 2021, 11:28 Greg KH, <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > On Tue, Nov 09, 2021 at 09:52:53AM +0100, Hans-Gert Dahmen wrote: > > > > > On 09.11.21 07:16, Greg KH wrote: > > > > > > On Tue, Nov 09, 2021 at 01:01:30AM +0100, Hans-Gert Dahmen wrote: > > > > > > > Make the 16MiB long memory-mapped BIOS region of the platform SPI flash > > > > > > > on X86_64 system available via /sys/kernel/firmware/flash_mmap/bios_region > > > > > > > for pen-testing, security analysis and malware detection on kernels > > > > > > > which restrict module loading and/or access to /dev/mem. > > > > > > > > > > > > That feels like a big security hole we would be opening up for no good > > > > > > reason. > > > > > > > > > > > > > It will be used by the open source Converged Security Suite. > > > > > > > https://github.com/9elements/converged-security-suite > > > > > > > > > > > > What is the reason for this, and what use is this new interface? > > > > > Because it is very hard to access the SPI flash to read the BIOS contents > > > > > for (security) analysis and this works without the more complex and > > > > > unfinished SPI drivers and it does so on a system where we may not access > > > > > the full /dev/mem. > > > > > > > > Why not fix the spi drivers to do this properly? What is preventing you > > > > from doing that instead of adding a new user/kernel api that you will > > > > have to support for the next 20+ years? > > > > > > > > > > Because this interface we want to use is inherently tied to the > > > workings of x86_64 CPUs. It is very stable and easy to use. The SPI > > > drivers in contrast have a history of being complex, buggy and in > > > general not reliable. > > > > Do you have a pointer to these complex and buggy drivers anywhere? > > Right now the intel spi driver has a _(DANGEROUS)_ tag [1], this is > preventing the kernel engineers from compiling the driver into > different distros. > A couple of months ago I tried to modify the driver and make it RO [2] > but the drivers author told me that they don't want to remove the > tag, even if the driver is compiled without any write/erase > functionality as I proposed. > The driver is fixed as the author claims but, as I said, they want to > preserve the tag. > This patch is close to what we need if we can't use the intel spi mechanism. > > > > > > This module will require very little maintenance > > > and will probably work in the future without needing modification for > > > newer platforms. It generally is meant as a fallback to the real SPI > > > flash drivers so that userspace programs are able to provide essential > > > functionality. > > > > If it is a "fallback", how is it going to interact on a system where the > > SPI driver is loaded? > > > > > > > > > +static int __init flash_mmap_init(void) > > > > > > > +{ > > > > > > > + int ret; > > > > > > > + > > > > > > > + pdev = platform_device_register_simple("flash_mmap", -1, NULL, 0); > > > > > > > + if (IS_ERR(pdev)) > > > > > > > + return PTR_ERR(pdev); > > > > > > > + > > > > > > > + ret = sysfs_create_group(&pdev->dev.kobj, &flash_mmap_group); > > > > > > > > > > > > You just raced with userspace and lost > > > > > > > Please set the attribute to the platform driver before you create the > > > > > > device. > > > > > > > > > > > > > > > > Sorry, but I went through tons of code and could not find a single instance > > > > > where I can use a default group for creation without using a probe function > > > > > that does the magic for me. Please help me find the correct way of doing > > > > > this without manually creating and adding kobjects. > > > > > > > > The problem here is that you are abusing the platform driver code and > > > > making a "fake" device that is not attached to anything real, and > > > > without a driver. That should not be done, as you do not know what > > > > hardware this driver is going to be run on. > > > > > > > > Bind to the real hardware resource please. > > > > > > In a previous mail in June you suggested this is some sort of platform > > > device, that is why I rewrote it as such. > > > > That's fine, but bind to the real platform device that describes this > > hardware! You are not doing anything like this at all here, you are > > just creating a random device in the sysfs tree and instantly allowing > > userspace access to raw memory. You are not actually controlling the > > platform device at all. > > > > > The hardware resource here > > > is the south bridge for x86_64 CPUs and the module dependencies will > > > compile it only for these platforms. > > > > What "module dependencies"? You do realize that distro kernels enable > > all modules to be built. We have an "autoload only the modules that this > > hardware needs" infrastructure that you need to tie into here, you are > > totally ignoring it (despite it being present for almost 20 years > > now...) > > > > > The situation is like, if you > > > have that CPU, you have the hardware, so it is implicitly bound or it > > > just will not execute on a machine code level. > > > > What do you mean "implicitly bound"? How does this tie into the driver > > model such that it will only be autoloaded, and have the driver bind to > > the hardware, that it knows it can control? > > > > That is what needs to be fixed here, you are just claiming that it can > > talk to the hardware without having any check at all for this. > > > > > I feel like your > > > requirement is somewhat impossible to satisfy and I really don't know > > > what to do. Please, can anyone help me by pointing out a proper > > > example elsewhere in the kernel? > > > > What resource in the system describes the hardware you wish to bind to? > > Write a driver for _that_ resource, and have it be properly autoloaded > > when that resource is present in the system. > > > > You have hundreds of examples running on your system today, but as I do > > not know where this hardware is described specifically, it's hard to be > > able to point you to an example that works like that. > > > > So again, what hardware signature describes the resource you wish to > > bind to? > > > > thanks, > > > > greg k-h > > Thanks, Mauro. Sorry, I forgot the links [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1734147/+index?comments=all [2] https://lore.kernel.org/linux-mtd/20210910211348.642103-1-mauro.lima@xxxxxxxxxxxxx/ Thanks
