Messages in this thread | | | Subject | Re: [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control on SET_FAN | From | Guenter Roeck <> | Date | Fri, 15 Nov 2019 06:46:20 -0800 |
| |
On 11/15/19 6:36 AM, Pali Rohár wrote: > On Friday 15 November 2019 14:44:59 Giovanni Mascellani wrote: >> Hi, >> >> Il 15/11/19 12:29, Pali Rohár ha scritto: >>> No. I have not tested that my patch on other models. You can look at >>> that my patch link, some people already reported that on some models it >>> does not work... >> >> Yes, I saw that. But they are also using other laptops, which could be >> excluded by the whitelist until we have a working command for them. >> >>> What is incompatible with Secure Boot? sys_iopl nor sys_ioperm has >>> nothing to do with UEFI Secure Boot. They are just ordinary syscalls >>> like any other and are executed on kernel side. And IIRC it is up to the >>> kernel how it set privileges for I/O ports. Maybe bootloaders under >>> Secure Boot can set some other default values, but kernel can overwrite >>> them. I really do not see reason why it could be incompatible with UEFI >>> Secure Boot nor why it should not work. It just looks like improper >>> setup from userspace. >> >> Ah, my fault here: there is a patch to lock down the kernel when it is >> started with Secure Boot[1], and I believed that was already in >> mainline, but apparently it is not. > > Ok, so, this is not a problem. > > I hope that such patch is not going to be part of mainline kernel as it > would break lot of things. UEFI Secure Boot and kernel lock down are two > different things. It would be really suspicious that for "workarounding" > broken functionality would be needed to turn of totally unrelated option > in firmware SETUP. > >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit?id=160a99536afc317b337212dd40eaba341702343e >> >>> It makes sense to have implemented in kernel switching between automatic >>> and manual mode as kernel has API for it. But it depends on the fact >>> that we know which SMM API to call. And currently it is just some random >>> call which we somehow observed that is working on 2 machines and on some >>> more other does not. Until we have fully working implementation we >>> cannot put it into kernel. >> >> Mmh, but then what is a plausible way forward to have this? Can't we >> start populating a whitelist for the machines we already have a solution >> for, and add more entries when they are discovered? This would already >> give a benefit to the users of supported laptops, without impacting >> users of unsupported laptops. My feeling is that if we pretend to have >> information for all possible models supported by dell-smm-hwmon, we will >> never benefit any user. Or can you suggest a plan? > > The following question is up to the hwmon maintainers. Guenter, should > we start creating whilelist of machines which support those SMM calls > for enabling / disabling BIOS auto mode? And maintaining this whilelist > in kernel dell-smm-hwmon driver? > Ok with me.
>>> What does not make sense for me is to have that "protection" in kernel. >> >> I am not really sure which "protection" you mean. I didn't mean to >> introduce any protection from userspace in my patch, I just wanted to >> make SET_FAN working. I think that the kernel module cannot (and should >> not) reliably protect itself from userspace sending random IO port >> reads/writes. > > I mean protection to disallow calling SET_FAN operation when auto mode > is enabled. > I don't have a problem with that, as long as it only applies in conjunction with the whitelist. The whitelist would determine if the pwmX_enable attribute is supported. If it is, pwmX can only be written if pwm1_enable is set to 1 (manual fan speed control). This is pretty common for other drivers.
Guenter
| |