[Xenomai] [RFC][PATCH] cobalt/synch: Do not drop PP on xnsynch_release if it wasn't committed

[Xenomai] [RFC][PATCH] cobalt/synch: Do not drop PP on xnsynch_release if it wasn't committed

[Jan Kiszka]

We must not try to drop the prio ceiling in the kernel if it wasn't
committed yet. That could easily happen when using a prio-ceiling mutex
to wait on a cond-var.

This is not the most robust solution as it bases the decision on
userspace controlled state which could be corrupted.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

As written above, I'm wondering if we shouldn't mirror the state into
kernel space. Or do we have it somewhere already?

Also, could you have a second look at the whole PP thing Philippe. Are
there more imbalances possible? I'll think through that again as well.
TIA!

Test case will follow tomorrow (likely).

 kernel/cobalt/synch.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
index 99397b01e..f2f10499b 100644
--- a/kernel/cobalt/synch.c
+++ b/kernel/cobalt/synch.c
@@ -942,12 +942,13 @@ bool xnsynch_release(struct xnsynch *synch, struct xnthread *curr)
 	 */
 	xnlock_get_irqsave(&nklock, s);
 
-	if (synch->status & XNSYNCH_PP) {
+	h = atomic_cmpxchg(lockp, currh, XN_NO_HANDLE);
+
+	if (h & XNSYNCH_FLCEIL) {
 		clear_pp_boost(synch, curr);
 		need_resched = true;
 	}
 
-	h = atomic_cmpxchg(lockp, currh, XN_NO_HANDLE);
 	if ((h & ~XNSYNCH_FLCEIL) != currh)
 		/* FLCLAIM set, synch is contended. */
 		need_resched = transfer_ownership(synch, curr);
-- 
2.13.6

Re: [Xenomai] [RFC][PATCH] cobalt/synch: Do not drop PP on xnsynch_release if it wasn't committed

[Philippe Gerum]

On 04/26/2018 08:01 PM, Jan Kiszka wrote:
> We must not try to drop the prio ceiling in the kernel if it wasn't
> committed yet. That could easily happen when using a prio-ceiling mutex
> to wait on a cond-var.
> 
> This is not the most robust solution as it bases the decision on
> userspace controlled state which could be corrupted.
> 

The same issue applies to all *acquire() forms.

> 
> As written above, I'm wondering if we shouldn't mirror the state into
> kernel space. Or do we have it somewhere already?
> 

XNSYNCH_CEILING in this context, since ceil_owner_priority() raises it
when committing the boost.

> Also, could you have a second look at the whole PP thing Philippe. Are
> there more imbalances possible?

Unlikely in kernel space where there is a single release point. However
the tricky part is about getting that state in sync with the
syscall-less fast path in userland.

> I'll think through that again as well.
> TIA!
> 

Ok, I'll have a look when I'm back on-line in two weeks.

-- 
Philippe.