linux-kernel - Re: [PATCH] Staging: comedi: add timeouts to while loops in s626.c

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: <5310C4CB.2060201@mev.co.uk>
Date:	Fri, 28 Feb 2014 17:18:03 +0000
From:	Ian Abbott <abbotti@....co.uk>
To:	Chase Southwood <chase.southwood@...oo.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:	Ian Abbott <ian.abbott@....co.uk>,
	"hsweeten@...ionengravers.com" <hsweeten@...ionengravers.com>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Staging: comedi: add timeouts to while loops in s626.c

On 2014-02-28 07:35, Chase Southwood wrote:
> Smatch located a handful of while loops testing readl calls in s626.c.
> Since these while loops depend on readl succeeding, it's safer to make
> sure they time out eventually.
>
> Signed-off-by: Chase Southwood <chase.southwood@...oo.com>
> ---
> Ian and/or Hartley, I'd love your comments on this.  It seems to me that
> we want these kinds of while loops properly timed out, but I want to make
> sure I'm doing everything properly.  First off, s626_debi_transfer() says
> directly that it is called from within critical sections, so I assume
> that means that the new comedi_timeout() function is no good here, and
> s626_send_dac() looked equally suspicious, so I opted for iterative
> timeouts.  Is this correct?  Also, for these timeouts, I used a very
> conservative 10000 iterations, would it be better to decrease that?

Well 10000 iterations is an improvement on infinity!  If the hardware is 
working, you'd expect it to go round a lot fewer iterations than that, 
but if the hardware is broken all bets are off, especially if it is 
generating interrupts.

> Also, do my error strings appear acceptable?

Mostly.  There's a type in one of the strings that says "TLS" instead of 
"TSL".

> And finally, are timeouts here even necessary or helpful, or are there
> any better ways to do it?

In the case of s626_send_dac(), it doesn't seem to be used in any 
critical sections, so it could make use of Hartley's comedi_timeout().

Some of the timeout errors could be propagated, especially for 
s626_send_dac() which is only reachable from very few paths.

There are other infinite loops involving calls to the s626_mc_test() 
function, but those could be dealt with by other patches.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@....co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
--
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/