linux-kernel - Re: [PATCH] i2c: iproc: fix race between client unreg and isr

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: <CAHO=5PF6WmgTYAA8vVd86cx0YTx0CKouJ2k+13hNVCPiEtMVYA@mail.gmail.com>
Date:   Mon, 27 Jul 2020 10:02:09 +0530
From:   Rayagonda Kokatanur <rayagonda.kokatanur@...adcom.com>
To:     Wolfram Sang <wsa@...nel.org>
Cc:     Ray Jui <ray.jui@...adcom.com>,
        Dhananjay Phadke <dphadke@...ux.microsoft.com>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ray Jui <rjui@...adcom.com>,
        BCM Kernel Feedback <bcm-kernel-feedback-list@...adcom.com>
Subject: Re: [PATCH] i2c: iproc: fix race between client unreg and isr

On Sat, Jul 25, 2020 at 3:48 PM Wolfram Sang <wsa@...nel.org> wrote:
>
>
> > I think the following sequence needs to be implemented to make this
> > safe, i.e., after 'synchronize_irq', no further slave interrupt will be
> > fired.
> >
> > In 'bcm_iproc_i2c_unreg_slave':
> >
> > 1. Set an atomic variable 'unreg_slave' (I'm bad in names so please come
> > up with a better name than this)
> >
> > 2. Disable all slave interrupts
> >
> > 3. synchronize_irq
> >
> > 4. Set slave to NULL
> >
> > 5. Erase slave addresses
>
> What about this in unreg_slave?
>
> 1. disable_irq()
>         This includes synchronize_irq() and avoids the race. Because irq
>         will be masked at interrupt controller level, interrupts coming
>         in at the I2C IP core level should still be pending once we
>         reenable the irq.
>
> 2. disable all slave interrupts
>
> 3. enable_irq()
>
> 4. clean up the rest (pointer, address)
>
> Or am I overlooking something?

This sequence will take care of all cases.

@Dhananjay Phadke is it possible to verify this from your side once.

Best regards,
Raaygonda