Bug 212295

Summary: pipe deadlocks since kernel v5.8 after resizing (race condition)
Product: File System Reporter: Lukas Schauer (kernel.org)
Component: OtherAssignee: fs_other
Status: NEW ---    
Severity: normal CC: brauner, dhowells, jgoerzen, kernel.org, me, regressions, sam
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 5.8-latest Subsystem:
Regression: Yes Bisected commit-id:
Attachments: Code to reproduce the issue
Patch fixing the race condition
[PATCH] fs/pipe: wakeup wr_wait after setting max_usage

Description Lukas Schauer 2021-03-15 18:00:06 UTC
Created attachment 295871 [details]
Code to reproduce the issue

Hi,

I've been experiencing some weird bugs with pipes sometimes being stuck in a deadlock since kernel v5.8 if they are being resized.

A child process is stuck in pipe_read:

  [<0>] pipe_read+0x2ca/0x410
  [<0>] new_sync_read+0x18d/0x1a0
  [<0>] vfs_read+0xf1/0x180
  [<0>] ksys_read+0xb5/0xd0
  [<0>] do_syscall_64+0x33/0x80
  [<0>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

While the parent process is stuck in the corresponding pipe_write:

  [<0>] pipe_write+0x274/0x5c0
  [<0>] new_sync_write+0x19c/0x1b0
  [<0>] vfs_write+0x184/0x250
  [<0>] ksys_write+0xb5/0xd0
  [<0>] do_syscall_64+0x33/0x80
  [<0>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

The bug is only triggered if pipes get resized, which seemingly very little processes actually do.

A git bisect landed on the following commit:

  commit c73be61cede5882f9605a852414db559c0ebedfd                           
  Author: David Howells <dhowells@redhat.com>                       
  Date:   Tue Jan 14 17:07:11 2020 +0000                                                
                                                                                      
    pipe: Add general notification queue support

I've attached some code that reproduces the bug for me (may take a few hundred loops). Removing the fcntl for F_SETPIPE_SZ removes the pipe_read/write deadlocks, so I guess the bug is somewhere in the resizing logic.
Comment 1 Christian Schwarz 2021-03-15 22:49:51 UTC
I can reproduce the issue using the provided code.
Comment 2 Lukas Schauer 2021-03-16 03:14:14 UTC
Created attachment 295881 [details]
Patch fixing the race condition

I've found the race condition.

After resizing a pipe a wakeup is issued for pipe_write, before actually raising the max_usage value for that pipe.

Depending on wether the pipe was full before resizing or not this could result in a deadlock situation.

I've attached a patch for this to this issue. It's build against v5.8 because that's what I've been using for testing. If necessary please let me know and I'll rebase it for a newer version.
Comment 3 Lukas Schauer 2021-03-24 14:29:42 UTC
Created attachment 296031 [details]
[PATCH] fs/pipe: wakeup wr_wait after setting max_usage

I revised the patch to better address the regression instead of weirdly pasting code around and also sent it to the linux-kernel mailing list with Alan Cox and David Howells in Cc.
Comment 4 John Goerzen 2022-06-20 13:42:28 UTC
What is the current status of getting this merged?  I recently encountered it in the wild.  Thanks.
Comment 5 The Linux kernel's regression tracker (Thorsten Leemhuis) 2023-11-30 14:45:14 UTC
I was recently added to this report. Does anyone still care? Then it would be good if that person could confirm that this still happens with mainline.

Side note, there is currently another patch that fixes some problem the culprit mentioned above introduced, but it might be totally unrelated. 
https://lore.kernel.org/all/20231124150822.2121798-1-jannh@google.com/
Comment 6 John Goerzen 2023-11-30 14:53:18 UTC
Hi Thorsten,

Yes, I do still care.  I have to maintain ugly and imperfect workarounds in various shell scripts; ie:

command1 | (sleep 2; command2)

I don't *think* that the patch you reference fixes this, because I don't believe splice is required to trigger this bug.  But I am not certain of this.
Comment 7 Christian Brauner 2023-12-01 13:02:40 UTC
A fix for this is now in:

https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/log/?h=vfs.misc
Comment 8 Christian Brauner 2023-12-01 13:03:41 UTC
I see no need to expedite this since this has been around since v5.8. So this would be fixed when vfs.misc is merged during the v6.8 merge window.
Comment 9 The Linux kernel's regression tracker (Thorsten Leemhuis) 2023-12-01 13:15:36 UTC
Christian, many thx for taking care of this. And FWIW, I totally agree wrt to "no need to expedite this".

Side note: when Sam James added me to this he also added me to two other regressions wrt to pipes & splice:

https://bugzilla.kernel.org/show_bug.cgi?id=207473
https://bugzilla.kernel.org/show_bug.cgi?id=209251

Those are even older (and the second one might not really qualify as regression due to a vendor kernel as base) and I have no idea if they still happen yet. Just thought you might wanted to know.