netdev - Re: [PATCH net-next v2 2/4] can: cc770: add legacy ISA bus driver for the CC770 and AN82527

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:	Tue, 10 Jan 2012 15:50:12 +0100
From:	Oliver Hartkopp <socketcan@...tkopp.net>
To:	Wolfgang Grandegger <wg@...ndegger.com>
CC:	info@...ax.com, David Laight <David.Laight@...LAB.COM>,
	henrik@...conx.com, netdev@...r.kernel.org,
	linux-can@...r.kernel.org, socketcan-users@...ts.berlios.de,
	IreneV <boir1@...dex.ru>,
	Stanislav Yelenskiy <stanislavelensky@...oo.com>, oe@...t.de,
	henrik@...us-sw.com
Subject: Re: [PATCH net-next v2 2/4] can: cc770: add legacy ISA bus driver
 for the CC770 and AN82527

On 10.01.2012 15:43, Wolfgang Grandegger wrote:

> On 01/10/2012 01:41 PM, Wolfgang Zarre wrote:
>> Hello David,
>>>
>>>> cc770_isa_port_write_reg_indirect(const struct cc770_priv *priv,
>>>>                            int reg, u8 val)
>>>>    {
>>>>        unsigned long base = (unsigned long)priv->reg_base;
>>>> +    unsigned long flags;
>>>>
>>>> +    spin_lock_irqsave(&outb_lock, flags);
>>>>        outb(reg, base);
>>>>        outb(val, base + 1);
>>>> +    spin_unlock_irqrestore(&outb_lock, flags);   
>>>
>>> Is there a 'read_reg_indirect' function??
>>
>> Yes, there is.
>>
>>> If so it also needs to use the same mutex.
>>
>> Actually, I don't think that we have a problem with mutex
>> beside that it's using just one inb() statement but having
>> for sure with an interrupt between both outb() statements which
>> seems to be critical for the cc770.
> 
> But the indirect read function also sets the address register before
> reading the data using inb(). This sequence should also not be
> interrupted and therefore we need to synchronize. For the indirect
> access of the SJA1000 we also need to add spinlocks. Wonder why nobody
> complained so far.


Maybe due to old single core hardware that has SJA1000 hw with indirect
addressing? Or this old hardware has not been 'kernel-upgraded' so far ...

In any case you are right with the missing spinlocks.

Regards,
Oliver
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html