linux-kernel - cancel_delayed_work() semantics broken?

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-next>] [day] [month] [year] [list]

Message-ID: <444a6439-b1a4-4740-9e7e-bc37267cfe73@default>
Date:	Thu, 18 Oct 2012 16:00:03 -0700 (PDT)
From:	Dan Magenheimer <dan.magenheimer@...cle.com>
To:	tj@...nel.org
Cc:	linux-kernel@...r.kernel.org, Konrad Wilk <konrad.wilk@...cle.com>
Subject: cancel_delayed_work() semantics broken?

Hi Tejun --

Forgive me if I am missing something, but it appears
that your commit:  57b30ae77bf00d2318df711ef9a4d2a9be0a3a2a
(workqueue: reimplement cancel_delayed_work() using try_to_grab_pending())
has subtly broken the semantics of the function. If
work was idle, according to the comment, it should
return false, correct?

It appears that very few callsites check the return value,
but ramster does, as does ocfs2 from whence the code at the
ramster callsite was derived.  They both decrement
a kref count based on the return value.

I am still looking at try_to_grab_pending as I am not sure
if its semantics have also changed.

Thanks,
Dan

Signed-off-by: Dan Magenheimer <dan.magenheimer@...cle.com>

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index d951daa..042d221 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2982,7 +2982,7 @@ bool cancel_delayed_work(struct delayed_work *dwork)

 	set_work_cpu_and_clear_pending(&dwork->work, work_cpu(&dwork->work));
 	local_irq_restore(flags);
-	return true;
+	return ret;
 }
 EXPORT_SYMBOL(cancel_delayed_work);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/