APPLIED: NAK: [Patch][Trusty SRU] ext4: prevent bugon on race between write/fcntl

Brad Figg brad.figg at canonical.com
Wed Feb 11 01:36:06 UTC 2015


On Wed, Feb 11, 2015 at 07:12:37AM +0800, Ming Lei wrote:
> On Wed, Feb 11, 2015 at 12:03 AM, Brad Figg <brad.figg at canonical.com> wrote:
> > On Mon, Feb 09, 2015 at 11:25:35AM +0800, Ming Lei wrote:
> >> From: Dmitry Monakhov <dmonakhov at openvz.org>
> >>
> >> Buglink:
> >>       https://bugs.launchpad.net/bugs/1418284
> >>
> >> Upstream commit:
> >>       a41537e69b4aa43f0fe(ext4: prevent bugon on race between write/fcntl)
> >>
> >> Patch for 3.14:
> >>       http://git.kernel.org/cgit/linux/kernel/git/stable/stable-queue.git/tree/queue-3.14/ext4-prevent-bugon-on-race-between-write-fcntl.patch
> >>
> >> From: Dmitry Monakhov <dmonakhov at openvz.org>
> >>
> >> commit a41537e69b4aa43f0fea02498c2595a81267383b upstream.
> >>
> >> O_DIRECT flags can be toggeled via fcntl(F_SETFL). But this value checked
> >> twice inside ext4_file_write_iter() and __generic_file_write() which
> >> result in BUG_ON inside ext4_direct_IO.
> >>
> >> Let's initialize iocb->private unconditionally.
> >>
> >> TESTCASE: xfstest:generic/036  https://patchwork.ozlabs.org/patch/402445/
> >>
> >> #TYPICAL STACK TRACE:
> >> kernel BUG at fs/ext4/inode.c:2960!
> >> invalid opcode: 0000 [#1] SMP
> >> Modules linked in: brd iTCO_wdt lpc_ich mfd_core igb ptp dm_mirror dm_region_hash dm_log dm_mod
> >> CPU: 6 PID: 5505 Comm: aio-dio-fcntl-r Not tainted 3.17.0-rc2-00176-gff5c017 #161
> >> Hardware name: Intel Corporation W2600CR/W2600CR, BIOS SE5C600.86B.99.99.x028.061320111235 06/13/2011
> >> task: ffff88080e95a7c0 ti: ffff88080f908000 task.ti: ffff88080f908000
> >> RIP: 0010:[<ffffffff811fabf2>]  [<ffffffff811fabf2>] ext4_direct_IO+0x162/0x3d0
> >> RSP: 0018:ffff88080f90bb58  EFLAGS: 00010246
> >> RAX: 0000000000000400 RBX: ffff88080fdb2a28 RCX: 00000000a802c818
> >> RDX: 0000040000080000 RSI: ffff88080d8aeb80 RDI: 0000000000000001
> >> RBP: ffff88080f90bbc8 R08: 0000000000000000 R09: 0000000000001581
> >> R10: 0000000000000000 R11: 0000000000000000 R12: ffff88080d8aeb80
> >> R13: ffff88080f90bbf8 R14: ffff88080fdb28c8 R15: ffff88080fdb2a28
> >> FS:  00007f23b2055700(0000) GS:ffff880818400000(0000) knlGS:0000000000000000
> >> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> CR2: 00007f23b2045000 CR3: 000000080cedf000 CR4: 00000000000407e0
> >> Stack:
> >>  ffff88080f90bb98 0000000000000000 7ffffffffffffffe ffff88080fdb2c30
> >>  0000000000000200 0000000000000200 0000000000000001 0000000000000200
> >>  ffff88080f90bbc8 ffff88080fdb2c30 ffff88080f90be08 0000000000000200
> >> Call Trace:
> >>  [<ffffffff8112ca9d>] generic_file_direct_write+0xed/0x180
> >>  [<ffffffff8112f2b2>] __generic_file_write_iter+0x222/0x370
> >>  [<ffffffff811f495b>] ext4_file_write_iter+0x34b/0x400
> >>  [<ffffffff811bd709>] ? aio_run_iocb+0x239/0x410
> >>  [<ffffffff811bd709>] ? aio_run_iocb+0x239/0x410
> >>  [<ffffffff810990e5>] ? local_clock+0x25/0x30
> >>  [<ffffffff810abd94>] ? __lock_acquire+0x274/0x700
> >>  [<ffffffff811f4610>] ? ext4_unwritten_wait+0xb0/0xb0
> >>  [<ffffffff811bd756>] aio_run_iocb+0x286/0x410
> >>  [<ffffffff810990e5>] ? local_clock+0x25/0x30
> >>  [<ffffffff810ac359>] ? lock_release_holdtime+0x29/0x190
> >>  [<ffffffff811bc05b>] ? lookup_ioctx+0x4b/0xf0
> >>  [<ffffffff811bde3b>] do_io_submit+0x55b/0x740
> >>  [<ffffffff811bdcaa>] ? do_io_submit+0x3ca/0x740
> >>  [<ffffffff811be030>] SyS_io_submit+0x10/0x20
> >>  [<ffffffff815ce192>] system_call_fastpath+0x16/0x1b
> >> Code: 01 48 8b 80 f0 01 00 00 48 8b 18 49 8b 45 10 0f 85 f1 01 00 00 48 03 45 c8 48 3b 43 48 0f 8f e3 01 00 00 49 83 7c
> >> 24 18 00 75 04 <0f> 0b eb fe f0 ff 83 ec 01 00 00 49 8b 44 24 18 8b 00 85 c0 89
> >> RIP  [<ffffffff811fabf2>] ext4_direct_IO+0x162/0x3d0
> >>  RSP <ffff88080f90bb58>
> >>
> >> Reported-by: Sasha Levin <sasha.levin at oracle.com>
> >> Signed-off-by: Theodore Ts'o <tytso at mit.edu>
> >> Signed-off-by: Dmitry Monakhov <dmonakhov at openvz.org>
> >> [hujianyang: Backported to 3.10
> >>  - Move initialization of iocb->private to ext4_file_write() as we don't
> >>    have ext4_file_write_iter(), which is introduced by commit 9b884164.
> >>  - Adjust context to make 'overwrite' changes apply to ext4_file_dio_write()
> >>    as ext4_file_dio_write() is not move into ext4_file_write()]
> >> Signed-off-by: hujianyang <hujianyang at huawei.com>
> >> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> >> Signed-off-by: Ming Lei <ming.lei at canonical.com>
> >>
> >> ---
> >>  fs/ext4/file.c |    8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> --- a/fs/ext4/file.c
> >> +++ b/fs/ext4/file.c
> >> @@ -100,7 +100,7 @@ ext4_file_dio_write(struct kiocb *iocb,
> >>       struct blk_plug plug;
> >>       int unaligned_aio = 0;
> >>       ssize_t ret;
> >> -     int overwrite = 0;
> >> +     int *overwrite = iocb->private;
> >>       size_t length = iov_length(iov, nr_segs);
> >>
> >>       if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS) &&
> >> @@ -118,8 +118,6 @@ ext4_file_dio_write(struct kiocb *iocb,
> >>       mutex_lock(&inode->i_mutex);
> >>       blk_start_plug(&plug);
> >>
> >> -     iocb->private = &overwrite;
> >> -
> >>       /* check whether we do a DIO overwrite or not */
> >>       if (ext4_should_dioread_nolock(inode) && !unaligned_aio &&
> >>           !file->f_mapping->nrpages && pos + length <= i_size_read(inode)) {
> >> @@ -143,7 +141,7 @@ ext4_file_dio_write(struct kiocb *iocb,
> >>                * So we should check these two conditions.
> >>                */
> >>               if (err == len && (map.m_flags & EXT4_MAP_MAPPED))
> >> -                     overwrite = 1;
> >> +                     *overwrite = 1;
> >>       }
> >>
> >>       ret = __generic_file_aio_write(iocb, iov, nr_segs, &iocb->ki_pos);
> >> @@ -170,6 +168,7 @@ ext4_file_write(struct kiocb *iocb, cons
> >>  {
> >>       struct inode *inode = file_inode(iocb->ki_filp);
> >>       ssize_t ret;
> >> +     int overwrite = 0;
> >>
> >>       /*
> >>        * If we have encountered a bitmap-format file, the size limit
> >> @@ -190,6 +189,7 @@ ext4_file_write(struct kiocb *iocb, cons
> >>               }
> >>       }
> >>
> >> +     iocb->private = &overwrite;
> >>       if (unlikely(iocb->ki_filp->f_flags & O_DIRECT))
> >>               ret = ext4_file_dio_write(iocb, iov, nr_segs, pos);
> >>       else
> >>
> >> --
> >> kernel-team mailing list
> >> kernel-team at lists.ubuntu.com
> >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> >
> > This has been queued in the 3.16 stable tree. We'll pick this up that way.
> 
> If you mean the patch in the link:
> 
>          https://patchwork.ozlabs.org/patch/409372/
> 
> it is basically same with upstream commit, and it can't be applied
> on 3.13 at all because it is against ext3 write_iter changes and the
> "Clean up ext4_file_write" patchset.
> 
> 
> Thanks,
> Ming Lei

I was looking in the wrong upstream stable queue. Sorry about that. I've
applied this to Trusty master-next.

Brad
-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com




More information about the kernel-team mailing list