linux-kernel - RE: [PATCH] staging: pi433: modify bit

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: <19e492521b4a4d31aa13c45e8616689b@AcuMS.aculab.com>
Date:   Wed, 1 Feb 2023 12:34:50 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Guru Mehar Rachaputi' <gurumeharrachaputi@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] staging: pi433: modify bit_rate from u16 to u32

From: Guru Mehar Rachaputi
> Sent: 31 January 2023 10:20
...
> > >  struct pi433_tx_cfg {
> > >  	__u32			frequency;
> > > -	__u16			bit_rate;
> > > +	__u32			bit_rate;
> > >  	__u32			dev_frequency;
> > >  	enum modulation		modulation;
> > >  	enum mod_shaping	mod_shaping;
> >
> > And didn't you just break existing userspace code?  If not, how?  If so,
> > how did you test this?
> >
> My apologies, I did not study code. While testing, the probe function of
> pi433 driver didn't appear in the lsmod operation. I suspected my
> testing was wrong.

You don't need to study any code, just the structure you changed.

On all architectures (except m68k) there is a two byte pad
after the bit_rate field, this is likely to be filled with
random data from the applications stack.

On little-endian architectures the low byte is in the same place,
so if the 'random data' is zero it will happen to work.

However, on big-endian architectures, the low byte moves so
the value passed (by old applications) will always be wrong.

In reality having a uapi structure with embedded padding should
be banned.
But that would need a compiler attribute to enforce it.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)