linux-kernel - Re: [PATCH v2 5/7] Watchdog: introduce "pretimeout" into framework

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]

Message-ID: <555DDD63.9070705@roeck-us.net>
Date:	Thu, 21 May 2015 06:28:03 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Fu Wei <fu.wei@...aro.org>
CC:	Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>,
	Linaro ACPI Mailman List <linaro-acpi@...ts.linaro.org>,
	linux-watchdog@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	Wei Fu <tekkamanninja@...il.com>,
	G Gregory <graeme.gregory@...aro.org>,
	Al Stone <al.stone@...aro.org>,
	Hanjun Guo <hanjun.guo@...aro.org>,
	Timur Tabi <timur@...eaurora.org>,
	Ashwin Chaugule <ashwin.chaugule@...aro.org>,
	Arnd Bergmann <arnd@...db.de>, vgandhi@...eaurora.org,
	wim@...ana.be, Jon Masters <jcm@...hat.com>,
	Leo Duran <leo.duran@....com>, Jon Corbet <corbet@....net>,
	"mark.rutland" <mark.rutland@....com>
Subject: Re: [PATCH v2 5/7] Watchdog: introduce "pretimeout" into framework

On 05/21/2015 03:50 AM, Fu Wei wrote:
> Hi Guenter,
>
> Great thanks, I have got your review feedback, I will fix the problems :-)
> For update_limits, I also don't want to have that in the watchdog
> driver, and you can't find it in my last version.
> And this version, I integrate the watchdog_init_timeout and
> watchdog_init_pretimeout. so I can not add a  driver specific
> "update_limits" between them.
> I think maybe we can not make this "update_limits" after calling
> init_timeouts, because we need to test and verify the timeout setting
> right after init_pretimeout by max_timeout and min_timeout. that is

Don't you have to do that in set_pretimeout as well, and even adjust
timeout if necessary ?

> why I call update_limits right after init_pretimeout and before
> init_timeout.
>
> any suggestion ?  Great thanks !  :-)
>

Any sequence of
	set_timeout()
	set_pretimeout()
can result in an invalid timeout, since the practical timeout limits
changed. Therefore, you'll have to validate (and possibly update)
the timeout after or during each call to set_pretimeout().
Changing min_timeout and max_timeout won't help there, that
validation still has to happen.

Given this, some other driver developer may choose to not change the
timeout limits at all, validate (and if necessary silently adjust)
the timeout value in the set_timeout function, and call the set_timeout
function from set_pretimeout to ensure that the timeout is still
valid after the call to set_pretimeout. At least I would most likely
implement it that way.

This means your choice of continuously updating the timeout limits
is really a driver developer choice, not something that is needed
in the driver core. It might even be problematic, since driver
developers might forget to re-validate the current timeout after
changing the pretimeout in a similar situation.

Having said that, and looking through the SBSA driver code, it seems
to me that it does not re-validate the current timeout after setting
a new pretimeout. Unless I am missing something,
	set_timeout(10);
	set_pretimeout(20);
might therefore result in an invalid / unexpected timeout value.
Setting
	set_timeout(10);
	set_pretimeout(10);
might have even more interesting results. Can you try what happens
with your driver if you set those values ?

Thanks,
Guenter

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