[PATCH 0/2] unwind: Fix a few edge cases in unwind_deferred_request()

[PATCH 0/2] unwind: Fix a few edge cases in unwind_deferred_request()

[Josh Poimboeuf]

Fixes for a couple of issues which were found in code review a few
months ago but got overlooked.

Josh Poimboeuf (2):
  unwind: Fix negative bit check in unwind_deferred_request()
  unwind: Fix return value for already-queued case in
    unwind_deferred_request()

 kernel/unwind/deferred.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

-- 
2.52.0

[PATCH 1/2] unwind: Fix negative bit check in unwind_deferred_request()

[Josh Poimboeuf]

The following warning will never trigger because 'bit' is unsigned:

	if (WARN_ON_ONCE(bit < 0))
		return -EINVAL;

That variable is used for two distinct purposes, so split it into two
variables accordingly: an 'int bit' and an 'unsigned long bit_mask'.
Rename a few other variables for consistency with the "*_mask" scheme.

Fixes: 357eda2d7450 ("unwind deferred: Use SRCU unwind_deferred_task_work()")
Reported-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 kernel/unwind/deferred.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/kernel/unwind/deferred.c b/kernel/unwind/deferred.c
index a88fb481c4a3..416af9b98bad 100644
--- a/kernel/unwind/deferred.c
+++ b/kernel/unwind/deferred.c
@@ -230,10 +230,9 @@ void unwind_deferred_task_exit(struct task_struct *task)
 int unwind_deferred_request(struct unwind_work *work, u64 *cookie)
 {
 	struct unwind_task_info *info = &current->unwind_info;
+	unsigned long bit_mask, old_mask, new_mask;
 	int twa_mode = TWA_RESUME;
-	unsigned long old, bits;
-	unsigned long bit;
-	int ret;
+	int bit, ret;
 
 	*cookie = 0;
 
@@ -257,17 +256,16 @@ int unwind_deferred_request(struct unwind_work *work, u64 *cookie)
 	if (WARN_ON_ONCE(bit < 0))
 		return -EINVAL;
 
-	/* Only need the mask now */
-	bit = BIT(bit);
+	bit_mask = BIT(bit);
 
 	guard(irqsave)();
 
 	*cookie = get_cookie(info);
 
-	old = atomic_long_read(&info->unwind_mask);
+	old_mask = atomic_long_read(&info->unwind_mask);
 
 	/* Is this already queued or executed */
-	if (old & bit)
+	if (old_mask & bit_mask)
 		return 1;
 
 	/*
@@ -276,15 +274,15 @@ int unwind_deferred_request(struct unwind_work *work, u64 *cookie)
 	 * work's bit or PENDING was already set, then this is already queued
 	 * to have a callback.
 	 */
-	bits = UNWIND_PENDING | bit;
-	old = atomic_long_fetch_or(bits, &info->unwind_mask);
-	if (old & bits) {
+	new_mask = UNWIND_PENDING | bit_mask;
+	old_mask = atomic_long_fetch_or(new_mask, &info->unwind_mask);
+	if (old_mask & new_mask) {
 		/*
 		 * If the work's bit was set, whatever set it had better
 		 * have also set pending and queued a callback.
 		 */
-		WARN_ON_ONCE(!(old & UNWIND_PENDING));
-		return old & bit;
+		WARN_ON_ONCE(!(old_mask & UNWIND_PENDING));
+		return old_mask & bit_mask;
 	}
 
 	/* The work has been claimed, now schedule it. */
-- 
2.52.0

[PATCH 2/2] unwind: Fix return value for already-queued case in unwind_deferred_request()

[Josh Poimboeuf]

The unwind_deferred_request() return value is documented in the function
header:

  * Return: 0 if the callback successfully was queued.
  *         1 if the callback is pending or was already executed.
  *         Negative if there's an error.

However, when the callback is already queued, it may return a non-1
value.  Fix it to match the documented behavior.

Fixes: be3d526a5b34 ("unwind deferred: Use bitmask to determine which callbacks to call")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 kernel/unwind/deferred.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/unwind/deferred.c b/kernel/unwind/deferred.c
index 416af9b98bad..edf39c61130a 100644
--- a/kernel/unwind/deferred.c
+++ b/kernel/unwind/deferred.c
@@ -282,7 +282,7 @@ int unwind_deferred_request(struct unwind_work *work, u64 *cookie)
 		 * have also set pending and queued a callback.
 		 */
 		WARN_ON_ONCE(!(old_mask & UNWIND_PENDING));
-		return old_mask & bit_mask;
+		return !!(old_mask & bit_mask);
 	}
 
 	/* The work has been claimed, now schedule it. */
-- 
2.52.0