linux-kernel - Re: [PATCH] HID: steam: use hid_device.driver_data instead of hid_set

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:   Sat, 9 Jun 2018 23:29:59 +0200
From:   Mariusz Ceier <mceier@...il.com>
To:     Rodrigo Rivas Costa <rodrigorivascosta@...il.com>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        "Pierre-Loup A. Griffais" <pgriffais@...vesoftware.com>,
        Clément VUCHENER <clement.vuchener@...il.com>,
        lkml <linux-kernel@...r.kernel.org>,
        linux-input <linux-input@...r.kernel.org>
Subject: Re: [PATCH] HID: steam: use hid_device.driver_data instead of hid_set_drvdata()

Hello Rodrigo,
   I confirm that this patch fixes the steam controller related crash
for me. I don't know if it's correct, but at least it's:

Tested-by: Mariusz Ceier <mceier+kernel@...il.com>

I'm attaching the backtrace of crash I was getting before applying the patch.

Best regards,
Mariusz Ceier


On 22 May 2018 at 22:10, Rodrigo Rivas Costa
<rodrigorivascosta@...il.com> wrote:
> When creating the low-level hidraw device, the reference to steam_device
> was stored using hid_set_drvdata(). But this value is not guaranteed to
> be kept when set before calling probe. If this pointer is reset, it
> crashes when opening the emulated hidraw device.
>
> It looks like hid_set_drvdata() is for users "avobe" this hid_device,
> while hid_device.driver_data it for users "below" this one.
>
> In this case, we are creating a virtual hidraw device, so we must use
> hid_device.driver_data.
>
> Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@...il.com>
> ---
>
> This patch is to be applied over hid/for-4.18/hid-steam. Is this the
> proper way to signal it?
>
> I don't know exactly when the problem started. I am pretty sure that it
> worked with 4.16.2, but it failed with 4.16.9. Or maybe it is caused by
> upgrading the firmware of the controller, although the protocol seems
> identical.
>
>  drivers/hid/hid-steam.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index cb86cc834201..0422ec2b13d2 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -573,7 +573,7 @@ static bool steam_is_valve_interface(struct hid_device *hdev)
>
>  static int steam_client_ll_parse(struct hid_device *hdev)
>  {
> -       struct steam_device *steam = hid_get_drvdata(hdev);
> +       struct steam_device *steam = hdev->driver_data;
>
>         return hid_parse_report(hdev, steam->hdev->dev_rdesc,
>                         steam->hdev->dev_rsize);
> @@ -590,7 +590,7 @@ static void steam_client_ll_stop(struct hid_device *hdev)
>
>  static int steam_client_ll_open(struct hid_device *hdev)
>  {
> -       struct steam_device *steam = hid_get_drvdata(hdev);
> +       struct steam_device *steam = hdev->driver_data;
>         int ret;
>
>         ret = hid_hw_open(steam->hdev);
> @@ -605,7 +605,7 @@ static int steam_client_ll_open(struct hid_device *hdev)
>
>  static void steam_client_ll_close(struct hid_device *hdev)
>  {
> -       struct steam_device *steam = hid_get_drvdata(hdev);
> +       struct steam_device *steam = hdev->driver_data;
>
>         mutex_lock(&steam->mutex);
>         steam->client_opened = false;
> @@ -623,7 +623,7 @@ static int steam_client_ll_raw_request(struct hid_device *hdev,
>                                 size_t count, unsigned char report_type,
>                                 int reqtype)
>  {
> -       struct steam_device *steam = hid_get_drvdata(hdev);
> +       struct steam_device *steam = hdev->driver_data;
>
>         return hid_hw_raw_request(steam->hdev, reportnum, buf, count,
>                         report_type, reqtype);
> @@ -710,7 +710,7 @@ static int steam_probe(struct hid_device *hdev,
>                 ret = PTR_ERR(steam->client_hdev);
>                 goto client_hdev_fail;
>         }
> -       hid_set_drvdata(steam->client_hdev, steam);
> +       steam->client_hdev->driver_data = steam;
>
>         /*
>          * With the real steam controller interface, do not connect hidraw.
> --
> 2.17.0
>

View attachment "steam_backtrace.txt" of type "text/plain" (6158 bytes)