netdev - Re: [PATCH net-next v1 1/1] ppp: Fix KMSAN warning by initializing 2-byte header

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: <20250225093249.GI1615191@kernel.org>
Date: Tue, 25 Feb 2025 09:32:49 +0000
From: Simon Horman <horms@...nel.org>
To: Jiayuan Chen <jiayuan.chen@...ux.dev>
Cc: Jakub Kicinski <kuba@...nel.org>, bpf@...r.kernel.org,
	netdev@...r.kernel.org, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, pabeni@...hat.com, ricardo@...liere.net,
	viro@...iv.linux.org.uk, dmantipov@...dex.ru,
	aleksander.lobakin@...el.com, linux-ppp@...r.kernel.org,
	linux-kernel@...r.kernel.org, mrpre@....com,
	syzbot+853242d9c9917165d791@...kaller.appspotmail.com
Subject: Re: [PATCH net-next v1 1/1] ppp: Fix KMSAN warning by initializing
 2-byte header

On Fri, Feb 21, 2025 at 09:48:33AM +0800, Jiayuan Chen wrote:
> On Thu, Feb 20, 2025 at 03:27:03PM -0800, Jakub Kicinski wrote:
> > On Tue, 18 Feb 2025 21:31:44 +0800 Jiayuan Chen wrote:
> > > -		*(u8 *)skb_push(skb, 2) = 1;
> > > +		*(u16 *)skb_push(skb, 2) = 1;
> > 
> > This will write the 1 to a different byte now, on big endian machines.
> > Probably doesn't matter but I doubt it's intentional?
> > -- 
> > pw-bot: cr
> You are correct that I assigned the value in a way that produces different
> data on big-endian and little-endian systems, although it doesn't cause
> any issues.
> I think it's better to assign it correctly according to the corresponding
> header and add more comments to avoid confusion for other developers in
> the future.

I agree correctness is good.

Perhaps I am over-thinking things, but does the following approach
achieve both of the following?

a) Initialise both bytes.
b) Place the 1 consistently on both big and little endian hosts,
   as is the case without this patch (which I assume is correct).

	*(__be16 *)skb_push(skb, 2) = cpu_to_be16(1);