linux-kernel - Re: [RFC PATCH] ipc/mqueue: avoid sleep after wakeup

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

Date:   Fri, 14 May 2021 17:51:47 +0200
From:   Manfred Spraul <manfred@...orfullife.com>
To:     Hillf Danton <hdanton@...a.com>, Davidlohr Bueso <dbueso@...e.de>
Cc:     Matthias von Faber <matthias.vonfaber@...-tech.de>,
        Varad Gautam <varad.gautam@...e.com>,
        Christian Brauner <christian.brauner@...ntu.com>,
        Oleg Nesterov <oleg@...hat.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] ipc/mqueue: avoid sleep after wakeup

Hi Hillf,

On 5/14/21 5:01 AM, Hillf Danton wrote:
> The pipeline waker could start doing its job once waiter releases lock and
> get the work done before waiter takes a nap, so check wait condition before
> sleep to avoid waiting the wakeup that will never come, though that does not
> hurt much thanks to timer timeouts like a second.

First: The timeout could be infinity, thus the code must not rely on a 
timeout wakeup.

A wrong wait is would be a bug.

>
> Check signal for the same reason.
>
> Signed-off-by: Hillf Danton <hdanton@...a.com>
> ---
>
> --- y/ipc/mqueue.c
> +++ x/ipc/mqueue.c
> @@ -710,15 +710,24 @@ static int wq_sleep(struct mqueue_inode_
>   		__set_current_state(TASK_INTERRUPTIBLE);
>   
>   		spin_unlock(&info->lock);
> -		time = schedule_hrtimeout_range_clock(timeout, 0,
> -			HRTIMER_MODE_ABS, CLOCK_REALTIME);
>   

I do not see a bug:

We do the __set_current_state() while holding the spinlock. If there is 
a wakeup, then the wakeup will change current->state to TASK_RUNNING.

schedule() will not remove us from the run queue when current->state is 
TASK_RUNNING. The same applies if there are pending signals: schedule() 
checks for pending signals and sets current->state to TASK_RUNNING.

Since the __set_current_state() is done while we hold info->lock, and 
since the wakeup cannot happen before we have dropped the lock [because 
the task that wakes us up needs the same lock], I do not see how a 
wakeup could be lost.

Thus: Which issue do you see?

--

     Manfred