On 03/04, Andrew Morton wrote:
>
> From: Oleg Nesterov <oleg@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Cc: Andi Kleen <andi@xxxxxxxxxxxxxx>
> Cc: Jonathan Corbet <corbet@xxxxxxx>
> Cc: <stable@xxxxxxxxxx> [2.6.28.x]
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
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;
> }
> _
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
