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

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: <20180309202242.GA3073@sirena.org.uk>
Date:   Fri, 9 Mar 2018 20:22:42 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Takashi Iwai <tiwai@...e.com>, Pavel Machek <pavel@....cz>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        moderated for non-subscribers <alsa-devel@...a-project.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ASoC: soc-core: Add missing NULL check

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@...nel.org> 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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)