linux-kernel - [PATCH v1 15/19] hid-ft260: uart: remove FIXME for wake-up workaround

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240210215147.77629-16-michael.zaidman@gmail.com>
Date: Sat, 10 Feb 2024 23:51:43 +0200
From: Michael Zaidman <michael.zaidman@...il.com>
To: chrysh@...istina-quast.de,
	daniel.beer@...rinstitute.com,
	jikos@...nel.org
Cc: linux-kernel@...r.kernel.org,
	linux-input@...r.kernel.org,
	linux-serial@...r.kernel.org,
	ilpo.jarvinen@...ux.intel.com,
	johan@...nel.org,
	gregkh@...uxfoundation.org,
	equinox@...c24.net,
	michael.zaidman@...il.com
Subject: [PATCH v1 15/19] hid-ft260: uart: remove FIXME for wake-up workaround

The FIXME is related to the wake-up workaround cancelation - do we
need the reschedule_work flag alongside the cancel_work_sync usage?

The teardown sequence is as described below:

Upon tty session termination:
   ft260_uart_port_shutdown
       ft260_uart_wakeup_workaraund_enable: deactivate wakeup workaround
   t260_uart_cleanup
   ft260_uart_port_put

On rmmod:
   ft260_remove
       cancel_work_sync
           ft260_uart_port_remove
               timer_delete_sync
   ft260_uart_port_put
   ft260_uart_port_destroy

The ft260_uart_start_wakeup timer_work can occur after the cancel_work_sync
returns, rescheduling the wakeup_work again. The reschedule_work flag set
earlier at the ft260_uart_port_shutdown time prevents it.

The alternative could be performing the timer_delete_sync first, preventing
the wakeup_timer and wakeup_work from rescheduling, and then canceling the
last wakeup_work by calling the cancel_work_sync. However, we still need the
reschedule_work flag to enable or bypass the power saving mode according to
the requested baud rate.

Signed-off-by: Michael Zaidman <michael.zaidman@...il.com>
---
 drivers/hid/hid-ft260.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 1c113f735524..52ccee83250e 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1585,7 +1585,6 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
 	struct device *devt;
 
 	INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
-	// FIXME: Do I need that if I have cancel_work_sync?
 	// FIXME: are all kfifo access secured by lock? with irq or not?
 	ft260_uart_wakeup_workaraund_enable(dev, true);
 	/* Work not started at this point */
@@ -1721,7 +1720,6 @@ static void ft260_remove(struct hid_device *hdev)
 		return;
 
 	if (dev->iface_type == FT260_IFACE_UART) {
-		// FIXME:
 		cancel_work_sync(&dev->wakeup_work);
 		tty_port_unregister_device(&dev->port, ft260_tty_driver,
 					   dev->index);
-- 
2.40.1