lkml.org 
[lkml]   [2021]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.14 204/849] component: do not leave master devres group open after bind
    Date
    From: Kai Vehmanen <kai.vehmanen@linux.intel.com>

    commit c87761db2100677a69be551365105125d872af5b upstream.

    In current code, the devres group for aggregate master is left open
    after call to component_master_add_*(). This leads to problems when the
    master does further managed allocations on its own. When any
    participating driver calls component_del(), this leads to immediate
    release of resources.

    This came up when investigating a page fault occurring with i915 DRM
    driver unbind with 5.15-rc1 kernel. The following sequence occurs:

    i915_pci_remove()
    -> intel_display_driver_unregister()
    -> i915_audio_component_cleanup()
    -> component_del()
    -> component.c:take_down_master()
    -> hdac_component_master_unbind() [via master->ops->unbind()]
    -> devres_release_group(master->parent, NULL)

    With older kernels this has not caused issues, but with audio driver
    moving to use managed interfaces for more of its allocations, this no
    longer works. Devres log shows following to occur:

    component_master_add_with_match()
    [ 126.886032] snd_hda_intel 0000:00:1f.3: DEVRES ADD 00000000323ccdc5 devm_component_match_release (24 bytes)
    [ 126.886045] snd_hda_intel 0000:00:1f.3: DEVRES ADD 00000000865cdb29 grp< (0 bytes)
    [ 126.886049] snd_hda_intel 0000:00:1f.3: DEVRES ADD 000000001b480725 grp< (0 bytes)

    audio driver completes its PCI probe()
    [ 126.892238] snd_hda_intel 0000:00:1f.3: DEVRES ADD 000000001b480725 pcim_iomap_release (48 bytes)

    component_del() called() at DRM/i915 unbind()
    [ 137.579422] i915 0000:00:02.0: DEVRES REL 00000000ef44c293 grp< (0 bytes)
    [ 137.579445] snd_hda_intel 0000:00:1f.3: DEVRES REL 00000000865cdb29 grp< (0 bytes)
    [ 137.579458] snd_hda_intel 0000:00:1f.3: DEVRES REL 000000001b480725 pcim_iomap_release (48 bytes)

    So the "devres_release_group(master->parent, NULL)" ends up freeing the
    pcim_iomap allocation. Upon next runtime resume, the audio driver will
    cause a page fault as the iomap alloc was released without the driver
    knowing about it.

    Fix this issue by using the "struct master" pointer as identifier for
    the devres group, and by closing the devres group after
    the master->ops->bind() call is done. This allows devres allocations
    done by the driver acting as master to be isolated from the binding state
    of the aggregate driver. This modifies the logic originally introduced in
    commit 9e1ccb4a7700 ("drivers/base: fix devres handling for master device")

    Fixes: 9e1ccb4a7700 ("drivers/base: fix devres handling for master device")
    Cc: stable@vger.kernel.org
    Acked-by: Imre Deak <imre.deak@intel.com>
    Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
    Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
    BugLink: https://gitlab.freedesktop.org/drm/intel/-/issues/4136
    Link: https://lore.kernel.org/r/20211013161345.3755341-1-kai.vehmanen@linux.intel.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/base/component.c | 5 +++--
    1 file changed, 3 insertions(+), 2 deletions(-)

    --- a/drivers/base/component.c
    +++ b/drivers/base/component.c
    @@ -246,7 +246,7 @@ static int try_to_bring_up_master(struct
    return 0;
    }

    - if (!devres_open_group(master->parent, NULL, GFP_KERNEL))
    + if (!devres_open_group(master->parent, master, GFP_KERNEL))
    return -ENOMEM;

    /* Found all components */
    @@ -258,6 +258,7 @@ static int try_to_bring_up_master(struct
    return ret;
    }

    + devres_close_group(master->parent, NULL);
    master->bound = true;
    return 1;
    }
    @@ -282,7 +283,7 @@ static void take_down_master(struct mast
    {
    if (master->bound) {
    master->ops->unbind(master->parent);
    - devres_release_group(master->parent, NULL);
    + devres_release_group(master->parent, master);
    master->bound = false;
    }
    }

    \
     
     \ /
      Last update: 2021-11-16 02:43    [W:3.277 / U:0.492 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site