From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751180AbdJRR3B (ORCPT ); Wed, 18 Oct 2017 13:29:01 -0400 Received: from mail-bn3nam01on0068.outbound.protection.outlook.com ([104.47.33.68]:56112 "EHLO NAM01-BN3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750829AbdJRR27 (ORCPT ); Wed, 18 Oct 2017 13:28:59 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=aduggan@synaptics.com; Subject: Re: [PATCH] HID: rmi: Check that a device is a RMI device before calling RMI functions To: Jiri Kosina , Benjamin Tissoires Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Hendrik Langer References: <1508290656-18871-1-git-send-email-aduggan@synaptics.com> <20171018084803.GE24559@mail.corp.redhat.com> From: Andrew Duggan Message-ID: <37d50592-39b6-680f-6d02-1a7daa9a9208@synaptics.com> Date: Wed, 18 Oct 2017 10:28:45 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [73.202.48.75] X-ClientProxiedBy: SN4PR0501CA0059.namprd05.prod.outlook.com (10.167.112.164) To DM5PR03MB2587.namprd03.prod.outlook.com (10.168.196.141) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 1fe93812-e9fd-4024-4d93-08d5164db646 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(2017030254152)(2017052603199)(201703131423075)(201703031133081)(201702281549075);SRVR:DM5PR03MB2587; X-Microsoft-Exchange-Diagnostics: 1;DM5PR03MB2587;3:b2/LfmPzNr7NO6h3LH/h9wCi0nRT3DTAVKYYw4yEvmHIRFRTC0cCczrHL6Vj9vYEALqNxVRxri9wz6ObtPGUe2DeyRF32c+rqkH/4q75dYrpkrTiAZTVd6aMoEHQsAb3MwMf0Ubi07/x+jF/j0Nld1O5hyMS7WKqytJyqTqnRgzdRrFewqM8OMbZky0G5q75UqwO1R+xw+DTQqDEKafezvDndxcixKwiHbIm8rxF6cDFvOJuUn5ygGFXy7dcWgiF;25:zken4rJGrk9TQsnZMg+t4u3EnWfx1fz/crj5E/XvpN89rpAAwLLc4tiQIC+YAikUCutsXHdwH4Al4o9zopR1u3R6iJCMESArIbFrsgM4J8n7eHjR9DYWZMfS3XLbGh1etqBqYJj9fSUktUPEU4EePBa1SPwJ3pHIJU2e9PI+k6Ph05/4WRUdbaSMFo7P+26OAFDTE2hFPTWGi1Xk5xG2YuZAkPIK/kE9Iz6ajk7leJDP8KCAJhXsN+WK4nOI6DGRWUQSAcCmurgCS4qb3IGFi9UaTs1YW8UBs3RpJDPH8fC/N3xT2E0lJbFBLq3Qgz64uTn72rewRdTcpcIFIzrvfg==;31:AVChmLB2uQ79J1guV00rO15pO59tgr8FKvTdFUFsZYbIIsk76e0sSoyg/j+KH4n4IAjKuVZsWHD/9z5nKpJL0Cr7pK7dlDn2tHF7PBXUSn+zOKSXnb9qm7hXLFba33b4AeXWXZVoElqW/LUt3fp1v0ZjvX5EktzD+j1qiqnVTzkiVCAddsfRaBsXeV2Hxy7DfgsxzkHV9OpUP1OvoWywtLdAtZ0w/pgWobcw6hHP7so= X-MS-TrafficTypeDiagnostic: DM5PR03MB2587: X-Microsoft-Exchange-Diagnostics: 1;DM5PR03MB2587;20:jiHE5Q6kVAIfuPEJXc6/znQ8A+j6UJ1CZoBIH3zdPCuylDc+y/dDPhjXxhtDg4VjyNty6iz0a8IHTsm29PtM3aQ4XKciYfLLnzqn4nlNj3p8piy6fh0jMrdWw/0mJzUZH4/wtcniGAF/XWThUN9msgGva6DDmJaXgUlgTcvgOD11HVme2oR2OAwUq6e6526YVMljR7u2C0v1y6DmB3jztW7SrjGJao0FsFIFKgqgGvCucNsUZUGypNg9UgaL6Oirw1609dlzx9bvD80cBc+nwQdx3Jtw1MdNdCS5MlsF8iAPMcWAFK8c1dRJQ/Pn9deg3awAZWQE5tGw062A0wgZG6J2+S1kswWfSP3p5LY13VyZs3rhaAbYvqOXtfkzGjt6FtOBhW5xEmc7cHLtOQHMQOyvfZSd7wAdnGh9AFi/JzE=;4:mdRFq0u0/BbS3VAjfMf21pIgXe63C8+s22PdQUMSeS88tvSicie2NYaf/i2n8f2v3e32Lwa2PiXe2E0XckpFwIeijtSJTrIg77J2C2BZShJ6EYawZ3EZ9EzLnVCUOTqgyqxX7BsEYINbQrFtJ/NcdU8ydch4dnGGCXNTdejHF5fFKUQTEbjv8V65UVW5eF8XfE3vXdtc7k9EfxxafLCvxGtM/Q0AXTpZ0m/KFFU6gL8Cyu1KtvO1wH8+Js2VWxdh X-Exchange-Antispam-Report-Test: UriScan:; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(93006095)(93001095)(100000703101)(100105400095)(3002001)(10201501046)(6041248)(20161123555025)(20161123560025)(20161123564025)(20161123558100)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:DM5PR03MB2587;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:DM5PR03MB2587; X-Forefront-PRVS: 0464DBBBC4 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6049001)(6009001)(346002)(376002)(377454003)(52314003)(189002)(24454002)(199003)(54094003)(6486002)(97736004)(7736002)(16576012)(53546010)(83506001)(101416001)(4326008)(76176999)(110136005)(316002)(36756003)(6306002)(64126003)(50466002)(5890100001)(305945005)(54356999)(6116002)(6246003)(8936002)(8676002)(65826007)(58126008)(81156014)(25786009)(5660300001)(53936002)(8666007)(81166006)(68736007)(2906002)(66066001)(229853002)(65956001)(90366009)(77096006)(2950100002)(16526018)(3846002)(65806001)(478600001)(106356001)(50986999)(6666003)(47776003)(2870700001)(86362001)(31696002)(189998001)(966005)(33646002)(105586002)(23676002)(31686004);DIR:OUT;SFP:1101;SCL:1;SRVR:DM5PR03MB2587;H:[10.5.0.3];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtETTVQUjAzTUIyNTg3OzIzOjBFb0RFL3pPaFQ5M1o4OXRESHhINXljTUFZ?= =?utf-8?B?OEdDRHI2djFvcHNlTGd6RVRQdVIxUTFFZWtOSms1SEhFTjF4WjBpRWV2KzJl?= =?utf-8?B?NHFsY2lvSHZiV09aeFJ3Mkx5emRQeVUvbnJKZFp5N1FXWklZVnFNUUNmZCtU?= =?utf-8?B?c3R1RUxqVzd6OERsMnJrSGdlOTRncks0S2FuTWE3WjVPZFNsRHR3dHdXdnQ2?= =?utf-8?B?RE1hKzRmdGFRbUNRREh1bUQyMnFiNHE3dWJLdFJGUDVtdmRZZ3Y1bkdqVCtF?= =?utf-8?B?TWdRZUR2NDhkU2E2UGp6QU1QNFZaVy9PT0Q4TlBjdWQ4VitxYTdMOW5FT2dk?= =?utf-8?B?YXNqajhSaGlYaUJ6d1VkWUNwLzhhTnB0YnpJbk9CckpmU3FVWmd4Tk9URnh1?= =?utf-8?B?Z2FmRGZxK1k5VmZRczltYk5jTE5ycHdLWFNWY0xvZ2Fya0hIYUdmYXlydGVa?= =?utf-8?B?U2cxUWhCWFBDMnAzMmUxaGpabWZieWpZN3ZUSFVsNWxIR2VzTVZPeHpZRmQr?= =?utf-8?B?MXUxQnFMRUpGQ0JTT2RIN0RvcVZhTkNBUjltWWsvYjlINjRQWG82QTBpS1pR?= =?utf-8?B?QkJSOUhwN3gyK3lNVU8zY3RYaGZVeGwxVUtLY1VkOHJVT3B0SVd4dHdRY0sr?= =?utf-8?B?c1hDQjdDcHBoWTV2SFphc1c3ZyttYVE1TFB6dmpzelhGRyt6MGZjdTltV2cz?= =?utf-8?B?THVaM1Z3dWtSWXZkbVFTSW45bzVlWkluSWs3OEIvTHB3VDByYmRIUjF6bzZx?= =?utf-8?B?ektDQ1BUSzZ1R0xXd1FRRTdveERkMHAycDUwYTEwbCtjdmc4NDVyalFaREs2?= =?utf-8?B?bnF1TW1ob0RqOHIzalZRRFFXMFJNR1B6K29sRERLN2IyU3BEa1FHMlhtQVpn?= =?utf-8?B?TWxucVFyREExQmcrUTY3OGlld3ZDYS85S09pN0U0eTVaNWVBUDl4OW9CcUY2?= =?utf-8?B?RUNBTzZBWkRXWVFsdE54NDl0S2ZGNVBDVSt2NWtBbFdraXgray9TTlpvWXI2?= =?utf-8?B?bVFUUFFQWW9JMHdsbUNmOTJlTDgvQ2xnUDcreTRzMnZ6Wkl4SUkxYkF0M2lm?= =?utf-8?B?MjRncGVicXFucXZZT2RHdUI3NC8yWnFWRHFCN3Uyb0ZRTWJUQzJoL3dLZm5U?= =?utf-8?B?VFZQQUs3eFR3YlJkN1NmMUljeVd2Z2VEWmhsZ2pFYVd4SWtBYVB3SmIyU3Vo?= =?utf-8?B?UUZYTWhUajZCZU93eU1kVmg2WUtaem41akdZeGh1WklMUWJTVWYrYW9CUzVv?= =?utf-8?B?bUxyZkZVRGhkbWVjaXhIRllYTmxUb0kreGs4Z09tQzZ2eHR6eGhGVExzaGNF?= =?utf-8?B?UElMOFRiT1NRRklrbHdma1VBRzRySm9HWnZWbW5kdjVLQnQ5dEw4OGZTbkc1?= =?utf-8?B?Z2Q4Q2RxUmVVcG1GS2RnQnQ3cXNPZi9neXVXQ0NyWUVhc1pyUXlrNmhTS3ZS?= =?utf-8?B?MGdMSkJSdC94SnkwdHdXYWpLeUpCVEZpUjM1TVZYNDZzTGJKS2tEUHBRVjE1?= =?utf-8?B?WUF4Q1RPNnNhQ0JqMjJNTFdNVEFUN0RnL2JWMkM2SkFMendBZ3NQYitvd0NZ?= =?utf-8?B?ek5GbUk2RjlueGtQbVZlRU90bldEcnVob2l2NHMwTkgzQURpZUxGRVU4QVZm?= =?utf-8?B?U3pQdDNnd0liZFhFQjRmNld3dHBSTW5VNStidUZzb0pLejNKak5TTFpNTnVv?= =?utf-8?B?NzlxSlZISm96eEZrbE50d0hlWEZkcXVWSHQzMytRbVoyb05ac29WUURRd084?= =?utf-8?B?Q2gvRmtlQXJ2R3RVOU1VWVVnZTR3QysxUlhoQ3Z4TzBkMEN4SEpCMlo5OElC?= =?utf-8?B?Q1RIVE51enpZM2JFUmF5VEZoanZ5MG9qN2dIdDN1SXVuRU1QRHRlNTQyZTRE?= =?utf-8?B?UzBvdERvakJyc3psWU9ialpTL09CMVJNTGxOL1BzQUpPRnFsR3R0bmZFNXpB?= =?utf-8?B?K3o3aUNBUTBadFdxUURrSEZSZTR3Rlkvald6czB3NXF5SkxabFhDL05IaklQ?= =?utf-8?B?VmZyVVlKRFpXZ3NsWkVCKytZWjFpV25iNThqQT09?= X-Microsoft-Exchange-Diagnostics: 1;DM5PR03MB2587;6:JQPuFxn6WZf9b1j2CSIcsouc2pz0yDPUN1Pi6kG/wkIqwYqnjtoa//uBO7U754eIeOhxM2TI2paeuQVuPxiDptIUTfHjCAIDiiGDI5oRR173PKqRtQfxqIOvh5D+/94VsXmeo91rQ6zcPzM0QtwkvUkKLW8dF0QAGq9xZpgkJ8/0t5oX+XmsfUIjWmQCxHXrOtDHMU2a52m2S47p4YsAL9zhGfK40WgymiiCunLvQXUlWTotR/gbCAmwb2/kcYaFmwl77ZB31DF5puyQGMHE/HDH0CiUtqn+xaYJe0c+3Ee/bxEJ5dTYHSW8ctAFukx7sabn+AIcBcNbQLB1L5IOiQ==;5:ZK8KL0KUVZURlVdUPO3pIdWp4LU4cXGVAvlZ7c5uowE9OkkNAXmIpdeFAv1lMWmFuqezPHb28JxSt1OGSZzpaCRGpwJqlyWIdvtMKlaSmzX1F1e4v5jFFFNLlo+bA6YXne/nL8GsoRig6SRmn/Ri9KvryiCIhNfLO/jbCgbII8Q=;24:ZdACKjCbAKyRqgdNiCm/a0d1CJTmv3Qa996G3XIb/DSdmem8Z94Q1XghBUhUtRWfC+XYvCewXnRY4aO3OiGXHLbX+cM24Xrj40/abZAb0eI=;7:niNW7j4912ege0znLtp5IEStDZmHttOIfVy1iDR76J2NYjzQrfxq30F5dJmblliyM+XWF7cip2W2wUA8D4AqYuzaD70oHu/T+uB4tqJxmeynCnMavh2XuOU+efNmCYK8pj4qYPgeTUW5Bv5yzsBYLp22z9Wgg9jgKj7LSa8+IrMTdq4HxV0bcgCkzKqsqv6jshzOaz+/OywoqMEZvttfpL6a5+sB7rq+3+5GSzkSn2o= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: synaptics.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Oct 2017 17:28:56.3834 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 335d1fbc-2124-4173-9863-17e7051a2a0e X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR03MB2587 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/18/2017 07:13 AM, Jiri Kosina wrote: > On Wed, 18 Oct 2017, Benjamin Tissoires wrote: > >>> The hid-rmi driver may handle non rmi devices on composite USB devices. >>> Callbacks need to make sure that the current device is a RMI device before >>> calling RMI specific functions. Most callbacks already have this check, but >>> this patch adds checks to the remaining callbacks. >>> >>> Signed-off-by: Andrew Duggan >>> --- >>> This is the patch which hopefully will address the X1 tablet dock freeze: >>> http://www.spinics.net/lists/linux-input/msg53582.html >>> >>> I was not able to test on a composite USB device so I have not tested confirmed >>> this will fix the reported issues. But, based on the description I think it will. >>> I also added a check for rmi_raw_event() since it could be possible that another >>> hid device using one of the same report IDs as an RMI device could result in calling >>> into unitialized RMI functions. It was also the only callbacl left not checking the >>> RMI_DEVICE flag. I wonder if this explains the attach crash. >>> >>> Anyway, I would appriciate it if Hendrik or someone else with the device could test this >>> patch to confirm it fixes the reported behavior. >> Shouldn't rmi_hid_reset() also get the same check? > I think so as well. Since rmi_hid_reset() is only called from with in the RMI4 driver I didn't think that it needed a check. All of the functions set in struct hid_driver should  now check the RMI_DEVICE flag before calling into the RMI4 driver. But, if I am missing something we can add additional checks. Thanks, Andrew >> From what I can see, even if the patch doesn't fix the immediate issue, >> such a patch should definitively go in as those checks are clearly >> missing. > Agreed; however I'd like to get Hendrik's Tested-by: if possible in case > this really fixes the issue, so I am not merging it right away. > > Hendrik, are you by any chance able to test this patch in a reasonable > timeframe please? > > Thanks! >