Re: [PATCH] ASoC: soc-core: Add missing NULL check

From: Mark Brown
Date: Fri Mar 09 2018 - 15:22:58 EST


On Fri, Mar 09, 2018 at 10:45:16AM -0800, Kees Cook wrote:
> On Fri, Mar 9, 2018 at 4:50 AM, Mark Brown <broonie@xxxxxxxxxx> wrote:
> > On Thu, Mar 08, 2018 at 12:06:53PM -0800, Kees Cook wrote:

> >> If a codec is not attached to the sound soc, a NULL deref is possible as a
> >> regular user in /sys.

> > I can't parse this, sorry. What is the "sound soc"?

> SoC's sound component? I'm not sure either. :) I was just sending the
> patch that I mentioned from the thread where Pavel mentioned this
> Oops.

Oh, Pavel's thing. I didn't look at that yet. I'm afraid your
description still isn't making much sense to me - I'm guessing that
you're just papering over an immediate crack rather than having
analyized the situation in any depth?

> >> + if (!codec || !codec->driver)
> >> + return 0;

> > How are we managing to create a sysfs file for a CODEC which doesn't
> > have a CODEC struct associated with it? That is obviously nonsensical
> > and suggests we've got some more serious problem going on here - if
> > there's no CODEC those sysfs attributes simply shouldn't be there.

> No idea! Hopefully Pavel has more details...

That's where the fix should be, it implies that there's some larger data
corruption/confusion problem somewhere else. If we've created the file
but left a NULL pointer I'd expect that there is a good chance that
there'll be other things that think we've got a CODEC and try to
defererence the pointer, it's an assumption that's present throughout
the code.

I think I might just remove the file though, it's been non-functional on
most systems for a while now as almost all the drivers migrated to
regmap and nobody complained so we should be safe. There's still
something that ought to be investigated here.

Attachment: signature.asc
Description: PGP signature