2013-01-29 23:27:57

by Jan Kara

[permalink] [raw]
Subject: [PATCH 0/4 v2] Fix possible use after free with AIO


Hi,

since nobody seem to have picked up any of the patches (at least
nobody told me so), I've added acks I received to the patches and I'm
sending them to Al for merging. Al, can you please merge these? Thanks.

Honza

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs


2013-01-29 23:28:17

by Jan Kara

[permalink] [raw]
Subject: [PATCH 4/4] fs: Fix possible use-after-free with AIO

Running AIO is pinning inode in memory using file reference. Once AIO
is completed using aio_complete(), file reference is put and inode can
be freed from memory. So we have to be sure that calling aio_complete()
is the last thing we do with the inode.

CC: Christoph Hellwig <[email protected]>
CC: Jens Axboe <[email protected]>
CC: Jeff Moyer <[email protected]>
CC: [email protected]
Acked-by: Jeff Moyer <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
---
fs/direct-io.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index cf5b44b..f853263 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -261,9 +261,9 @@ static ssize_t dio_complete(struct dio *dio, loff_t offset, ssize_t ret, bool is
dio->end_io(dio->iocb, offset, transferred,
dio->private, ret, is_async);
} else {
+ inode_dio_done(dio->inode);
if (is_async)
aio_complete(dio->iocb, ret, 0);
- inode_dio_done(dio->inode);
}

return ret;
--
1.7.1


2013-01-29 23:28:17

by Jan Kara

[permalink] [raw]
Subject: [PATCH 2/4] xfs: Fix possible use-after-free with AIO

Running AIO is pinning inode in memory using file reference. Once AIO
is completed using aio_complete(), file reference is put and inode can
be freed from memory. So we have to be sure that calling aio_complete()
is the last thing we do with the inode.

CC: [email protected]
CC: Ben Myers <[email protected]>
CC: [email protected]
Reviewed-by: Ben Myers <[email protected]>
Acked-by: Jeff Moyer <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
---
fs/xfs/xfs_aops.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 4111a40..5f707e5 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -86,11 +86,11 @@ xfs_destroy_ioend(
}

if (ioend->io_iocb) {
+ inode_dio_done(ioend->io_inode);
if (ioend->io_isasync) {
aio_complete(ioend->io_iocb, ioend->io_error ?
ioend->io_error : ioend->io_result, 0);
}
- inode_dio_done(ioend->io_inode);
}

mempool_free(ioend, xfs_ioend_pool);
--
1.7.1


2013-01-29 23:28:17

by Jan Kara

[permalink] [raw]
Subject: [PATCH 3/4] ocfs2: Fix possible use-after-free with AIO

Running AIO is pinning inode in memory using file reference. Once AIO
is completed using aio_complete(), file reference is put and inode can
be freed from memory. So we have to be sure that calling aio_complete()
is the last thing we do with the inode.

CC: Joel Becker <[email protected]>
CC: [email protected]
CC: [email protected]
Acked-by: Jeff Moyer <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
---
fs/ocfs2/aops.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 6577432..340bd02 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -593,9 +593,9 @@ static void ocfs2_dio_end_io(struct kiocb *iocb,
level = ocfs2_iocb_rw_locked_level(iocb);
ocfs2_rw_unlock(inode, level);

+ inode_dio_done(inode);
if (is_async)
aio_complete(iocb, ret, 0);
- inode_dio_done(inode);
}

/*
--
1.7.1


2013-01-29 23:27:58

by Jan Kara

[permalink] [raw]
Subject: [PATCH 1/4] ext4: Fix possible use-after-free with AIO

Running AIO is pinning inode in memory using file reference. Once AIO
is completed using aio_complete(), file reference is put and inode can
be freed from memory. So we have to be sure that calling aio_complete()
is the last thing we do with the inode.

CC: [email protected]
CC: "Theodore Ts'o" <[email protected]>
CC: [email protected]
Reviewed-by: Carlos Maiolino <[email protected]>
Acked-by: Jeff Moyer <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
---
fs/ext4/inode.c | 2 +-
fs/ext4/page-io.c | 9 ++++-----
2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index cbfe13b..ba06638 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2977,9 +2977,9 @@ static void ext4_end_io_dio(struct kiocb *iocb, loff_t offset,
if (!(io_end->flag & EXT4_IO_END_UNWRITTEN)) {
ext4_free_io_end(io_end);
out:
+ inode_dio_done(inode);
if (is_async)
aio_complete(iocb, ret, 0);
- inode_dio_done(inode);
return;
}

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 0016fbc..b42d04f 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -103,14 +103,13 @@ static int ext4_end_io(ext4_io_end_t *io)
"(inode %lu, offset %llu, size %zd, error %d)",
inode->i_ino, offset, size, ret);
}
- if (io->iocb)
- aio_complete(io->iocb, io->result, 0);
-
- if (io->flag & EXT4_IO_END_DIRECT)
- inode_dio_done(inode);
/* Wake up anyone waiting on unwritten extent conversion */
if (atomic_dec_and_test(&EXT4_I(inode)->i_unwritten))
wake_up_all(ext4_ioend_wq(inode));
+ if (io->flag & EXT4_IO_END_DIRECT)
+ inode_dio_done(inode);
+ if (io->iocb)
+ aio_complete(io->iocb, io->result, 0);
return ret;
}

