linux-kernel - Re: [PATCH v7 14/34] i2c: tegra: Clean up probe 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: <20200921111536.GG3950626@ulmo>
Date:   Mon, 21 Sep 2020 13:15:36 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Dmitry Osipenko <digetx@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        Wolfram Sang <wsa@...-dreams.de>,
        Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        linux-tegra@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 14/34] i2c: tegra: Clean up probe function

On Thu, Sep 17, 2020 at 04:46:45PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 3:37 PM Thierry Reding <thierry.reding@...il.com> wrote:
> > On Wed, Sep 09, 2020 at 01:39:46AM +0300, Dmitry Osipenko wrote:
> 
> ...
> 
> > > +     ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr,
> > > +                            IRQF_NO_SUSPEND, dev_name(&pdev->dev),
> > > +                            i2c_dev);
> > > +     if (ret)
> > > +             return ret;
> >
> > Is it safe to install the interrupt handler at this point? What if,
> > perhaps because some bootloader didn't properly quiesce the I2C
> > controller, an interrupt triggers immediately after this?
> 
> It\s easy to check with debug shared IRQ, but here IRQ is not shared...
> So, with a hack into the code and enabled debug it will be achievable to test.
> 
> And you probably meant that IRQ triggers *before* the handler is in place?

It shouldn't be possible for the interrupt to trigger before the handler
is in place, because requesting the IRQ here is what will unmask the IRQ
at the parent.

I'm more concerned that the hardware may be in some state where it
already has a pending interrupt and therefore unmasking (as part of the
request here) is going to immediately trigger an interrupt. If we
haven't set up i2c_dev completely at this point this may cause issues
because the interrupt handler will now have to deal with a partially
initialized structure.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)