linux-kernel - Re: [patch 02/12] can: c_can: Fix hardware raminit function

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: <53293B31.2060406@hartkopp.net>
Date:	Wed, 19 Mar 2014 07:37:37 +0100
From:	Oliver Hartkopp <socketcan@...tkopp.net>
To:	Thomas Gleixner <tglx@...utronix.de>,
	Marc Kleine-Budde <mkl@...gutronix.de>
CC:	LKML <linux-kernel@...r.kernel.org>,
	Wolfgang Grandegger <wg@...ndegger.com>,
	Markus Pargmann <mpa@...gutronix.de>,
	Benedikt Spranger <b.spranger@...utronix.de>,
	linux-can@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [patch 02/12] can: c_can: Fix hardware raminit function

On 18.03.2014 23:15, Thomas Gleixner wrote:
> On Tue, 18 Mar 2014, Marc Kleine-Budde wrote:
>> On 03/18/2014 06:19 PM, Thomas Gleixner wrote:
>>> +static void c_can_hw_raminit_wait(const struct c_can_priv *priv, u32 mask,
>>> +				  u32 val)
>>> +{
>>> +	/* We look only at the bits of our instance. */
>>> +	val &= mask;
>>> +	while ((readl(priv->raminit_ctrlreg) & mask) != val)
>>> +		udelay(1);
>>
>> Do we have to add a timeout here, or is it "safe" to have a potential
>> endless loop here? As you have probably tortured the hardware and driver
>> a lot (or have been tortured by them), I assume you would have added a
>> timeout check if you had seen a lockup here.
> 
> I haven't seen any failure on that. We could add a timeout for
> paranoia reasons. I'm quite sure that the raminit works as advertised
> when we do it the right way. The only way to wreckage it so far is by
> not waiting for it to complete.

As long as it is 100% guaranteed that we

1. really work on a valid C_CAN core
2. this CAN controller can not be unplugged

not adding a timeout would be ok.

I remember a system hang with a SJA1000 based PCMCIA card when unplugged
under heavy load:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a7762b10c12a70c5dbf2253142764b728ac88c3a

Regards,
Oliver

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