Bug 14416

Summary:	Null pointer dereference in fs/pipe.c
Product:	File System	Reporter:	Earl Chew (earl_chew)
Component:	Other	Assignee:	Alexey Dobriyan (adobriyan)
Status:	RESOLVED CODE_FIX
Severity:	normal	CC:	adobriyan, stsp2
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	2.6.21	Subsystem:
Regression:	No	Bisected commit-id:
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 3 Earl Chew 2009-11-06 23:54:52 UTC

See http://git.kernel.org/?p=linux/kernel/git/stable/stable-queue.git;a=blob;f=queue-2.6.31/fs-pipe.c-null-pointer-dereference.patch;h=5378492ad11d88de03755c3dcb75449b5a2d75a0;hb=36e97dec52821f76536a25b763e320eb7434c2a5

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"