linux-kernel - Re: [PATCH] [RFT] drm: adv7511/33: fix adv7511_cec

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: <CALAqxLUXWk=aPCJff+1tYLgPrQA87voROJu2Rw6Rya11=TczNQ@mail.gmail.com>
Date:   Thu, 16 Nov 2017 14:20:30 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Archit Taneja <architt@...eaurora.org>,
        Xinliang Liu <xinliang.liu@...aro.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Sean Paul <seanpaul@...omium.org>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        David Airlie <airlied@...ux.ie>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Bhumika Goyal <bhumirks@...il.com>,
        Inki Dae <inki.dae@...sung.com>,
        dri-devel@...ts.freedesktop.org,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [RFT] drm: adv7511/33: fix adv7511_cec_init() failure handling

On Thu, Nov 16, 2017 at 1:50 PM, John Stultz <john.stultz@...aro.org> wrote:
> On Wed, Nov 15, 2017 at 4:37 AM, Arnd Bergmann <arnd@...db.de> wrote:
>> An otherwise correct cleanup patch from Dan Carpenter turned a broken
>> failure handling from a feature patch by Hans Verkuil into a kernel
>> Oops, so bisection points to commit 7af35b0addbc ("drm/kirin: Checking
>> for IS_ERR() instead of NULL") rather than 3b1b975003e4 ("drm:
>> adv7511/33: add HDMI CEC support").
>>
>> I've managed to piece together several partial problems, though
>> I'm still struggling with the bigger picture:
>>
>> adv7511_probe() registers a drm_bridge structure that was allocated
>> with devm_kzalloc(). It calls adv7511_cec_init(), which fails for an
>> unknown reason, which in turn triggers the registered structure to be
>> removed.
>>
>> Elsewhere, kirin_drm_platform_probe() gets called, which calls
>> of_graph_get_remote_node(), and that returns NULL. Before Dan's
>> patch we would go on with a NULL pointer here and register that,
>> now kirin_drm_platform_probe() fails with -ENODEV.
>>
>> In a third driver, dsi_parse_dt() calls drm_of_find_panel_or_bridge(),
>> which after not finding a panel goes on to call of_drm_find_bridge(),
>> and that crashes due to the earlier list corruption.
>>
>> This addresses the first issue by making sure that adv7511_probe()
>> does not leave behind any corrupted list entries. This should
>> get the system back to boot but needs testing. From my understanding,
>> there is at least one more bug that needs to be resolved to actually
>> get everything to work again.
>
> So I've started hitting the issue this patch tries to address (now
> that the related code landed in Linus' tree). The only issue is that
> with this fix, I don't see graphics initializing properly, so I
> suspect something is still wrong with the error handling (though what
> exactly I'm not sure).

So this seems to only happen when CONFIG_DRM_I2C_ADV7511_CEC is
enabled. If it is on, I don't get any graphics, but if its disabled
graphics works.

Tying this with Arnd's patch, I'm guessing adv7511_cec_init() is
failing, but it seems like instead of just disabling the CEC feature,
we're failing to load the driver entirely.

Maybe should the logic be something like:
#ifdef CONFIG_DRM_I2C_ADV7511_CEC
        ret = adv7511_cec_init(dev, adv7511, offset);
        if (ret)
#endif
            regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
                        ADV7511_CEC_CTRL_POWER_DOWN);

?
thanks
-john