linux-kernel - Re: [PATCH] drm/mipi-dsi: Fix detach call without attach

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] [day] [month] [year] [list]

Date:   Wed, 6 Dec 2023 13:59:26 +0100
From:   Maxime Ripard <mripard@...nel.org>
To:     Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc:     "H. Nikolaus Schaller" <hns@...delico.com>,
        Tony Lindgren <tony@...mide.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH] drm/mipi-dsi: Fix detach call without attach

Hi,

On Wed, Dec 06, 2023 at 02:48:58PM +0200, Tomi Valkeinen wrote:
> Hi mipi dsi maintainers (I'm not sure who that is =),
> 
> On 21/09/2023 13:50, Tomi Valkeinen wrote:
> > It's been reported that DSI host driver's detach can be called without
> > the attach ever happening:
> > 
> > https://lore.kernel.org/all/20230412073954.20601-1-tony@atomide.com/
> > 
> > After reading the code, I think this is what happens:
> > 
> > We have a DSI host defined in the device tree and a DSI peripheral under
> > that host (i.e. an i2c device using the DSI as data bus doesn't exhibit
> > this behavior).
> > 
> > The host driver calls mipi_dsi_host_register(), which causes (via a few
> > functions) mipi_dsi_device_add() to be called for the DSI peripheral. So
> > now we have a DSI device under the host, but attach hasn't been called.
> > 
> > Normally the probing of the devices continues, and eventually the DSI
> > peripheral's driver will call mipi_dsi_attach(), attaching the
> > peripheral.
> > 
> > However, if the host driver's probe encounters an error after calling
> > mipi_dsi_host_register(), and before the peripheral has called
> > mipi_dsi_attach(), the host driver will do cleanups and return an error
> > from its probe function. The cleanups include calling
> > mipi_dsi_host_unregister().
> > 
> > mipi_dsi_host_unregister() will call two functions for all its DSI
> > peripheral devices: mipi_dsi_detach() and mipi_dsi_device_unregister().
> > The latter makes sense, as the device exists, but the former may be
> > wrong as attach has not necessarily been done.
> > 
> > To fix this, track the attached state of the peripheral, and only detach
> > from mipi_dsi_host_unregister() if the peripheral was attached.
> > 
> > Note that I have only tested this with a board with an i2c DSI
> > peripheral, not with a "pure" DSI peripheral.
> > 
> > However, slightly related, the unregister machinery still seems broken.
> > E.g. if the DSI host driver is unbound, it'll detach and unregister the
> > DSI peripherals. After that, when the DSI peripheral driver unbound
> > it'll call detach either directly or using the devm variant, leading to
> > a crash. And probably the driver will crash if it happens, for some
> > reason, to try to send a message via the DSI bus.
> > 
> > But that's another topic.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> > ---
> 
> Any comments? I can push this via drm-misc, but I'd like an ack.

An ack is a requirement, it's not optional.

Acked-by: Maxime Ripard <mripard@...nel.org>

Maxime

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