Bug 14416

Summary: Null pointer dereference in fs/pipe.c
Product: File System Reporter: Earl Chew (earl_chew)
Component: OtherAssignee: Alexey Dobriyan (adobriyan)
Status: RESOLVED CODE_FIX    
Severity: normal CC: adobriyan, stsp2
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.21 Tree: Mainline
Regression: No
Attachments: Script to trigger defect
Proposed patch

Description Earl Chew 2009-10-16 03:48:32 UTC
[<ffffffff802899a5>] pipe_rdwr_open+0x35/0x70
  [<ffffffff8028125c>] __dentry_open+0x13c/0x230
  [<ffffffff8028143d>] do_filp_open+0x2d/0x40
  [<ffffffff802814aa>] do_sys_open+0x5a/0x100
  [<ffffffff8021faf3>] sysenter_do_call+0x1b/0x67

For more details see email log at:

http://lkml.org/lkml/2009/10/14/184

I've had a look at the fs/pipe.c in 2.6.31 :

http://lxr.linux.no/linux+v2.6.31/fs/pipe.c#L800

and it seems that the defect should be present there too.

To reproduce easily, apply the following patch:

--- pipe.c.orig 2009-10-15 20:33:53.000000000 -0700
+++ pipe.c      2009-10-15 20:17:40.000000000 -0700
@@ -736,2 +736,3 @@
 {
+       msleep(100);
        mutex_lock(&inode->i_mutex);

then use the attached script to trigger the bug.
Comment 1 Earl Chew 2009-10-16 03:50:01 UTC
Created attachment 23425 [details]
Script to trigger defect
Comment 2 Earl Chew 2009-10-16 04:15:09 UTC
Created attachment 23426 [details]
Proposed patch
Comment 4 Stas Sergeev 2009-11-07 08:45:28 UTC
Hi!

I just ocasionally went through the
forum where this problem was discussed.
People there had a concern whether the
proposed fix is the best possible solution.
They thought the proper locking must
make the checks like this unneeded.
This raised my curiosity, and after a
quick look at the code, I am wondering too.
If the open() happens on an already released
pipe, then can this be a missing synchronization
in VFS? Of course pipe.c can handle it on
its own, but how does VFS allow such a race?
Comment 5 Alexey Dobriyan 2009-11-15 17:11:47 UTC
commit ad3960243e55320d74195fb85c975e0a8cc4466c
"fs: pipe.c null pointer dereference"