linux-kernel - Re: [PATCH] lock_page() doesn't lock if __wait_on_bit

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]

Date:	Sat, 12 Dec 2015 11:41:26 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Chris Mason <clm@...com>, Peter Zijlstra <peterz@...radead.org>,
	Dave Jones <dsj@...com>, LKML <linux-kernel@...r.kernel.org>,
	Jon Christopherson <jon@...s.org>
Cc:	NeilBrown <neilb@...e.de>, Ingo Molnar <mingo@...nel.org>,
	David Howells <dhowells@...hat.com>,
	Steven Whitehouse <swhiteho@...hat.com>
Subject: Re: [PATCH] lock_page() doesn't lock if __wait_on_bit_lock returns -EINTR

On Sat, Dec 12, 2015 at 10:33 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> Peter, did that patch also handle just plain "lock_page()" case?

Looking more at it, I think this all goes back to commit 743162013d40
("sched: Remove proliferation of wait_on_bit() action functions").

Before that, we had wait_on_page_bit() doing:

        __wait_on_bit(page_waitqueue(page), &wait, sleep_on_page,
TASK_UNINTERRUPTIBLE);

and after that, the "sleep_on_page" got changed to "bit_wait_io".

But that is bogus, because sleep_on_page() used to look like this:

    static int sleep_on_page(void *word)
    {
        io_schedule();
        return 0;
    }

while bit_wait_io() looks like this:

    __sched int bit_wait_io(void *word)
    {
        if (signal_pending_state(current->state, current))
                return 1;
        io_schedule();
        return 0;
    }

which is ok, because as long as the task state is
TASK_UNINTERRUPTIBLE, the whole signal_pending_state() thing turns
into a no-op.

So far, so fine.

However, then commit 68985633bccb ("sched/wait: Fix signal handling in
bit wait helpers") _really_ screwed up, and changed the function to

    __sched int bit_wait(struct wait_bit_key *word)
    {
        schedule();
        if (signal_pending(current))
                return -EINTR;
        return 0;
    }

so now it returns an error when no error should happen. Which in turn
makes __wait_on_bit() exit the bit-wait loop early.

It looks like PeterZ's pending patch should fix this, by passing in
the proper TASK_UNINTERRUPTIBLE to the bit_wait_io function, and going
back to signal_pending_state(). PeterZ, did I follow the history of
this correctly?

                   Linus
--
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/