linux-kernel - Re: A potential broken at platform driver?

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: <20190603180255.GA18054@kroah.com>
Date:   Mon, 3 Jun 2019 20:02:55 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Richard Gong <richard.gong@...ux.intel.com>
Cc:     robh+dt@...nel.org, mark.rutland@....com, dinguyen@...nel.org,
        atull@...nel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, sen.li@...el.com,
        Richard Gong <richard.gong@...el.com>
Subject: Re: A potential broken at platform driver?

On Mon, Jun 03, 2019 at 10:57:18AM -0500, Richard Gong wrote:
> 
> Hi Greg,
> 
> Following your suggestion, I replaced devm_device_add_groups() with .group =
> rus_groups in my version #4 submission. But I found out that RSU driver
> outputs the garbage data if I use .group = rsu_groups.

What is "garbage"?

> To make RSU driver work properly, I have to revert the change at version #4
> and use devm_device_add_groups() again. Sorry, I didn't catch this problem
> early.
> 
> I did some debug & below are captured log, you can see priv pointer get
> messed at current_image_show(). I am not sure if something related to
> platform driver work properly. I attach my debug patch in this mail.
> 
> 1. Using .groups = rsu_groups,
> 
> [    1.191115] *** rsu_status_callback:
> [    1.194782] res->a1=2000000
> [    1.197588] res->a1=0
> [    1.199865] res->a2=0
> [    1.202150] res->a3=0
> [    1.204433] priv=0xffff80007aa28e80
> [    1.207933] version=0, state=0, current_image=2000000, fail_image=0,
> error_location=0, error_details=0
> [    1.217249] *** stratix10_rsu_probe: priv=0xffff80007aa28e80
> root@...atix10:/sys/bus/platform/drivers/stratix10-rsu# cat current_image
> [   38.849341] *** current_image_show: priv=0xffff80007aa28d00
> ... output garbage data
> priv pointer got changed

I don't understand this, sorry.  Are you sure you are actually using the
correct pointer to your device?

> @@ -394,7 +432,7 @@ static struct platform_driver stratix10_rsu_driver = {
>  	.remove = stratix10_rsu_remove,
>  	.driver = {
>  		.name = "stratix10-rsu",
> -		.groups = rsu_groups,
> +//		.groups = rsu_groups,

Are you sure this is the correct pointer?  I think that might be
pointing to the driver's attributes, not the device's attributes.

If platform drivers do not have a way to register groups properly, then
that really needs to be fixed, as trying to register it by yourself as
you are doing, is ripe for racing with userspace.

thanks,

greg k-h