* [patch for 2.6.29? 1/3] pipe_rdwr_fasync: fix the error handling to prevent the leak/crash
@ 2009-03-04 20:12 akpm
2009-03-04 20:51 ` Oleg Nesterov
0 siblings, 1 reply; 3+ messages in thread
From: akpm @ 2009-03-04 20:12 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, akpm, oleg, andi, corbet, stable, viro
From: Oleg Nesterov <oleg@redhat.com>
If the second fasync_helper() fails, pipe_rdwr_fasync() returns the error
but leaves the file on ->fasync_readers.
This was always wrong, but since 233e70f4228e78eb2f80dc6650f65d3ae3dbf17c
"saner FASYNC handling on file close" we have the new problem. Because in
this case setfl() doesn't set FASYNC bit, __fput() will not do
->fasync(0), and we leak fasync_struct with ->fa_file pointing to the
freed file.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: <stable@kernel.org> [2.6.28.x]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
fs/pipe.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff -puN fs/pipe.c~pipe_rdwr_fasync-fix-the-error-handling-to-prevent-the-leak-crash fs/pipe.c
--- a/fs/pipe.c~pipe_rdwr_fasync-fix-the-error-handling-to-prevent-the-leak-crash
+++ a/fs/pipe.c
@@ -693,11 +693,12 @@ pipe_rdwr_fasync(int fd, struct file *fi
int retval;
mutex_lock(&inode->i_mutex);
-
retval = fasync_helper(fd, filp, on, &pipe->fasync_readers);
- if (retval >= 0)
+ if (retval >= 0) {
retval = fasync_helper(fd, filp, on, &pipe->fasync_writers);
-
+ if (retval < 0) /* this can happen only if on == T */
+ fasync_helper(-1, filp, 0, &pipe->fasync_readers);
+ }
mutex_unlock(&inode->i_mutex);
return retval;
}
_
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch for 2.6.29? 1/3] pipe_rdwr_fasync: fix the error handling to prevent the leak/crash
2009-03-04 20:12 [patch for 2.6.29? 1/3] pipe_rdwr_fasync: fix the error handling to prevent the leak/crash akpm
@ 2009-03-04 20:51 ` Oleg Nesterov
2009-03-13 5:02 ` [stable] " Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Oleg Nesterov @ 2009-03-04 20:51 UTC (permalink / raw)
To: akpm; +Cc: viro, linux-fsdevel, andi, corbet, stable
On 03/04, Andrew Morton wrote:
>
> From: Oleg Nesterov <oleg@redhat.com>
>
> If the second fasync_helper() fails, pipe_rdwr_fasync() returns the error
> but leaves the file on ->fasync_readers.
>
> This was always wrong, but since 233e70f4228e78eb2f80dc6650f65d3ae3dbf17c
> "saner FASYNC handling on file close" we have the new problem. Because in
> this case setfl() doesn't set FASYNC bit, __fput() will not do
> ->fasync(0), and we leak fasync_struct with ->fa_file pointing to the
> freed file.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Cc: Al Viro <viro@ZenIV.linux.org.uk>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: <stable@kernel.org> [2.6.28.x]
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Just in case...
This bug is minor. fasync_helper() can only fail if
kmem_cache_alloc(fasync_cache, GFP_KERNEL) fails, this should "never" happen.
Perhaps -stable doesn't need this fix.
>
> fs/pipe.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff -puN fs/pipe.c~pipe_rdwr_fasync-fix-the-error-handling-to-prevent-the-leak-crash fs/pipe.c
> --- a/fs/pipe.c~pipe_rdwr_fasync-fix-the-error-handling-to-prevent-the-leak-crash
> +++ a/fs/pipe.c
> @@ -693,11 +693,12 @@ pipe_rdwr_fasync(int fd, struct file *fi
> int retval;
>
> mutex_lock(&inode->i_mutex);
> -
> retval = fasync_helper(fd, filp, on, &pipe->fasync_readers);
> - if (retval >= 0)
> + if (retval >= 0) {
> retval = fasync_helper(fd, filp, on, &pipe->fasync_writers);
> -
> + if (retval < 0) /* this can happen only if on == T */
> + fasync_helper(-1, filp, 0, &pipe->fasync_readers);
> + }
> mutex_unlock(&inode->i_mutex);
> return retval;
> }
> _
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [stable] [patch for 2.6.29? 1/3] pipe_rdwr_fasync: fix the error handling to prevent the leak/crash
2009-03-04 20:51 ` Oleg Nesterov
@ 2009-03-13 5:02 ` Greg KH
0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2009-03-13 5:02 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: akpm, linux-fsdevel, stable, andi, viro, corbet
On Wed, Mar 04, 2009 at 09:51:28PM +0100, Oleg Nesterov wrote:
> On 03/04, Andrew Morton wrote:
> >
> > From: Oleg Nesterov <oleg@redhat.com>
> >
> > If the second fasync_helper() fails, pipe_rdwr_fasync() returns the error
> > but leaves the file on ->fasync_readers.
> >
> > This was always wrong, but since 233e70f4228e78eb2f80dc6650f65d3ae3dbf17c
> > "saner FASYNC handling on file close" we have the new problem. Because in
> > this case setfl() doesn't set FASYNC bit, __fput() will not do
> > ->fasync(0), and we leak fasync_struct with ->fa_file pointing to the
> > freed file.
> >
> > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> > Cc: Al Viro <viro@ZenIV.linux.org.uk>
> > Cc: Andi Kleen <andi@firstfloor.org>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: <stable@kernel.org> [2.6.28.x]
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>
> Just in case...
>
> This bug is minor. fasync_helper() can only fail if
> kmem_cache_alloc(fasync_cache, GFP_KERNEL) fails, this should "never" happen.
>
> Perhaps -stable doesn't need this fix.
Can't hurt, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-03-13 5:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-04 20:12 [patch for 2.6.29? 1/3] pipe_rdwr_fasync: fix the error handling to prevent the leak/crash akpm
2009-03-04 20:51 ` Oleg Nesterov
2009-03-13 5:02 ` [stable] " Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.