From:   David Laight <David.Laight@ACULAB.COM>
To:     'Guru Mehar Rachaputi' <gurumeharrachaputi@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC:     "linux-staging@lists.linux.dev" <linux-staging@lists.linux.dev>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] staging: pi433: modify bit_rate from u16 to u32
Thread-Topic: [PATCH] staging: pi433: modify bit_rate from u16 to u32
Thread-Index: AQHZNV2KzQ5LmfuLeUG1rTftT0qCPq66BnZA
Date:   Wed, 1 Feb 2023 12:34:50 +0000
Message-ID: <19e492521b4a4d31aa13c45e8616689b@AcuMS.aculab.com>
References: <Y9h42l/8EcPqn63x@combine-ThinkPad-S1-Yoga>
 <Y9ilj812USHrSXe+@kroah.com> <Y9jrOKMGl7ITKxhP@combine-ThinkPad-S1-Yoga>
In-Reply-To: <Y9jrOKMGl7ITKxhP@combine-ThinkPad-S1-Yoga>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

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)