From:   Benjamin Tissoires <benjamin.tissoires@redhat.com>
To:     Jiri Kosina <jikos@kernel.org>, Hanno Zulla <kontakt@hanno.de>,
        Hanno Zulla <abos@hanno.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Pietro Borrello <borrello@diag.uniroma1.it>
Cc:     Cristiano Giuffrida <c.giuffrida@vu.nl>,
        "Bos, H.J." <h.j.bos@vu.nl>, Jakob Koschel <jkl820.git@gmail.com>,
        Jiri Kosina <jkosina@suse.cz>,
        Roderick Colenbrander <roderick@gaikai.com>,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20230125-hid-unregister-leds-v4-0-7860c5763c38@diag.uniroma1.it>
References: <20230125-hid-unregister-leds-v4-0-7860c5763c38@diag.uniroma1.it>
Subject: Re: (subset) [PATCH v4 0/5] HID: use spinlocks to safely schedule
 led workers
Message-Id: <167648508201.1580939.291629021543169868.b4-ty@redhat.com>
Date:   Wed, 15 Feb 2023 19:18:02 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sun, 12 Feb 2023 18:59:58 +0000, Pietro Borrello wrote:
> I noticed a recurring pattern is present in multiple hid devices in the
> Linux tree, where the LED controller of a device schedules a work_struct
> to interact with the hardware.
> The work_struct is embedded in the device structure and thus, is freed
> at device removal.
> 
> The issue is that a LED worker may be scheduled by a timer concurrently
> with device removal, causing the work_struct to be accessed after having
> been freed.
> I was able to trigger the issue in hid-bigbenff.c and hid-asus.c
> where the work_structs may be scheduled by the LED controller
> while the device is disconnecting, triggering use-after-frees.
> I can attach the reproducer, but it's very simple USB configuration,
> using the /dev/raw-gadget interface with some more USB interactions
> to manage LEDs configuration and pass checks in asus_kbd_init()
> and asus_kbd_get_functions() in case of hid-asus.c.
> I triggered the issue by connecting a device and immediately
> disconnecting it, so that the remove function runs before the LED one
> which remains pending.
> 
> [...]

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.3/asus), thanks!

[4/5] HID: asus: use spinlock to protect concurrent accesses
      https://git.kernel.org/hid/hid/c/315c537068a1
[5/5] HID: asus: use spinlock to safely schedule workers
      https://git.kernel.org/hid/hid/c/4ab3a086d10e

Cheers,
-- 
Benjamin Tissoires <benjamin.tissoires@redhat.com>