linux-kernel - Re: [PATCH wireless 1/9] wifi: cfg80211: avoid garbage value of 'io_type' in brcmf_cfg80211

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] [day] [month] [year] [list]

Message-ID: <3f7bcda7-092f-4d94-80c7-2015e860fbe2@suswa.mountain>
Date: Wed, 3 Jul 2024 15:23:14 +0200
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Arend Van Spriel <arend.vanspriel@...adcom.com>
Cc: Su Hui <suhui@...china.com>, Kalle Valo <kvalo@...nel.org>,
	johannes.berg@...el.com, kees@...nel.org, a@...repo.ru,
	marcan@...can.st, quic_alokad@...cinc.com, zyytlz.wz@....com,
	petr.tesarik.ext@...wei.com, duoming@....edu.cn,
	colin.i.king@...il.com, frankyl@...adcom.com, meuleman@...adcom.com,
	phaber@...adcom.com, linville@...driver.com,
	linux-wireless@...r.kernel.org, brcm80211@...ts.linux.dev,
	brcm80211-dev-list.pdl@...adcom.com, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH wireless 1/9] wifi: cfg80211: avoid garbage value of
 'io_type' in brcmf_cfg80211_attach()

On Wed, Jul 03, 2024 at 06:42:32AM +0200, Arend Van Spriel wrote:
> On July 3, 2024 3:42:18 AM Su Hui <suhui@...china.com> wrote:
> 
> > On 2024/7/2 23:39, Arend Van Spriel wrote:
> > > On July 2, 2024 5:29:27 PM Kalle Valo <kvalo@...nel.org> wrote:
> > > 
> > > > Arend Van Spriel <arend.vanspriel@...adcom.com> writes:
> > > > 
> > > > > On July 2, 2024 3:57:27 PM Dan Carpenter <dan.carpenter@...aro.org>
> > > > > wrote:
> > > > > 
> > > > > > On Tue, Jul 02, 2024 at 08:24:44PM +0800, Su Hui wrote:
> > > > > > > brcmf_fil_cmd_int_get() reads the value of 'io_type' and passes it to
> > > > > > > brcmf_fil_cmd_data_get(). Initialize 'io_type' to avoid garbage
> > > > > > > value.
> > > > > > 
> > > > > > Since you're going to be resending anyway, please delete the space
> > > > > > char
> > > > > > from the start of the line.
> > > > > > 
> > > > > > It's weird that brcmf_fil_cmd_data_get() uses the uninitialized data.
> > > > > > It looks like it just goes to great lengths to preserve the original
> > > > > > data in io_type...  So it likely is harmless enough but still a
> > > > > > strange
> > > > > > and complicated way write a no-op.
> > > > > 
> > > > > Not sure if it helps, but I tried to explain the reason in response to
> > > > > patch 0 (cover letter).
> > > > 
> > > > Would it make more sense to have just one patch? It's the same issue
> > > > anyway.
> > > 
> > > Yes, but I would solve it in brcmf_fil_* functions (fwil.[ch]).
> > It seems you will send a new patch to solve this issue.
> > And I guess there is no need for me to resend a v2 patchset or just one
> > patch.
> 
> I am not entirely sure. If both gcc and clang would warn about using
> uninitialized data I would be fine with these patches rolled into one.

We should definitely fix this, it's just a matter of how.  UBSan will
also detect these at run time.  And honestly, it's not clear to me where
these eventually do get copied to?  Is it to the firmware?  In that case
it might be that we'd treat these as a CVE.

regards,
dan carpenter