linux-kernel - Re: [PATCH v2] extcon: fix extcon_get_extcon

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: <20211216155916.GA7738@kadam>
Date:   Thu, 16 Dec 2021 18:59:16 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Chanwoo Choi <cw00.choi@...sung.com>
Cc:     MyungJoo Ham <myungjoo.ham@...sung.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Sebastian Reichel <sre@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Hans de Goede <hdegoede@...hat.com>,
        Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-omap@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH v2] extcon: fix extcon_get_extcon_dev() error handling

On Thu, Dec 16, 2021 at 05:38:04PM +0900, Chanwoo Choi wrote:
> > 
> > To be honest, I'm not sure how this differs from other functions which
> > return -EPROBE_DEFER.  How do other functions guarantee they will only
> > be called from probe()?
> 
> If it is possible to know extcon_get_extcon_dev() will be only callled on probe,
> it is no problem. But, it is not able to guarantee that extcon_get_extcon_dev()
> is called on probe. Because of this reason, this issue should be handled in each device driver.
> 
> -EPROBE_DEFER is only for probe step. If return -EPROBE_DEFER except for probe,
> it is wrong return value.

The future is vast and unknowable.  We can't really future proof code
and we should never try do that if it makes the code more complicated
right now.

When Andy submitted basically the same patch as me three years ago we
worried about future developers so we didn't merge his patch.  But
three years later no non-probe() were introduced.  Meanwhile the bad API
created bugs in the kernel for current users.

To some extent, we have to trust future developers to do sane things.

I have also created a static checker test for people who call
EPROBE_DEFER outside of probe functions.  I don't know that this test
will work.  It will take a few days for the call tree to be built.
Another option would be to change the warning from "is this called from
something besides probe()" to "is this called from an ioctl".  That
would generate fewer false positives.

Or potentially, I could save a most recent function pointer in the call
tree.  I'll play around with that in the coming months.

Anyway, I've attached my first draft just to show you my thinking on
this.

regards,
dan carpenter

View attachment "smatch_kernel_probe.c" of type "text/x-csrc" (2518 bytes)

View attachment "check_EPROBE_DEFER.c" of type "text/x-csrc" (1620 bytes)