linux-kernel - Re: [PATCH] Request driver inclusion

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 18 Jun 2009 14:45:41 +0200
From:	Borislav Petkov <petkovbb@...glemail.com>
To:	Peter Feuerer <peter@...e.net>
Cc:	Andreas Mohr <andi@...as.de>, Ed Tomlinson <edt@....ca>,
	akpm@...ux-foundation.org, Len Brown <len.brown@...el.com>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Request driver inclusion - acer aspire one fan control

Hi,

On Thu, Jun 18, 2009 at 1:49 PM, Peter Feuerer<peter@...e.net> wrote:
>>>>> Actually I think pre_suspend_kernelmode is needed, so it won't be
>>>>> dropped.
>>>>
>>>> and it is needed, because...?
>>>
>>> It's needed because we do now a clean revert to bios mode before we
>>> suspend.
>>> And after resume we have to switch to kernelmode again, if the driver was
>>> in
>>> kernelmode before suspend. So we need to keep track of in what state the
>>> driver was before suspending. That's what's this variable is for.
>>
>> You've got that state in the 'kernelmode' variable. See full comment:
>> http://marc.info/?l=linux-kernel&m=124482114200865
>
> We are talking about patch 0.5.9 and not 0.5.8, are we?
> http://patchwork.kernel.org/patch/30733/mbox/
>
> have a look at at line 543:
> +       /* remember previous setting */
> +       pre_suspend_kernelmode = kernelmode;
> +
> +       if (kernelmode) {
> +               acerhdf_revert_to_bios_mode();
> +               if (acerhdf_thz_dev)
> +                       thermal_zone_device_update(acerhdf_thz_dev);
> +       }

ok, this starts to look quite a bit overengineered for no reason. First,
acerhdf_revert_to_bios_mode() sets the fan to auto. Then, you've added
a thermal_zone_device_update() call in there which does set the fan to
auto indirectly _again_. And we end up with _three_ variables which
represent only _one_ state. Here's what it should do:

suspend:
 - set fan to auto

resume:
 - the thermal layer figures out what to do based on the 'kernelmode' and
 current temperature.

That's it, everything else is too much.

I'll have a deeper look during the weekend.

-- 
Regards/Gruss,
Boris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/