Re: [PATCH] hwmon: (dell-smm-hwmon) Disable BIOS fan control on SET_FAN

[Pali RohÃr]

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?

> > 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.

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx