Re: [PATCH v2 2/3] pwm: tegra: Check for match_data being NULL

["Uwe Kleine-König" <u.kleine-koenig@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 3300 bytes --]

On Mon, Sep 21, 2026 at 06:24:39PM +0200, Thierry Reding wrote:
> On Mon, Sep 21, 2026 at 04:34:09PM +0200, Uwe Kleine-König wrote:
> > Hello Thierry,
> > 
> > On Mon, Sep 21, 2026 at 11:47:17AM +0200, Thierry Reding wrote:
> > > On Fri, Sep 18, 2026 at 04:33:46PM +0200, Uwe Kleine-König wrote:
> > > > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> > > > index efb7ab60f602..b461d3877f43 100644
> > > > --- a/drivers/pwm/pwm-tegra.c
> > > > +++ b/drivers/pwm/pwm-tegra.c
> > > > @@ -323,6 +323,14 @@ static int tegra_pwm_probe(struct platform_device *pdev)
> > > >  	int ret;
> > > >  
> > > >  	soc = of_device_get_match_data(dev);
> > > > +	if (!soc) {
> > > > +		/*
> > > > +		 * This can only happen if pdev was matched via pdev->name
> > > > +		 * (which should not happen today) or in combination with a
> > > > +		 * driver override.
> > > > +		 */
> > > > +		return dev_err_probe(dev, -ENODEV, "Unsupported device\n");
> > > > +	}
> > > 
> > > We don't usually do this. Matching via anything other than OF device ID
> > > tables (or ACPI, I suppose) is a programming error and you deserve the
> > > crash which forces you to fix things rather than continue with an error
> > > that is easy to miss.
> > 
> > I don't agree to "you deserve the crash". IMHO even root should be
> > unable to make the kernel crash. I don't understand what you think
> > should be fixed if I hit that crash. My userspace interactions in /sys?
> > Which error is easy to miss?
> 
> Oh, root can easily make the kernel crash in any number of ways. That's
> really kind of baked into the concept.

Yeah, right. root can poke in /dev/mem (unless STRICT_DEVMEM=y). And
root can allocate memory until the machine crashes (unless a resource
limit is in place). And root can unbind devices, or bring down the
network, but that shouldn't result in a kernel crash. If it does, that's
a bug worth fixing.
 
> To me this is in the same category as force-unloading a module. You can
> do it, but you should know that it's potentially dangerous and most of
> the time doesn't make sense either.

Yes, force-unloading is another such thing, but this can only be done
if MODULE_FORCE_LOAD (default n) is enabled.

The only thing I'm aware that root can do to crash the kernel where I'm
not aware of a guard rail is `kill 1`.

> It's called forcing because there
> are guardrails in place to prevent you from trying to do it.
> 
> If a device cannot operate without device data, it doesn't make sense to
> bind to it with a driver override because then you just don't get that
> data.

Ack, it doesn't make sense, and so IMHO it's worth to spend a check to
prevent that from happening.

> I'll grant you that purposefully crashing the system is maybe a bit of
> an exaggeration if there's a knob specifically designed to let you do
> this, hence why I volunteered to look into opting out of driver_override
> where it doesn't make sense.

Cc: me if you find something. Until that happens I consider introducing
that check the right thing to do. My patch is essentially a codifycation
of such an opt-out. :-D But I agree a more semantical version would be
nicer (but not sooo nice that *I*'d start a new quest for it).

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]