Re: [PATCH] HID: steam: use hid_device.driver_data instead of hid_set_drvdata()

From: Mariusz Ceier
Date: Sat Jun 09 2018 - 17:30:32 EST

Next message: Mikulas Patocka: "[PATCH] hpfs: Use EUCLEAN for filesystem errors"
Previous message: Adam Borowski: "Re: [PATCH 04/24] 32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option"
In reply to: Jiri Kosina: "Re: [PATCH] HID: steam: use hid_device.driver_data instead of hid_set_drvdata()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxx>

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@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>
> 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
>
[ 4004.866471] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
[ 4004.866475] PGD 800000086ade5067 P4D 800000086ade5067 PUD 0
[ 4004.866479] Oops: 0000 [#1] PREEMPT SMP PTI
[ 4004.866481] CPU: 2 PID: 4884 Comm: CSteamControlle Tainted: G W 4.17.0-amdgpu-dev+ #1
[ 4004.866482] Hardware name: Gigabyte Technology Co., Ltd. Z170X-Gaming 7/Z170X-Gaming 7, BIOS F22j 01/11/2018
[ 4004.866486] RIP: 0010:steam_client_ll_open+0x17/0x50
[ 4004.866487] Code: e8 9e cb e0 ff 4c 89 e7 e8 d6 6a ff ff eb c0 0f 1f 40 00 e8 1b 09 26 00 55 48 89 e5 41 54 53 48 83 ec 08 48 8b 9f 68 19 00 00 <48> 8b 7b 18 e8 40 6a ff ff 85 c0 75 1e 4c 8d 63 28 89 45 ec 4c 89
[ 4004.866510] RSP: 0018:ffffc0a841a5bb18 EFLAGS: 00010296
[ 4004.866511] RAX: ffffffffbdba0d50 RBX: 0000000000000000 RCX: 0000000000000001
[ 4004.866512] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9c77752ee000
[ 4004.866513] RBP: ffffc0a841a5bb30 R08: ffff9c777a002d80 R09: ffff9c776b169800
[ 4004.866514] R10: ffff9c7779a1d025 R11: 0034776172646968 R12: ffff9c77752efba8
[ 4004.866515] R13: ffff9c776d256e00 R14: ffff9c7774c2b0c0 R15: ffffffffbd414750
[ 4004.866517] FS: 0000000000000000(0000) GS:ffff9c779dd00000(0063) knlGS:00000000e65cab40
[ 4004.866518] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 4004.866519] CR2: 0000000000000018 CR3: 0000000864614004 CR4: 00000000003606e0
[ 4004.866521] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 4004.866522] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 4004.866522] Call Trace:
[ 4004.866526] ? kmem_cache_alloc_trace+0x169/0x1c0
[ 4004.866528] hid_hw_open+0x4c/0x70
[ 4004.866530] hidraw_open+0xae/0x1d0
[ 4004.866532] ? cdev_put.part.1+0x20/0x20
[ 4004.866534] chrdev_open+0xa6/0x1c0
[ 4004.866536] do_dentry_open+0x1bd/0x2f0
[ 4004.866538] vfs_open+0x4f/0x80
[ 4004.866540] do_last+0x4da/0x1240
[ 4004.866542] ? inode_permission+0x54/0x190
[ 4004.866544] ? path_init+0x19c/0x330
[ 4004.866546] path_openat+0xa0/0x300
[ 4004.866548] ? putname+0x4c/0x60
[ 4004.866550] do_filp_open+0x9b/0x110
[ 4004.866552] ? __check_object_size+0xa4/0x1b0
[ 4004.866554] do_sys_open+0x1ba/0x250
[ 4004.866556] ? do_filp_open+0x5/0x110
[ 4004.866558] ? do_sys_open+0x1ba/0x250
[ 4004.866560] __ia32_compat_sys_openat+0x1d/0x20
[ 4004.866562] do_fast_syscall_32+0xb1/0x2c0
[ 4004.866565] entry_SYSENTER_compat+0x84/0x96
[ 4004.866566] RIP: 0023:0xf7f79c59
[ 4004.866567] Code: 85 d2 74 02 89 0a 5b 5d c3 8b 04 24 c3 8b 0c 24 c3 8b 1c 24 c3 8b 3c 24 c3 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
[ 4004.866589] RSP: 002b:00000000e65c88a0 EFLAGS: 00000282 ORIG_RAX: 0000000000000127
[ 4004.866591] RAX: ffffffffffffffda RBX: 00000000ffffff9c RCX: 00000000ee8650fc
[ 4004.866592] RDX: 0000000000000002 RSI: 0000000000000000 RDI: 00000000f7c11000
[ 4004.866593] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[ 4004.866594] R10: 0000000000000000 R11: 0000000000000282 R12: 0000000000000000
[ 4004.866595] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 4004.866596] Modules linked in: rpcsec_gss_krb5 x86_pkg_temp_thermal kvm_intel kvm snd_hda_codec_hdmi snd_hda_codec_ca0132 snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep irqbypass snd_pcm crc32c_intel ghash_clmulni_intel mxm_wmi snd_timer cryptd alx wmi snd video soundcore coretemp efivarfs dm_zero dm_thin_pool dm_persistent_data dm_bio_prison dm_log_userspace dm_flakey dm_delay virtio_pci virtio_scsi virtio_blk virtio_console virtio_balloon sha512_generic libiscsi scsi_transport_iscsi tulip cxgb3 cxgb cxgb4 vxge bonding vxlan ip6_udp_tunnel udp_tunnel macvlan vmxnet3 virtio_net net_failover failover virtio_ring virtio tg3 libphy sky2 r8169 pcnet32 mii e1000 bnx2 atl1c fuse xfs jfs reiserfs btrfs zstd_decompress zstd_compress xxhash linear raid10 raid1 raid0 dm_raid raid456 async_raid6_recov async_memcpy
[ 4004.866634] async_pq async_xor async_tx xor raid6_pq dm_mirror dm_region_hash dm_log firewire_core crc_itu_t sl811_hcd usb_storage aic94xx libsas lpfc qla2xxx megaraid_sas megaraid_mbox megaraid_mm aacraid sx8 hpsa 3w_9xxx 3w_xxxx mptsas scsi_transport_sas mptfc scsi_transport_fc mptspi mptscsih mptbase sym53c8xx initio arcmsr aic7xxx aic79xx scsi_transport_spi sr_mod cdrom sg pdc_adma sata_inic162x sata_mv ata_piix sata_qstor sata_vsc sata_uli sata_sis sata_sx4 sata_nv sata_via sata_svw sata_sil24 sata_sil sata_promise pata_via pata_jmicron pata_marvell pata_sis pata_netcell pata_pdc202xx_old pata_atiixp pata_amd pata_ali pata_it8213 pata_pcmcia pata_serverworks pata_oldpiix pata_artop pata_it821x pata_hpt3x2n pata_hpt3x3 pata_hpt37x pata_hpt366 pata_cmd64x pata_sil680 pata_pdc2027x sd_mod ahci
[ 4004.866669] libahci
[ 4004.866671] CR2: 0000000000000018
[ 4004.866673] ---[ end trace 9c22eadca9234307 ]---
[ 4004.907301] snd_hda_intel 0000:00:1f.3: Unstable LPIB (351688 >= 176400); disabling LPIB delay counting
[ 4004.963375] RIP: 0010:steam_client_ll_open+0x17/0x50
[ 4004.963378] Code: e8 9e cb e0 ff 4c 89 e7 e8 d6 6a ff ff eb c0 0f 1f 40 00 e8 1b 09 26 00 55 48 89 e5 41 54 53 48 83 ec 08 48 8b 9f 68 19 00 00 <48> 8b 7b 18 e8 40 6a ff ff 85 c0 75 1e 4c 8d 63 28 89 45 ec 4c 89
[ 4004.963404] RSP: 0018:ffffc0a841a5bb18 EFLAGS: 00010296
[ 4004.963405] RAX: ffffffffbdba0d50 RBX: 0000000000000000 RCX: 0000000000000001
[ 4004.963407] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9c77752ee000
[ 4004.963408] RBP: ffffc0a841a5bb30 R08: ffff9c777a002d80 R09: ffff9c776b169800
[ 4004.963409] R10: ffff9c7779a1d025 R11: 0034776172646968 R12: ffff9c77752efba8
[ 4004.963410] R13: ffff9c776d256e00 R14: ffff9c7774c2b0c0 R15: ffffffffbd414750
[ 4004.963412] FS: 0000000000000000(0000) GS:ffff9c779dd00000(0063) knlGS:00000000e65cab40
[ 4004.963413] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 4004.963414] CR2: 0000000000000018 CR3: 0000000864614005 CR4: 00000000003606e0
[ 4004.963415] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 4004.963416] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400

Next message: Mikulas Patocka: "[PATCH] hpfs: Use EUCLEAN for filesystem errors"
Previous message: Adam Borowski: "Re: [PATCH 04/24] 32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option"
In reply to: Jiri Kosina: "Re: [PATCH] HID: steam: use hid_device.driver_data instead of hid_set_drvdata()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]