linux-kernel - [PATCH v2] misc/sgi-xp: Replace in

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: <20201119103151.ppo45mj53ulbxjx4@linutronix.de>
Date:   Thu, 19 Nov 2020 11:31:51 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Cliff Whickman <cpw@....com>, Arnd Bergmann <arnd@...db.de>,
        Robin Holt <robinmholt@...il.com>,
        Steve Wahl <steve.wahl@....com>,
        Dimitri Sivanich <dimitri.sivanich@....com>,
        Russ Anderson <russ.anderson@....com>
Subject: [PATCH v2] misc/sgi-xp: Replace in_interrupt() usage

From: Thomas Gleixner <tglx@...utronix.de>

The usage of in_interrupt() in xpc_partition_disengaged() is clearly
intended to avoid canceling the timeout timer when the function is invoked
from the timer callback.

While in_interrupt() is deprecated and ill defined as it does not provide
what the name suggests it catches the intended case.

Add an argument to xpc_partition_disengaged() which is true if called
from timer and otherwise false.
Use del_timer_sync() instead of del_singleshot_timer_sync() which is the
same thing.

Note: This does not prevent reentrancy into the function as the function
has no concurrency control and timer callback and regular task context
callers can happen concurrently on different CPUs or the timer can
interrupt the task context before it is able to cancel it.

While the only driver which is providing the arch_xpc_ops callbacks
(xpc_uv) seems not to have a reentrancy problem and the only negative
effect would be a double dev_info() entry in dmesg, the whole mechanism is
conceptually broken.

But that's not subject of this cleanup endeavour and left as an exercise to
the folks who might have interest to make that code fully correct.

[bigeasy: Add the argument, use del_timer_sync().]

Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Cliff Whickman <cpw@....com>
Cc: Arnd Bergmann <arnd@...db.de>
Cc: Robin Holt <robinmholt@...il.com>
Cc: Steve Wahl <steve.wahl@....com>
Cc: Dimitri Sivanich <dimitri.sivanich@....com>
Cc: Russ Anderson <russ.anderson@....com>
---
v1…v2: Use xpc_partition_disengaged_from_timer() and don't export the
       argument. Suggested by Greg.

 drivers/misc/sgi-xp/xpc.h           |  1 +
 drivers/misc/sgi-xp/xpc_main.c      |  2 +-
 drivers/misc/sgi-xp/xpc_partition.c | 20 +++++++++++++++-----
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/sgi-xp/xpc.h b/drivers/misc/sgi-xp/xpc.h
index 71db60edff655..225f2bb84e39b 100644
--- a/drivers/misc/sgi-xp/xpc.h
+++ b/drivers/misc/sgi-xp/xpc.h
@@ -634,6 +634,7 @@ extern int xpc_setup_rsvd_page(void);
 extern void xpc_teardown_rsvd_page(void);
 extern int xpc_identify_activate_IRQ_sender(void);
 extern int xpc_partition_disengaged(struct xpc_partition *);
+extern int xpc_partition_disengaged_from_timer(struct xpc_partition *part);
 extern enum xp_retval xpc_mark_partition_active(struct xpc_partition *);
 extern void xpc_mark_partition_inactive(struct xpc_partition *);
 extern void xpc_discovery(void);
diff --git a/drivers/misc/sgi-xp/xpc_main.c b/drivers/misc/sgi-xp/xpc_main.c
index e5244fc1dab30..84610bbcc1314 100644
--- a/drivers/misc/sgi-xp/xpc_main.c
+++ b/drivers/misc/sgi-xp/xpc_main.c
@@ -179,7 +179,7 @@ xpc_timeout_partition_disengage(struct timer_list *t)
 
 	DBUG_ON(time_is_after_jiffies(part->disengage_timeout));
 
-	(void)xpc_partition_disengaged(part);
+	xpc_partition_disengaged_from_timer(part);
 
 	DBUG_ON(part->disengage_timeout != 0);
 	DBUG_ON(xpc_arch_ops.partition_engaged(XPC_PARTID(part)));
diff --git a/drivers/misc/sgi-xp/xpc_partition.c b/drivers/misc/sgi-xp/xpc_partition.c
index 57df06820bae2..1999d02923dee 100644
--- a/drivers/misc/sgi-xp/xpc_partition.c
+++ b/drivers/misc/sgi-xp/xpc_partition.c
@@ -262,8 +262,8 @@ xpc_get_remote_rp(int nasid, unsigned long *discovered_nasids,
  * from us. Though we requested the remote partition to deactivate with regard
  * to us, we really only need to wait for the other side to disengage from us.
  */
-int
-xpc_partition_disengaged(struct xpc_partition *part)
+static int __xpc_partition_disengaged(struct xpc_partition *part,
+				      bool from_timer)
 {
 	short partid = XPC_PARTID(part);
 	int disengaged;
@@ -289,9 +289,9 @@ xpc_partition_disengaged(struct xpc_partition *part)
 		}
 		part->disengage_timeout = 0;
 
-		/* cancel the timer function, provided it's not us */
-		if (!in_interrupt())
-			del_singleshot_timer_sync(&part->disengage_timer);
+		/* Cancel the timer function if not called from it */
+		if (!from_timer)
+			del_timer_sync(&part->disengage_timer);
 
 		DBUG_ON(part->act_state != XPC_P_AS_DEACTIVATING &&
 			part->act_state != XPC_P_AS_INACTIVE);
@@ -303,6 +303,16 @@ xpc_partition_disengaged(struct xpc_partition *part)
 	return disengaged;
 }
 
+int xpc_partition_disengaged(struct xpc_partition *part)
+{
+	return __xpc_partition_disengaged(part, false);
+}
+
+int xpc_partition_disengaged_from_timer(struct xpc_partition *part)
+{
+	return __xpc_partition_disengaged(part, true);
+}
+
 /*
  * Mark specified partition as active.
  */
-- 
2.29.2