--
1.7.1

2013-01-30 00:57:01

by Ben Myers

[permalink] [raw]
Subject: Re: [PATCH 2/4] xfs: Fix possible use-after-free with AIO

Hi Jan,

On Wed, Jan 30, 2013 at 12:27:59AM +0100, Jan Kara wrote:
> Running AIO is pinning inode in memory using file reference. Once AIO
> is completed using aio_complete(), file reference is put and inode can
> be freed from memory. So we have to be sure that calling aio_complete()
> is the last thing we do with the inode.
>
> CC: [email protected]
> CC: Ben Myers <[email protected]>
> CC: [email protected]
> Reviewed-by: Ben Myers <[email protected]>
> Acked-by: Jeff Moyer <[email protected]>
> Signed-off-by: Jan Kara <[email protected]>

We picked this up in the xfs tree. Sorry to keep you hanging.

Regards,
Ben

2013-01-30 03:51:25

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/4] ext4: Fix possible use-after-free with AIO

On Wed, Jan 30, 2013 at 12:27:58AM +0100, Jan Kara wrote:
> Running AIO is pinning inode in memory using file reference. Once AIO
> is completed using aio_complete(), file reference is put and inode can
> be freed from memory. So we have to be sure that calling aio_complete()
> is the last thing we do with the inode.
>
> CC: [email protected]
> CC: "Theodore Ts'o" <[email protected]>
> CC: [email protected]
> Reviewed-by: Carlos Maiolino <[email protected]>
> Acked-by: Jeff Moyer <[email protected]>
> Signed-off-by: Jan Kara <[email protected]>

I've picked up the ext4 change. Sorry for not getting to this sooner.

- Ted


2013-01-30 14:45:38

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 0/4 v2] Fix possible use after free with AIO

On Wed, Jan 30, 2013 at 12:27:57AM +0100, Jan Kara wrote:
>
> Hi,
>
> since nobody seem to have picked up any of the patches (at least
> nobody told me so), I've added acks I received to the patches and I'm
> sending them to Al for merging. Al, can you please merge these? Thanks.

VFS part (4/4) picked; IMO at least ext* and xfs should go through the
filesystem trees. If ocfs2 folks don't pick fs/ocfs2 part, I'll grab
it as well - not sure how active their tree is these days...

2013-01-31 02:11:19

by Zefan Li

[permalink] [raw]
Subject: Re: [PATCH 0/4 v2] Fix possible use after free with AIO

On 2013/1/30 22:45, Al Viro wrote:
> On Wed, Jan 30, 2013 at 12:27:57AM +0100, Jan Kara wrote:
>>
>> Hi,
>>
>> since nobody seem to have picked up any of the patches (at least
>> nobody told me so), I've added acks I received to the patches and I'm
>> sending them to Al for merging. Al, can you please merge these? Thanks.
>
> VFS part (4/4) picked; IMO at least ext* and xfs should go through the
> filesystem trees. If ocfs2 folks don't pick fs/ocfs2 part, I'll grab
> it as well - not sure how active their tree is these days...

I would say just grab it.

We've been sending bug fixes to ocfs2 mailing list since Aug 2012, but none
was pushed into upstream.


2013-01-31 19:31:57

by Joel Becker

[permalink] [raw]
Subject: Re: [PATCH 0/4 v2] Fix possible use after free with AIO

On Wed, Jan 30, 2013 at 02:45:35PM +0000, Al Viro wrote:
> On Wed, Jan 30, 2013 at 12:27:57AM +0100, Jan Kara wrote:
> >
> > Hi,
> >
> > since nobody seem to have picked up any of the patches (at least
> > nobody told me so), I've added acks I received to the patches and I'm
> > sending them to Al for merging. Al, can you please merge these? Thanks.
>
> VFS part (4/4) picked; IMO at least ext* and xfs should go through the
> filesystem trees. If ocfs2 folks don't pick fs/ocfs2 part, I'll grab
> it as well - not sure how active their tree is these days...

I thought akpm had this through -mm. I acked it there.

Joel

> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--

"Same dancers in the same old shoes.
You get too careful with the steps you choose.
You don't care about winning but you don't want to lose
After the thrill is gone."

http://www.jlbec.org/
[email protected]