linux-kernel - Re: [PATCH v3 4/6] bus: Add Baikal-T1 AXI-bus driver

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: <CAHp75VfqnLTK-fT-KdzN-3=u=dLTJjo=9+b=oKXUoUOsbeg00Q@mail.gmail.com>
Date:   Thu, 28 May 2020 16:51:03 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc:     Serge Semin <fancer.lancer@...il.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Paul Burton <paulburton@...nel.org>,
        Olof Johansson <olof@...om.net>,
        Rob Herring <robh+dt@...nel.org>,
        "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
        "soc@...nel.org" <soc@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 4/6] bus: Add Baikal-T1 AXI-bus driver

On Thu, May 28, 2020 at 4:40 PM Serge Semin
<Sergey.Semin@...kalelectronics.ru> wrote:
>
> On Thu, May 28, 2020 at 01:00:57AM +0300, Andy Shevchenko wrote:
> > On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@...kalelectronics.ru>
> > wrote:
> >
> > > AXI3-bus is the main communication bus connecting all high-speed
> > > peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
> > > SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
> > > Interconnect (so called AXI Main Interconnect) routing IO requests from
> > > one SoC block to another. This driver provides a way to detect any bus
> > > protocol errors and device not responding situations by means of an
> > > embedded on top of the interconnect errors handler block (EHB). AXI
> > > Interconnect QoS arbitration tuning is currently unsupported.
> > > The bus doesn't provide a way to detect the interconnected devices,
> > > so they are supposed to be statically defined like by means of the
> > > simple-bus sub-nodes.
> >
> >
> >
> > Few comments in case if you need a new version. Main point is about
> > sysfs_streq().
>
> Hello, Andy. Thanks for your comments. I'll address most of them in a follow-up
> patches. See just one note below.
>
> >
> >
> > > Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> > > Cc: Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>
> > > Cc: Paul Burton <paulburton@...nel.org>
> > > Cc: Olof Johansson <olof@...om.net>
> > > Cc: Rob Herring <robh+dt@...nel.org>
> > > Cc: linux-mips@...r.kernel.org
> > > Cc: soc@...nel.org
> > > Cc: devicetree@...r.kernel.org
> > >
>
> [nip]
>
> > > +
> > > +static void bt1_axi_clear_data(void *data)
> > > +{
> > > +       struct bt1_axi *axi = data;
> > > +       struct platform_device *pdev = to_platform_device(axi->dev);
> > > +
> > > +       platform_set_drvdata(pdev, NULL);
> >
> >
>
> > Doesn't device driver core do this already?
>
> It doesn't on remove.

__device_release_driver() calls dev_set_drvdata(dev, NULL);
What did I miss?

> This cleanups the drvdata pointer when the driver is
> unloaded at the moment of remove() callback calling. This is a good
> practice to leave the device the same as it has been before usage including
> the pointer value. In this case if theoretically someone (though very
> unlikely, but anyway) would try to use the pointer without having it
> initialized, the NULL-dereference would pop up, otherwise we may
> corrupt someones memory, which is very nasty.

The above is right and I agree with.

-- 
With Best Regards,
Andy Shevchenko