linux-kernel - Re: [PATCH v4 1/5] i2c:ocores: stop transfer on timeout

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 for Android: free password hash cracker in your pocket

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

Message-ID: <d406a2ea-9d37-3eca-d975-818a22ca2458@axentia.se>
Date:   Mon, 11 Feb 2019 10:44:46 +0000
From:   Peter Rosin <peda@...ntia.se>
To:     Federico Vaga <federico.vaga@...n.ch>,
        Peter Korsgaard <peter@...sgaard.com>,
        Andrew Lunn <andrew@...n.ch>
CC:     "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/5] i2c:ocores: stop transfer on timeout

On 2019-02-11 09:31, Federico Vaga wrote:
> Detecting a timeout is ok, but we also need to assert a STOP command on
> the bus in order to prevent it from generating interrupts when there are
> no on going transfers.
> 
> Example: very long transmission.
> 
> 1. ocores_xfer: START a transfer
> 2. ocores_isr : handle byte by byte the transfer
> 3. ocores_xfer: goes in timeout [[bugfix here]]
> 4. ocores_xfer: return to I2C subsystem and to the I2C driver
> 5. I2C driver : it may clean up the i2c_msg memory
> 6. ocores_isr : receives another interrupt (pending bytes to be
>                 transferred) but the i2c_msg memory is invalid now
> 
> So, since the transfer was too long, we have to detect the timeout and
> STOP the transfer.
> 
> Another point is that we have a critical region here. When handling the
> timeout condition we may have a running IRQ handler. For this reason I
> introduce a spinlock.
> 
> In order to make easier to understan locking I have:
> - added a new function to handle timeout
> - modified the current ocores_process() function in order to be protected
>   by the new spinlock
> Like this it is obvious at first sight that this locking serializes
> the execution of ocores_process() and ocores_process_timeout()
> 

*snip*

> @@ -184,14 +197,14 @@ static void ocores_process(struct ocores_i2c *i2c)
>  
>  				oc_setreg(i2c, OCI2C_DATA, addr);
>  				oc_setreg(i2c, OCI2C_CMD,  OCI2C_CMD_START);

Didn't checkpatch complain about the double space? Fixing it fits in
patch 5...

Cheers,
Peter