From patchwork Wed Oct 16 18:29:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrey Smirnov X-Patchwork-Id: 11193901 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3EE2A15AB for ; Wed, 16 Oct 2019 18:30:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0535F21D7E for ; Wed, 16 Oct 2019 18:30:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="f6px+ONp" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2436534AbfJPS35 (ORCPT ); Wed, 16 Oct 2019 14:29:57 -0400 Received: from mail-pg1-f194.google.com ([209.85.215.194]:33274 "EHLO mail-pg1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2403921AbfJPS35 (ORCPT ); Wed, 16 Oct 2019 14:29:57 -0400 Received: by mail-pg1-f194.google.com with SMTP id i76so14791054pgc.0; Wed, 16 Oct 2019 11:29:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=u+oHnKpzVYG/PGPYj/u8ShRJvcNJAvZMRRYNdxrQSLg=; b=f6px+ONpvWjnEeOZdpvCgNIGtaA8EX1xJWBNw/n6U5DTQU8rnq/ayh2tTYLl64sYLL 5XgqdS3wWs+Pyd3JjLl4ABHUkuEqUDmsBo1GY4OT21sG7Uc28S8IkYzUUJnots4fZIfa qMOFNhh6jHV872Cps073NK92+i8QkCxlEN7hOxw51VjANB/eYEf+8w6kShGcRfQS/vAq QLCBhT4x4AtT0GLZHR5WqKWEXvk0LAu2CK6AHlUaWn9P7WvhRwObCcrJlWFdxDRH9fGx 5fjtMsUPc5kvabeeRt2NiZLMt5V3QuEjVc6W1za9Tewd0FnaVGqus/79ZcudPKW6gNrK 3dlg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=u+oHnKpzVYG/PGPYj/u8ShRJvcNJAvZMRRYNdxrQSLg=; b=KdJMcSP3+yj9ZrvAkMBFfIiYFUCogKHjYvmYg8/ATfM+hK2eTK2nTLWOQNo352/OWq djHL3BoC9SrC7AyDD91TJiDZHX9gN5YS9Ip4PwkIOJljmcqcFqXNHkJUL8yBfqTHddbq sfAIL2ExQG1fZG7jaiPOMsl45HnOydbhVDm4jqRcvwPr02xGAzoBOO1i05oddAxw8VIy AM1RIjT/oqS11SiXhASGSVs0/16aBc/NmSM4IWasMCtQ+0N3RhKOJwh20NT2zhJPNr5x 7P+mDfL86t5csaCIU38KNJ3XOZ3wat6zK5Z0CHdXh2IHc0wFBmYgX5BK9djMpJAbTIny tIgw== X-Gm-Message-State: APjAAAViRTJ42XwMWuY4WpkrqENfqp8WX7GrAoC25OjEx0OX/05rBYZ3 Nm+JNKGz7m/QYxuPQPnVB9KMvG8AAFY= X-Google-Smtp-Source: APXvYqwI5cmug9uN1uCMQl4R2GsiUNQM2UyeYn9YomlL5lYbi8BOViz1pkD6q8APRxUUbkI9Joed8A== X-Received: by 2002:a63:9d49:: with SMTP id i70mr28478129pgd.120.1571250595533; Wed, 16 Oct 2019 11:29:55 -0700 (PDT) Received: from localhost.localdomain ([2607:fb90:8061:1b46:c3ff:7f12:48ed:6323]) by smtp.gmail.com with ESMTPSA id z4sm2980127pjt.17.2019.10.16.11.29.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Oct 2019 11:29:54 -0700 (PDT) From: Andrey Smirnov To: linux-input@vger.kernel.org Cc: Andrey Smirnov , Sam Bazley , Jiri Kosina , Benjamin Tissoires , Henrik Rydberg , "Pierre-Loup A . Griffais" , Austin Palmer , linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH v2 1/3] HID: logitech-hidpp: split g920_get_config() Date: Wed, 16 Oct 2019 11:29:33 -0700 Message-Id: <20191016182935.5616-2-andrew.smirnov@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191016182935.5616-1-andrew.smirnov@gmail.com> References: <20191016182935.5616-1-andrew.smirnov@gmail.com> MIME-Version: 1.0 Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org Original version of g920_get_config() contained two kind of actions: 1. Device specific communication to query/set some parameters which requires active communication channel with the device, or, put in other way, for the call to be sandwiched between hid_device_io_start() and hid_device_io_stop(). 2. Input subsystem specific FF controller initialization which, in order to access a valid 'struct hid_input' via 'hid->inputs.next', requires claimed hidinput which means be executed after the call to hid_hw_start() with connect_mask containing HID_CONNECT_HIDINPUT. Location of g920_get_config() can only fulfill requirements for #1 and not #2, which might result in following backtrace: [ 88.312258] logitech-hidpp-device 0003:046D:C262.0005: HID++ 4.2 device connected. [ 88.320298] BUG: kernel NULL pointer dereference, address: 0000000000000018 [ 88.320304] #PF: supervisor read access in kernel mode [ 88.320307] #PF: error_code(0x0000) - not-present page [ 88.320309] PGD 0 P4D 0 [ 88.320315] Oops: 0000 [#1] SMP PTI [ 88.320320] CPU: 1 PID: 3080 Comm: systemd-udevd Not tainted 5.4.0-rc1+ #31 [ 88.320322] Hardware name: Apple Inc. MacBookPro11,1/Mac-189A3D4F975D5FFC, BIOS 149.0.0.0.0 09/17/2018 [ 88.320334] RIP: 0010:hidpp_probe+0x61f/0x948 [hid_logitech_hidpp] [ 88.320338] Code: 81 00 00 48 89 ef e8 f0 d6 ff ff 41 89 c6 85 c0 75 b5 0f b6 44 24 28 48 8b 5d 00 88 44 24 1e 89 44 24 0c 48 8b 83 18 1c 00 00 <48> 8b 48 18 48 8b 83 10 19 00 00 48 8b 40 40 48 89 0c 24 0f b7 80 [ 88.320341] RSP: 0018:ffffb0a6824aba68 EFLAGS: 00010246 [ 88.320345] RAX: 0000000000000000 RBX: ffff93a50756e000 RCX: 0000000000010408 [ 88.320347] RDX: 0000000000000000 RSI: ffff93a51f0ad0a0 RDI: 000000000002d0a0 [ 88.320350] RBP: ffff93a50416da28 R08: ffff93a50416da70 R09: ffff93a50416da70 [ 88.320352] R10: 000000148ae9e60c R11: 00000000000f1525 R12: ffff93a50756e000 [ 88.320354] R13: ffff93a50756f8d0 R14: 0000000000000000 R15: ffff93a50756fc38 [ 88.320358] FS: 00007f8d8c1e0940(0000) GS:ffff93a51f080000(0000) knlGS:0000000000000000 [ 88.320361] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 88.320363] CR2: 0000000000000018 CR3: 00000003996d8003 CR4: 00000000001606e0 [ 88.320366] Call Trace: [ 88.320377] ? _cond_resched+0x15/0x30 [ 88.320387] ? create_pinctrl+0x2f/0x3c0 [ 88.320393] ? kernfs_link_sibling+0x94/0xe0 [ 88.320398] ? _cond_resched+0x15/0x30 [ 88.320402] ? kernfs_activate+0x5f/0x80 [ 88.320406] ? kernfs_add_one+0xe2/0x130 [ 88.320411] hid_device_probe+0x106/0x170 [ 88.320419] really_probe+0x147/0x3c0 [ 88.320424] driver_probe_device+0xb6/0x100 [ 88.320428] device_driver_attach+0x53/0x60 [ 88.320433] __driver_attach+0x8a/0x150 [ 88.320437] ? device_driver_attach+0x60/0x60 [ 88.320440] bus_for_each_dev+0x78/0xc0 [ 88.320445] bus_add_driver+0x14d/0x1f0 [ 88.320450] driver_register+0x6c/0xc0 [ 88.320453] ? 0xffffffffc0d67000 [ 88.320457] __hid_register_driver+0x4c/0x80 [ 88.320464] do_one_initcall+0x46/0x1f4 [ 88.320469] ? _cond_resched+0x15/0x30 [ 88.320474] ? kmem_cache_alloc_trace+0x162/0x220 [ 88.320481] ? do_init_module+0x23/0x230 [ 88.320486] do_init_module+0x5c/0x230 [ 88.320491] load_module+0x26e1/0x2990 [ 88.320502] ? ima_post_read_file+0xf0/0x100 [ 88.320508] ? __do_sys_finit_module+0xaa/0x110 [ 88.320512] __do_sys_finit_module+0xaa/0x110 [ 88.320520] do_syscall_64+0x5b/0x180 [ 88.320525] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 88.320528] RIP: 0033:0x7f8d8d1f01fd [ 88.320532] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 5b 8c 0c 00 f7 d8 64 89 01 48 [ 88.320535] RSP: 002b:00007ffefa3bb068 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 88.320539] RAX: ffffffffffffffda RBX: 000055922040cb40 RCX: 00007f8d8d1f01fd [ 88.320541] RDX: 0000000000000000 RSI: 00007f8d8ce4984d RDI: 0000000000000006 [ 88.320543] RBP: 0000000000020000 R08: 0000000000000000 R09: 0000000000000007 [ 88.320545] R10: 0000000000000006 R11: 0000000000000246 R12: 00007f8d8ce4984d [ 88.320547] R13: 0000000000000000 R14: 000055922040efc0 R15: 000055922040cb40 [ 88.320551] Modules linked in: hid_logitech_hidpp(+) fuse rfcomm ccm xt_CHECKSUM xt_MASQUERADE bridge stp llc nf_nat_tftp nf_conntrack_tftp nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_nat ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat tun iptable_mangle iptable_raw iptable_security nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables cmac bnep sunrpc dm_crypt nls_utf8 hfsplus intel_rapl_msr intel_rapl_common ath9k_htc ath9k_common x86_pkg_temp_thermal intel_powerclamp b43 ath9k_hw coretemp snd_hda_codec_hdmi cordic kvm_intel snd_hda_codec_cirrus mac80211 snd_hda_codec_generic ledtrig_audio kvm snd_hda_intel snd_intel_nhlt irqbypass snd_hda_codec btusb btrtl snd_hda_core ath btbcm ssb snd_hwdep btintel snd_seq crct10dif_pclmul iTCO_wdt snd_seq_device crc32_pclmul bluetooth mmc_core iTCO_vendor_support joydev cfg80211 [ 88.320602] applesmc ghash_clmulni_intel ecdh_generic snd_pcm input_polldev intel_cstate ecc intel_uncore thunderbolt snd_timer i2c_i801 libarc4 rfkill intel_rapl_perf lpc_ich mei_me pcspkr bcm5974 snd bcma mei soundcore acpi_als sbs kfifo_buf sbshc industrialio apple_bl i915 i2c_algo_bit drm_kms_helper drm uas crc32c_intel usb_storage video hid_apple [ 88.320630] CR2: 0000000000000018 [ 88.320633] ---[ end trace 933491c8a4fadeb7 ]--- [ 88.320642] RIP: 0010:hidpp_probe+0x61f/0x948 [hid_logitech_hidpp] [ 88.320645] Code: 81 00 00 48 89 ef e8 f0 d6 ff ff 41 89 c6 85 c0 75 b5 0f b6 44 24 28 48 8b 5d 00 88 44 24 1e 89 44 24 0c 48 8b 83 18 1c 00 00 <48> 8b 48 18 48 8b 83 10 19 00 00 48 8b 40 40 48 89 0c 24 0f b7 80 [ 88.320647] RSP: 0018:ffffb0a6824aba68 EFLAGS: 00010246 [ 88.320650] RAX: 0000000000000000 RBX: ffff93a50756e000 RCX: 0000000000010408 [ 88.320652] RDX: 0000000000000000 RSI: ffff93a51f0ad0a0 RDI: 000000000002d0a0 [ 88.320655] RBP: ffff93a50416da28 R08: ffff93a50416da70 R09: ffff93a50416da70 [ 88.320657] R10: 000000148ae9e60c R11: 00000000000f1525 R12: ffff93a50756e000 [ 88.320659] R13: ffff93a50756f8d0 R14: 0000000000000000 R15: ffff93a50756fc38 [ 88.320662] FS: 00007f8d8c1e0940(0000) GS:ffff93a51f080000(0000) knlGS:0000000000000000 [ 88.320664] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 88.320667] CR2: 0000000000000018 CR3: 00000003996d8003 CR4: 00000000001606e0 To solve this issue: 1. Split g920_get_config() such that all of the device specific communication remains a part of the function and input subsystem initialization bits go to hidpp_ff_init() 2. Move call to hidpp_ff_init() from being a part of g920_get_config() to be the last step of .probe(), right after a call to hid_hw_start() with connect_mask containing HID_CONNECT_HIDINPUT. Fixes: 91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable") Signed-off-by: Andrey Smirnov Tested-by: Sam Bazley Cc: Jiri Kosina Cc: Benjamin Tissoires Cc: Henrik Rydberg Cc: Pierre-Loup A. Griffais Cc: Austin Palmer Cc: linux-input@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: stable@vger.kernel.org # 5.2+ --- drivers/hid/hid-logitech-hidpp.c | 150 ++++++++++++++++++++----------- 1 file changed, 96 insertions(+), 54 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 1ac1ecc1e67c..85911586b3b6 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -1669,6 +1669,7 @@ static void hidpp_touchpad_raw_xy_event(struct hidpp_device *hidpp_dev, #define HIDPP_FF_EFFECTID_NONE -1 #define HIDPP_FF_EFFECTID_AUTOCENTER -2 +#define HIDPP_AUTOCENTER_PARAMS_LENGTH 18 #define HIDPP_FF_MAX_PARAMS 20 #define HIDPP_FF_RESERVED_SLOTS 1 @@ -2009,7 +2010,7 @@ static int hidpp_ff_erase_effect(struct input_dev *dev, int effect_id) static void hidpp_ff_set_autocenter(struct input_dev *dev, u16 magnitude) { struct hidpp_ff_private_data *data = dev->ff->private; - u8 params[18]; + u8 params[HIDPP_AUTOCENTER_PARAMS_LENGTH]; dbg_hid("Setting autocenter to %d.\n", magnitude); @@ -2081,7 +2082,8 @@ static void hidpp_ff_destroy(struct ff_device *ff) kfree(data->effect_ids); } -static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index) +static int hidpp_ff_init(struct hidpp_device *hidpp, + struct hidpp_ff_private_data *data) { struct hid_device *hid = hidpp->hid_dev; struct hid_input *hidinput; @@ -2089,9 +2091,7 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index) const struct usb_device_descriptor *udesc = &(hid_to_usb_dev(hid)->descriptor); const u16 bcdDevice = le16_to_cpu(udesc->bcdDevice); struct ff_device *ff; - struct hidpp_report response; - struct hidpp_ff_private_data *data; - int error, j, num_slots; + int error, j, num_slots = data->num_effects; u8 version; if (list_empty(&hid->inputs)) { @@ -2116,27 +2116,17 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index) for (j = 0; hidpp_ff_effects_v2[j] >= 0; j++) set_bit(hidpp_ff_effects_v2[j], dev->ffbit); - /* Read number of slots available in device */ - error = hidpp_send_fap_command_sync(hidpp, feature_index, - HIDPP_FF_GET_INFO, NULL, 0, &response); - if (error) { - if (error < 0) - return error; - hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n", - __func__, error); - return -EPROTO; - } - - num_slots = response.fap.params[0] - HIDPP_FF_RESERVED_SLOTS; - error = input_ff_create(dev, num_slots); if (error) { hid_err(dev, "Failed to create FF device!\n"); return error; } - - data = kzalloc(sizeof(*data), GFP_KERNEL); + /* + * Create a copy of passed data, so we can transfer memory + * ownership to FF core + */ + data = kmemdup(data, sizeof(*data), GFP_KERNEL); if (!data) return -ENOMEM; data->effect_ids = kcalloc(num_slots, sizeof(int), GFP_KERNEL); @@ -2152,10 +2142,7 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index) } data->hidpp = hidpp; - data->feature_index = feature_index; data->version = version; - data->slot_autocenter = 0; - data->num_effects = num_slots; for (j = 0; j < num_slots; j++) data->effect_ids[j] = -1; @@ -2169,37 +2156,14 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index) ff->set_autocenter = hidpp_ff_set_autocenter; ff->destroy = hidpp_ff_destroy; - - /* reset all forces */ - error = hidpp_send_fap_command_sync(hidpp, feature_index, - HIDPP_FF_RESET_ALL, NULL, 0, &response); - - /* Read current Range */ - error = hidpp_send_fap_command_sync(hidpp, feature_index, - HIDPP_FF_GET_APERTURE, NULL, 0, &response); - if (error) - hid_warn(hidpp->hid_dev, "Failed to read range from device!\n"); - data->range = error ? 900 : get_unaligned_be16(&response.fap.params[0]); - /* Create sysfs interface */ error = device_create_file(&(hidpp->hid_dev->dev), &dev_attr_range); if (error) hid_warn(hidpp->hid_dev, "Unable to create sysfs interface for \"range\", errno %d!\n", error); - /* Read the current gain values */ - error = hidpp_send_fap_command_sync(hidpp, feature_index, - HIDPP_FF_GET_GLOBAL_GAINS, NULL, 0, &response); - if (error) - hid_warn(hidpp->hid_dev, "Failed to read gain values from device!\n"); - data->gain = error ? 0xffff : get_unaligned_be16(&response.fap.params[0]); - /* ignore boost value at response.fap.params[2] */ - /* init the hardware command queue */ atomic_set(&data->workqueue_size, 0); - /* initialize with zero autocenter to get wheel in usable state */ - hidpp_ff_set_autocenter(dev, 0); - hid_info(hid, "Force feedback support loaded (firmware release %d).\n", version); @@ -2732,24 +2696,93 @@ static int k400_connect(struct hid_device *hdev, bool connected) #define HIDPP_PAGE_G920_FORCE_FEEDBACK 0x8123 -static int g920_get_config(struct hidpp_device *hidpp) +static int g920_ff_set_autocenter(struct hidpp_device *hidpp, + struct hidpp_ff_private_data *data) { + struct hidpp_report response; + u8 params[HIDPP_AUTOCENTER_PARAMS_LENGTH] = { + [1] = HIDPP_FF_EFFECT_SPRING | HIDPP_FF_EFFECT_AUTOSTART, + }; + int ret; + + /* initialize with zero autocenter to get wheel in usable state */ + + dbg_hid("Setting autocenter to 0.\n"); + ret = hidpp_send_fap_command_sync(hidpp, data->feature_index, + HIDPP_FF_DOWNLOAD_EFFECT, + params, ARRAY_SIZE(params), + &response); + if (ret) + hid_warn(hidpp->hid_dev, "Failed to autocenter device!\n"); + else + data->slot_autocenter = response.fap.params[0]; + + return ret; +} + +static int g920_get_config(struct hidpp_device *hidpp, + struct hidpp_ff_private_data *data) +{ + struct hidpp_report response; u8 feature_type; - u8 feature_index; int ret; + memset(data, 0, sizeof(*data)); + /* Find feature and store for later use */ ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_G920_FORCE_FEEDBACK, - &feature_index, &feature_type); + &data->feature_index, &feature_type); if (ret) return ret; - ret = hidpp_ff_init(hidpp, feature_index); + /* Read number of slots available in device */ + ret = hidpp_send_fap_command_sync(hidpp, data->feature_index, + HIDPP_FF_GET_INFO, + NULL, 0, + &response); + if (ret) { + if (ret < 0) + return ret; + hid_err(hidpp->hid_dev, + "%s: received protocol error 0x%02x\n", __func__, ret); + return -EPROTO; + } + + data->num_effects = response.fap.params[0] - HIDPP_FF_RESERVED_SLOTS; + + /* reset all forces */ + ret = hidpp_send_fap_command_sync(hidpp, data->feature_index, + HIDPP_FF_RESET_ALL, + NULL, 0, + &response); if (ret) - hid_warn(hidpp->hid_dev, "Unable to initialize force feedback support, errno %d\n", - ret); + hid_warn(hidpp->hid_dev, "Failed to reset all forces!\n"); - return 0; + ret = hidpp_send_fap_command_sync(hidpp, data->feature_index, + HIDPP_FF_GET_APERTURE, + NULL, 0, + &response); + if (ret) { + hid_warn(hidpp->hid_dev, + "Failed to read range from device!\n"); + } + data->range = ret ? + 900 : get_unaligned_be16(&response.fap.params[0]); + + /* Read the current gain values */ + ret = hidpp_send_fap_command_sync(hidpp, data->feature_index, + HIDPP_FF_GET_GLOBAL_GAINS, + NULL, 0, + &response); + if (ret) + hid_warn(hidpp->hid_dev, + "Failed to read gain values from device!\n"); + data->gain = ret ? + 0xffff : get_unaligned_be16(&response.fap.params[0]); + + /* ignore boost value at response.fap.params[2] */ + + return g920_ff_set_autocenter(hidpp, data); } /* -------------------------------------------------------------------------- */ @@ -3512,6 +3545,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id) int ret; bool connected; unsigned int connect_mask = HID_CONNECT_DEFAULT; + struct hidpp_ff_private_data data; /* report_fixup needs drvdata to be set before we call hid_parse */ hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL); @@ -3621,7 +3655,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id) if (ret) goto hid_hw_init_fail; } else if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)) { - ret = g920_get_config(hidpp); + ret = g920_get_config(hidpp, &data); if (ret) goto hid_hw_init_fail; } @@ -3643,6 +3677,14 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id) goto hid_hw_start_fail; } + if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) { + ret = hidpp_ff_init(hidpp, &data); + if (ret) + hid_warn(hidpp->hid_dev, + "Unable to initialize force feedback support, errno %d\n", + ret); + } + return ret; hid_hw_init_fail: From patchwork Wed Oct 16 18:29:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrey Smirnov X-Patchwork-Id: 11193897 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 45C7A15AB for ; Wed, 16 Oct 2019 18:30:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1C1A5222C6 for ; Wed, 16 Oct 2019 18:30:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="O9YFnGz7" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404890AbfJPS37 (ORCPT ); Wed, 16 Oct 2019 14:29:59 -0400 Received: from mail-pl1-f193.google.com ([209.85.214.193]:45609 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2403921AbfJPS36 (ORCPT ); Wed, 16 Oct 2019 14:29:58 -0400 Received: by mail-pl1-f193.google.com with SMTP id u12so11641954pls.12; Wed, 16 Oct 2019 11:29:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=hvcS0GPGgj0yc1Pyi9HzxvjjgsKfH8J62dj1ItCzLg0=; b=O9YFnGz7PNUveGDRZlRQ50ShQGtpWUW9JC8cBMl6BNssEmQwHgnkpTTI+8S3DJzhHe qfVQT/idJsPUmBI9y9aN2XK8fh7VnBSJ51dEUdNv2snHKL+09VVDmT6OqI5eetEyuMrT 1aEDexWWxqCW1rF38wXuVukBpGpzs/r2fVo3zrDOvttuFZ5qwOW7x116Lvwwe8JoBm13 3+1cwReNV51UY+z4pQPrYwY5Mp7EL2ZaTXNecKXoGuxn3Vkuy5g6GXB4gx7tKqI6rBAz vEqYRY38xKhQb554C/xsK69vItEHG3ix39FIgReJ3j1r9szBw+5VBqsSER6R9cUgcYh0 sR7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=hvcS0GPGgj0yc1Pyi9HzxvjjgsKfH8J62dj1ItCzLg0=; b=TX2iOjYsxdOpXCCt6HBhvCx/IJLzZSd9StmP/5GHeXXufUi0iW/3D44zyjSHmD+HdE d6unLBdBtYECTEwjzWT+Kmx/Twme+mbn3ScM5/39VyR6hfdvkb5C7PbsszUAAfGwpGoq GxlB6vBj8+bSeJ0naQ+X2UngB8Y5iTQgz+pzPfuTswGTDtT0qFUupiFzkNKt1O4oDgXb sWDwfDk87yY1XHoBVk8qpCouGadEPWGdCOKAMnfEAiRZvGXwUFAaevv/NOfFhTE4QcA9 f5rGlwCX0usWQGZoajS8zj+hB3w59XmGKaUWuwsC/5NvLu5bem3TV6dQQqa2VMHA/m8h TFew== X-Gm-Message-State: APjAAAVjY70mXGbaBeaNgFClp3oEBSz3E5WyUsvK034Iv61TfzwMd5mZ DsdwGHHPLG1XlTiqirlyUFm+yxaYIVE= X-Google-Smtp-Source: APXvYqy7y9X98pDMfQf/ZDPQdpI8Cn4yAww0OZiX8VwOa4QxZUaa9I7EmiMLs61UoEVnfVT0CtWcAA== X-Received: by 2002:a17:902:bd47:: with SMTP id b7mr42701792plx.28.1571250597611; Wed, 16 Oct 2019 11:29:57 -0700 (PDT) Received: from localhost.localdomain ([2607:fb90:8061:1b46:c3ff:7f12:48ed:6323]) by smtp.gmail.com with ESMTPSA id z4sm2980127pjt.17.2019.10.16.11.29.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Oct 2019 11:29:56 -0700 (PDT) From: Andrey Smirnov To: linux-input@vger.kernel.org Cc: Andrey Smirnov , Sam Bazely , Jiri Kosina , Benjamin Tissoires , Henrik Rydberg , "Pierre-Loup A . Griffais" , Austin Palmer , linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH v2 2/3] HID: logitech-hidpp: rework device validation Date: Wed, 16 Oct 2019 11:29:34 -0700 Message-Id: <20191016182935.5616-3-andrew.smirnov@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191016182935.5616-1-andrew.smirnov@gmail.com> References: <20191016182935.5616-1-andrew.smirnov@gmail.com> MIME-Version: 1.0 Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org G920 device only advertises REPORT_ID_HIDPP_LONG and REPORT_ID_HIDPP_VERY_LONG in its HID report descriptor, so querying for REPORT_ID_HIDPP_SHORT with optional=false will always fail and prevent G920 to be recognized as a valid HID++ device. To fix this and improve some other aspects, modify hidpp_validate_device() as follows: - Inline the code of hidpp_validate_report() to simplify distingushing between non-present and invalid report descriptors - Drop the check for id >= HID_MAX_IDS || id < 0 since all of our IDs are static and known to satisfy that at compile time - Change the algorithms to check all possible report types (including very long report) and deem the device as a valid HID++ device if it supports at least one - Treat invalid report length as a hard stop for the validation algorithm, meaning that if any of the supported reports has invalid length we assume the worst and treat the device as a generic HID device. - Fold initialization of hidpp->very_long_report_length into hidpp_validate_device() since it already fetches very long report length and validates its value Fixes: fe3ee1ec007b ("HID: logitech-hidpp: allow non HID++ devices to be handled by this module") Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204191 Reported-by: Sam Bazely Signed-off-by: Andrey Smirnov Cc: Jiri Kosina Cc: Benjamin Tissoires Cc: Henrik Rydberg Cc: Pierre-Loup A. Griffais Cc: Austin Palmer Cc: linux-input@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: stable@vger.kernel.org # 5.2+ --- drivers/hid/hid-logitech-hidpp.c | 54 ++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 85911586b3b6..8c4be991f387 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -3498,34 +3498,45 @@ static int hidpp_get_report_length(struct hid_device *hdev, int id) return report->field[0]->report_count + 1; } -static bool hidpp_validate_report(struct hid_device *hdev, int id, - int expected_length, bool optional) +static bool hidpp_validate_device(struct hid_device *hdev) { - int report_length; + struct hidpp_device *hidpp = hid_get_drvdata(hdev); + int id, report_length, supported_reports = 0; + + id = REPORT_ID_HIDPP_SHORT; + report_length = hidpp_get_report_length(hdev, id); + if (report_length) { + if (report_length < HIDPP_REPORT_SHORT_LENGTH) + goto bad_device; - if (id >= HID_MAX_IDS || id < 0) { - hid_err(hdev, "invalid HID report id %u\n", id); - return false; + supported_reports++; } + id = REPORT_ID_HIDPP_LONG; report_length = hidpp_get_report_length(hdev, id); - if (!report_length) - return optional; + if (report_length) { + if (report_length < HIDPP_REPORT_LONG_LENGTH) + goto bad_device; - if (report_length < expected_length) { - hid_warn(hdev, "not enough values in hidpp report %d\n", id); - return false; + supported_reports++; } - return true; -} + id = REPORT_ID_HIDPP_VERY_LONG; + report_length = hidpp_get_report_length(hdev, id); + if (report_length) { + if (report_length > HIDPP_REPORT_LONG_LENGTH && + report_length < HIDPP_REPORT_VERY_LONG_MAX_LENGTH) + goto bad_device; -static bool hidpp_validate_device(struct hid_device *hdev) -{ - return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT, - HIDPP_REPORT_SHORT_LENGTH, false) && - hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG, - HIDPP_REPORT_LONG_LENGTH, true); + supported_reports++; + hidpp->very_long_report_length = report_length; + } + + return supported_reports; + +bad_device: + hid_warn(hdev, "not enough values in hidpp report %d\n", id); + return false; } static bool hidpp_application_equals(struct hid_device *hdev, @@ -3572,11 +3583,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id) return hid_hw_start(hdev, HID_CONNECT_DEFAULT); } - hidpp->very_long_report_length = - hidpp_get_report_length(hdev, REPORT_ID_HIDPP_VERY_LONG); - if (hidpp->very_long_report_length > HIDPP_REPORT_VERY_LONG_MAX_LENGTH) - hidpp->very_long_report_length = HIDPP_REPORT_VERY_LONG_MAX_LENGTH; - if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE) hidpp->quirks |= HIDPP_QUIRK_UNIFYING; From patchwork Wed Oct 16 18:29:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrey Smirnov X-Patchwork-Id: 11193899 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 96B0915AB for ; Wed, 16 Oct 2019 18:30:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 76ED7218DE for ; Wed, 16 Oct 2019 18:30:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="RKtHCtJ1" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2436620AbfJPSaD (ORCPT ); Wed, 16 Oct 2019 14:30:03 -0400 Received: from mail-pl1-f194.google.com ([209.85.214.194]:34163 "EHLO mail-pl1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2436612AbfJPSaA (ORCPT ); Wed, 16 Oct 2019 14:30:00 -0400 Received: by mail-pl1-f194.google.com with SMTP id k7so11677582pll.1; Wed, 16 Oct 2019 11:30:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=4o2nRnrGmqJtA/J2E1Lis+BBBesb9H7tV/2Ne53kdSQ=; b=RKtHCtJ1HuSGd9wh2Thc2aoaLdL7DEMZoF2gkZE6URaDLFBN/TnL4wFXLUpvETEO/b zPtVUTCLIisvIfNnoLjBHDvYK4/jGJH1Lc2DPLWWYNh/7+oIr3MPDcsPS3JTBfICgTqy GgPhJ1Y+WSAO8wIIOK4VgbkHxLlSmp2XNyII6tYsZ2DWGni4Bn/9JAarMlToRcl3qBvO ilXwaKeQMzQygcMqUqtyi2q5mro42UK5oBOvYuLYMSXMVGrOcIe0pvuHeIr9X/SYK5Hp i3G6XKDg3lzge9ZRtIXBsHT4PuDe5X20SuK0JtGe7IoWmK5CN1ayHMVA0pKuz766TyCU pTSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=4o2nRnrGmqJtA/J2E1Lis+BBBesb9H7tV/2Ne53kdSQ=; b=ADiGI/ffUyxvelfnuAVzt/nioSXMntoK1ORHhFFgVs8PLSePM+EpAY3xQGCsGzMQQL /aykGnJ8WcpUZUenqbcYXPgA3omVhsNb82+ZwyNsFi9Ta1byo6l0gDdN6M5CuNKzSRHX ZjMLT6HlCd5NVhTrjmOe321VkATLUvCPjmigTaT9uHRfFEL0lwUPs7AUxjjUYKInuUde gOC8V56FKagSA9q0V0iMGP3RH84p1wxMzZrWJNC4g7+S4xNYNT3UnnWsmFZ0a76cJeAR tsux0hwRGYCxTAoI+9+x3EwLog2w52rj09CWCW8JCiEWp7SUp3dkF0UOSqoAm7g0NSgB rUbA== X-Gm-Message-State: APjAAAWxAd5U4alhEgNWJYWjQGgBpTAL0WYOQseRvVTMBp50obnkrtqT VFhuW1xjS3V5PlTk0Fgpx5g3p8ZGDiw= X-Google-Smtp-Source: APXvYqyc7LqnMdiRhMCp01pG2CYxy9bh7AF43xp1ON/ofqiZ/ejmNnhsXwvIUkUnV4EXPcoNHAZFtg== X-Received: by 2002:a17:902:778a:: with SMTP id o10mr41858030pll.93.1571250599437; Wed, 16 Oct 2019 11:29:59 -0700 (PDT) Received: from localhost.localdomain ([2607:fb90:8061:1b46:c3ff:7f12:48ed:6323]) by smtp.gmail.com with ESMTPSA id z4sm2980127pjt.17.2019.10.16.11.29.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Oct 2019 11:29:58 -0700 (PDT) From: Andrey Smirnov To: linux-input@vger.kernel.org Cc: Andrey Smirnov , Benjamin Tissoires , Jiri Kosina , Henrik Rydberg , "Pierre-Loup A . Griffais" , Austin Palmer , linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH v2 3/3] HID: logitech-hidpp: do all FF cleanup in hidpp_ff_destroy() Date: Wed, 16 Oct 2019 11:29:35 -0700 Message-Id: <20191016182935.5616-4-andrew.smirnov@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191016182935.5616-1-andrew.smirnov@gmail.com> References: <20191016182935.5616-1-andrew.smirnov@gmail.com> MIME-Version: 1.0 Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org All of the FF-related resources belong to corresponding FF device, so they should be freed as a part of hidpp_ff_destroy() to avoid potential race condidions. Fixes: ff21a635dd1a ("HID: logitech-hidpp: Force feedback support for the Logitech G920") Suggested-by: Benjamin Tissoires Signed-off-by: Andrey Smirnov Cc: Jiri Kosina Cc: Benjamin Tissoires Cc: Henrik Rydberg Cc: Pierre-Loup A. Griffais Cc: Austin Palmer Cc: linux-input@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: stable@vger.kernel.org # 4.9+ --- drivers/hid/hid-logitech-hidpp.c | 33 +++++--------------------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 8c4be991f387..2524b5104573 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -2078,7 +2078,12 @@ static DEVICE_ATTR(range, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH, hidpp static void hidpp_ff_destroy(struct ff_device *ff) { struct hidpp_ff_private_data *data = ff->private; + struct hid_device *hid = data->hidpp->hid_dev; + hid_info(hid, "Unloading HID++ force feedback.\n"); + + device_remove_file(&hid->dev, &dev_attr_range); + destroy_workqueue(data->wq); kfree(data->effect_ids); } @@ -2170,31 +2175,6 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, return 0; } -static int hidpp_ff_deinit(struct hid_device *hid) -{ - struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list); - struct input_dev *dev = hidinput->input; - struct hidpp_ff_private_data *data; - - if (!dev) { - hid_err(hid, "Struct input_dev not found!\n"); - return -EINVAL; - } - - hid_info(hid, "Unloading HID++ force feedback.\n"); - data = dev->ff->private; - if (!data) { - hid_err(hid, "Private data not found!\n"); - return -EINVAL; - } - - destroy_workqueue(data->wq); - device_remove_file(&hid->dev, &dev_attr_range); - - return 0; -} - - /* ************************************************************************** */ /* */ /* Device Support */ @@ -3713,9 +3693,6 @@ static void hidpp_remove(struct hid_device *hdev) sysfs_remove_group(&hdev->dev.kobj, &ps_attribute_group); - if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) - hidpp_ff_deinit(hdev); - hid_hw_stop(hdev); cancel_work_sync(&hidpp->work); mutex_destroy(&hidpp->send_mutex);