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]
Date:   Fri, 21 Jan 2022 19:40:46 -0600
From:   Rob Herring <robh@...nel.org>
To:     Peter Geis <pgwipeout@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] device_property_read_u16 reads out only zero

On Fri, Jan 21, 2022 at 7:27 PM Peter Geis <pgwipeout@...il.com> wrote:
>
> On Fri, Jan 21, 2022 at 5:19 PM Rob Herring <robh@...nel.org> wrote:
> >
> > On Fri, Jan 14, 2022 at 05:48:52PM -0500, Peter Geis wrote:
> > > Good Evening,
> > >
> > > I seem to have come across a strange bug with drivers/base/property.c
> > > while expanding the new cyttsp5 driver to handle device-tree
> > > overrides.
> > >
> > > With the property:
> > > touchscreen-size-x = <1863>;
> > > The following code:
> > > u32 test_u32 = 32; /* canary to catch writing a zero */
> > > u16 test_u16 = 16; /* canary to catch writing a zero */
> > > int ret;
> > >
> > > ret = device_property_read_u32(ts->dev, "touchscreen-size-x", &test_u32);
> > > if(ret)
> > > dev_err(ts->dev, "read_u32 failed ret: %d\n", ret);
> > >
> > > ret = device_property_read_u16(ts->dev, "touchscreen-size-x", &test_u16);
> > > if(ret)
> > > dev_err(ts->dev, "read_u16 failed ret: %d\n", ret);
> > >
> > > dev_err(ts->dev, "read_u32: %d, read_u16: %d\n", test_u32, test_u16);
> > >
> > > returns the following:
> > > [    1.010876] cyttsp5 5-0024: read_u32: 1863, read_u16: 0
> > >
> > > This was as of 5.16-rc8, using the
> > > gcc-arm-10.2-2020.11-x86_64-aarch64-none-linux-gnu compiler.
> > > I honestly am at a loss here, any insight you can provide here would
> > > be appreciated.
> >
> > The property "touchscreen-size-x" is a u32. Calling
> > device_property_read_u16 is an error though one is not returned here.
> > You get 0 because that is what the first 2 bytes of the property
> > contain. DT data is big-endian.
>
> I figured this was the case, but I was hopeful the operators would be
> smart enough to handle endian translations.
> Wouldn't all DT numeric properties be u32, meaning
> device_property_read_u16 and device_property_read_u8 are meaningless
> on little endian devices?

No.

> Or is there a way to force smaller values in the DT?

Yes. [ 0 1 2 ] notation is 8-bit. Or you can use the /bits/ 8|16|64 directive.

> > I suspect making this a hard error would break some users, but we could
> > try a WARN.
>
> I don't suspect it would be trivial to implement endian translation
> for these functions?

They do endian translation already. In your case byte 0 and byte 1 are swapped.

The DT data is purely a byte array once it's in the dtb. It's up to
the caller with specific knowledge about a property to know how to
interpret it. It would be nice if all the size and type information
was maintained in the dtb format, but it's not and no one has stepped
up to do a new format (changing would be painful too).

Rob

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