linux-kernel - Re: [PATCH] pipe_read: don't wake up the writer if the pipe is still full

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250305114433.GA28112@redhat.com>
Date: Wed, 5 Mar 2025 12:44:34 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: Hillf Danton <hdanton@...a.com>
Cc: K Prateek Nayak <kprateek.nayak@....com>,
	Mateusz Guzik <mjguzik@...il.com>,
	"Sapkal, Swapnil" <swapnil.sapkal@....com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pipe_read: don't wake up the writer if the pipe is still
 full

Hi Hillf,

again, I am not sure we understand each other, at least me...

On 03/05, Hillf Danton wrote:
>
> On Wed, 5 Mar 2025 00:49:09 +0100 Oleg Nesterov <oleg@...hat.com>
> >
> > Of course! Again, whatever the woken writer checks in pipe_writable()
> > lockless, another writer can make pipe_full() true again.
> >
> > But why do we care? Why do you think that the change you propose makes
>
> Because of the hang reported.

The hang happened because pipe_writable() could wrongly return false
when the buffer is not full.

Afaics, the Prateek's or Linus's fix solve this problem, and this is
all we need.

> > more sense than the fix from Prateek or the (already merged) Linus's fix?
> >
> See the loop in  ___wait_event(),
>
> 	for (;;) {
> 		prepare_to_wait_event();
>
> 		// flip
> 		if (condition)
> 			break;
>
> 		schedule();
> 	}

I will assume that this "// flip" means the case I described above:
before this writer checks the condition, another writer comes and
increments pipe->head.

> After wakeup, waiter will sleep again if condition flips false on the waker
> side before waiter checks condition, even if condition is atomic, no?

Yes, but in this case pipe_full() == true is correct, this writer can
safely sleep.

Even if flips again and becomes false right after the writer called
pipe_writable().

Note that it checks the condition after set_current_state(INTERRUPTIBLE)
and it is still on the pipe->wr_wait->head list.

The 2nd "flip" is only possible if some reader flushes another buffer
and updates pipe->tail, and in this case it will wake this writer again.

So I still can't understand your concerns, sorry.

Oleg.