From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB1A333BBAD for ; Sun, 2 Aug 2026 20:41:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785703311; cv=none; b=Q/Z7w2zkdSGgC9yKZymYs3wDRaVKikVDNqbUkoNZahwsBUNHgR8liZUKg2fhfznyMEqenDZsa6VQK1Z+OHsiICvmjNZlGzQVuMA6SI6M8y051qsqvBT2flfNxrCw1X83BG4mJxSx4UzJE3V9ccYoBqZmI1BwzMb1vu4Vg/DGe9M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785703311; c=relaxed/simple; bh=mdAaS/ICydQwFtXsmVmKdsLGrQQdGzTBXbVf6wf941k=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=cTzLuRxSrgIJAvFwU28rNldkIPhbahJtagUasyav3YeXUnIxmPPIpgCZIJDaZmG6UuUw/A1p74NBLtqAu7kIb+wZ4lE6iIe5taNqINSiLsycq2O20Jidd+50km85AB/Mh2luq8auTxPXF5BAe+fAneSPmoB3JZ1LLuLIr3yOoYU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Mu9SJvkz; arc=none smtp.client-ip=209.85.221.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Mu9SJvkz" Received: by mail-wr1-f50.google.com with SMTP id ffacd0b85a97d-47fe45db360so4224f8f.2 for ; Sun, 02 Aug 2026 13:41:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1785703308; x=1786308108; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:content-type :disposition-notification-to:references:in-reply-to:date:cc:to:from :subject:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=AMvC+Rujpi9rjCvTJL9hyNOIokIpUH49tZ8qOu+YRw0=; b=Mu9SJvkztuEjyWA/kpIOCesOfkzTA4iWreFEmqszFn+Tdw3g+C7Rves3gVTnqbxXwq EI4g983IRqTWw+NDtnoiSSgiR/kdRAq/Jxb1E9RB2HiTWpK+m1i6iX1WSi5EblhfIgtP RxxbDO8BaGaIT3aDam0h1xBQFONrETlg+13rAC2V8rOFPY/7awkDZQuLHHI+vlSxIxrL 1BAVlPbAq31speeXexroWNwzlg3fAyWUojC6giicxrX83M2eqc+TtowqpgNYc53Gdnoz TPrhD67sWnWvoekwGQFUbWUwDbXt11mKlB+xB/XEhpGf2odl0QYHGLtYV0Ko2QrSHwIM nXAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785703308; x=1786308108; h=mime-version:user-agent:content-transfer-encoding:content-type :disposition-notification-to:references:in-reply-to:date:cc:to:from :subject:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to:content-type; bh=AMvC+Rujpi9rjCvTJL9hyNOIokIpUH49tZ8qOu+YRw0=; b=ru4cqzugDibnx4OhjemH2/WqViP9q457opkrCAq0p4SC4mJBTnLYVT7TeA9CSiIgPW qof31RZnmf8Wdb0wp3R8kYSFIg54+iWFKciNDDWR5ES/R6yOt9noLqZs/UPKfLglOd8L vxXWzuaQuDbVJG5nbgMolL4aLUrRReVQFJ7c5F6qziX96qbalCIZF6WczpPKE1mUp60u 0JQ5dAxAcV9JjvBDeJXm8lmxxPuV2jFyavhyS02Agm+KokqKtgpxoZQjgW1mxcUPsgM3 V7V0RW5DpMhzjUhCWqm80ifZbookSoEFqyGBH+0Xmold7g/RTXn+zWpaCQrqZ5t0QRCH Yelg== X-Forwarded-Encrypted: i=1; AHgh+RrOQGseTjsRtDHf5CQOjcVM0OPjuzVlWVrs3To59tTXU29asdV7sDF5i91slOG71U1emy70xlQd++XQv+Y=@vger.kernel.org X-Gm-Message-State: AOJu0YzpgbMqU7gLz7SNBlnK/XygroCMA9+Cl5XaWJ7MpxTUw0FL7P3v GAhRedxindwYj3wS9WszF3nAOJqq0i0UHkEWnUj/ujTY59y1Y5rsc5Xm X-Gm-Gg: AR+sD10DD6JE7IvvTLeWDNO2UDjwxLFDA+UvKfNBznqGST/aw9rhKVZCDK8YcFtFytv U5L7zZ5W8+0KZyNj0dzcMSohwXem8bTQSEm0Al2+Bqr3143d4tixZAYmBrraanqHw/jLEHI9fGr KqFHhTlky8PZ/V9PiG7Ya5UfkMXmwz0rQuyOkc2KzXAZZSPpBGA7mUdFerikIFDC237TdOpx1MU pNHW3M6ouNNEKBlxj0a0s0rceHOveU6ril+ZgMosgZ/HyNSq0/Ri+e5aXWwlIUCdVYyMECNw0fs 45BJqMJMuSa6buNDL/0uItToRg94+0ud/AIvzkH4kQl5HuvB1YV4Crt+zapJ/yoa0vt0wnQ1asP uWNI7xdq4oqPXkQbigYx6A/Hi4BWIKyudZ7FJdIOVTWadECrK4fZ1WoJhF7o+gA/NKnQS8X1mKE Gnv7UKu0cClB0P7v/EPHbGj7UAnrT0Tk5KBtv3sQ1SGakelscNvKHU8KDTORMU46WvFuzaY5UCo 4ytS00fuRni/uF613orofYH14+FJDvGVxkghBIyX7OCXTeH X-Received: by 2002:a5d:5f42:0:b0:47f:7e91:3d55 with SMTP id ffacd0b85a97d-47fd72e70eemr11042359f8f.2.1785703308035; Sun, 02 Aug 2026 13:41:48 -0700 (PDT) Received: from [192.168.1.239] (87-205-5-123.static.ip.netia.com.pl. [87.205.5.123]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47fd458adc9sm28027572f8f.27.2026.08.02.13.41.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 02 Aug 2026 13:41:47 -0700 (PDT) Message-ID: <9ce239858b358c60a243e34f2d61def2f986966a.camel@gmail.com> Subject: Re: [PATCH] HID: universal-pidff: stop the device when force-feedback init fails From: Tomasz =?UTF-8?Q?Paku=C5=82a?= To: Baul Lee , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Oleg Makarenko , Jiri Kosina , Benjamin Tissoires Date: Sun, 02 Aug 2026 22:41:46 +0200 In-Reply-To: <20260729142432.16184-1-baul.lee@xbow.com> References: <20260729142432.16184-1-baul.lee@xbow.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.60.2 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Wed, 2026-07-29 at 23:24 +0900, Baul Lee wrote: > universal_pidff_probe() starts the device with hid_hw_start() and then, i= f > force-feedback initialisation fails, returns the error through a label th= at > only does "return error". The device is left started. >=20 > The HID core does not unwind on the driver's behalf. __hid_device_probe(= ) > releases the devres group, closes the report and clears hdev->driver: >=20 > if (ret) { > devres_release_group(&hdev->dev, hdev->devres_group_id); > hid_close_report(hdev); > hdev->driver =3D NULL; > } >=20 > The hidraw character device that hid_hw_start() registered through > hid_connect() is allocated with kzalloc() and added with cdev_device_add(= ), > so it is not devres-managed and survives that. With hdev->driver NULL, > hid_device_remove() skips hid_hw_stop() as well, because it only unwinds > while a driver is still attached. The registration therefore outlives th= e > device on both paths. >=20 > Opening the surviving /dev/hidrawX writes into freed memory. KASAN repor= ts > a use-after-free write from hidraw_open() -> hid_hw_open() -> the > transport's open callback, which takes a spinlock inside the freed object= . > A descriptor that carries a PID usage page and no input reports is enough= : > hidraw claims the device so hid_hw_start() succeeds, while hid->inputs > stays empty so force-feedback init fails. The other failure returns in > hid_pidff_init_with_quirks() - no output reports, an allocation failure, > pidff_init_fields(), pidff_check_autocenter(), an unusable effect count, > input_ff_create() - all reach the same label. >=20 > Stop the device on that path. hid-dr.c and hid-emsff.c, which start the > device with the same HID_CONNECT_DEFAULT & ~HID_CONNECT_FF mask, already = do > this. The two earlier gotos must keep returning without hid_hw_stop(), > since neither has a started device, so give the path that fails after the > start its own label. >=20 > Discovered by XBOW, triaged by Baul Lee >=20 > Fixes: f06bf8d94fff ("HID: Add hid-universal-pidff driver and supported d= evice ids") > Cc: stable@vger.kernel.org > Signed-off-by: Baul Lee > --- > drivers/hid/hid-universal-pidff.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) >=20 > diff --git a/drivers/hid/hid-universal-pidff.c b/drivers/hid/hid-universa= l-pidff.c > index 549dac555d40..60180467a0bf 100644 > --- a/drivers/hid/hid-universal-pidff.c > +++ b/drivers/hid/hid-universal-pidff.c > @@ -104,12 +104,14 @@ static int universal_pidff_probe(struct hid_device = *hdev, > error =3D init_function(hdev, id->driver_data); > if (error) { > hid_warn(hdev, "Error initialising force feedback\n"); > - goto err; > + goto err_stop; > } > =20 > hid_info(hdev, "Universal pidff driver loaded successfully!"); > =20 > return 0; > +err_stop: > + hid_hw_stop(hdev); > err: > return error; > } Actually, now that I look at it, this isn't the only place where the stop should happen. Any error after successful hid_hw_start() should trigger hid_hw_stop(). So for this to truly matter, you should replace return 0; with goto err_stop; on lines 92 and 101. Just add error value clearing so error =3D 0 because even though we're exiting there, it's not an error. Some devices just show up as multiple separate HID devices. Tomasz