Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1619603 > unrolled thread

Re: [patch 1/3] mm: protect set_page_dirty() from ongoing truncation

Started byalexander.levin@verizon.com
First post2017-04-10 04:30 +0200
Last post2017-04-10 18:00 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [patch 1/3] mm: protect set_page_dirty() from ongoing truncation alexander.levin@verizon.com - 2017-04-10 04:30 +0200
    Re: [patch 1/3] mm: protect set_page_dirty() from ongoing truncation Jan Kara <jack@suse.cz> - 2017-04-10 14:10 +0200
      Re: [patch 1/3] mm: protect set_page_dirty() from ongoing truncation alexander.levin@verizon.com - 2017-04-10 17:20 +0200
        Re: [patch 1/3] mm: protect set_page_dirty() from ongoing truncation Jan Kara <jack@suse.cz> - 2017-04-10 18:00 +0200

#1619603 — Re: [patch 1/3] mm: protect set_page_dirty() from ongoing truncation

Fromalexander.levin@verizon.com
Date2017-04-10 04:30 +0200
SubjectRe: [patch 1/3] mm: protect set_page_dirty() from ongoing truncation
Message-ID<tuxfA-30f-7@gated-at.bofh.it>
On Fri, Dec 05, 2014 at 09:52:44AM -0500, Johannes Weiner wrote:
> Tejun, while reviewing the code, spotted the following race condition
> between the dirtying and truncation of a page:
> 
> __set_page_dirty_nobuffers()       __delete_from_page_cache()
>   if (TestSetPageDirty(page))
>                                      page->mapping = NULL
> 				     if (PageDirty())
> 				       dec_zone_page_state(page, NR_FILE_DIRTY);
> 				       dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
>     if (page->mapping)
>       account_page_dirtied(page)
>         __inc_zone_page_state(page, NR_FILE_DIRTY);
> 	__inc_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> 
> which results in an imbalance of NR_FILE_DIRTY and BDI_RECLAIMABLE.
> 
> Dirtiers usually lock out truncation, either by holding the page lock
> directly, or in case of zap_pte_range(), by pinning the mapcount with
> the page table lock held.  The notable exception to this rule, though,
> is do_wp_page(), for which this race exists.  However, do_wp_page()
> already waits for a locked page to unlock before setting the dirty
> bit, in order to prevent a race where clear_page_dirty() misses the
> page bit in the presence of dirty ptes.  Upgrade that wait to a fully
> locked set_page_dirty() to also cover the situation explained above.
> 
> Afterwards, the code in set_page_dirty() dealing with a truncation
> race is no longer needed.  Remove it.
> 
> Reported-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Cc: <stable@vger.kernel.org>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Hi Johannes,

I'm seeing the following while fuzzing with trinity on linux-next (I've changed
the WARN to a VM_BUG_ON_PAGE for some extra page info).

[   18.991007] page:ffffea000307c8c0 count:3 mapcount:0 mapping:ffff88010444cbf8 index:0x1^M
[   18.993051] flags: 0x1fffc0000000011(locked|dirty)^M
[   18.993621] raw: 01fffc0000000011 ffff88010444cbf8 0000000000000001 00000003ffffffff^M
[   18.994522] raw: dead000000000100 dead000000000200 0000000000000000 ffff880109c38008^M                                                                     [   18.995418] page dumped because: VM_BUG_ON_PAGE(!PagePrivate(page) && !PageUptodate(page))^M
[   18.996381] page->mem_cgroup:ffff880109c38008^M                                                                                                            [   18.996935] ------------[ cut here ]------------^M                                                                                                         [   18.997483] kernel BUG at mm/page-writeback.c:2486!^M                                                                                                      [   18.998063] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN^M
[   18.998756] Modules linked in:^M                                                                                                                           [   18.999129] CPU: 5 PID: 1388 Comm: trinity-c34 Not tainted 4.11.0-rc5-next-20170407-dirty #12^M                                                            [   19.000117] task: ffff880106ee5d40 task.stack: ffff8800c0f40000^M                                                                                          [   19.000828] RIP: 0010:__set_page_dirty_nobuffers (??:?)
[   19.001491] RSP: 0018:ffff8800c0f47318 EFLAGS: 00010006^M
[   19.002103] RAX: 0000000000000000 RBX: 1ffff100181e8e67 RCX: 0000000000000000^M
[   19.002929] RDX: 0000000000000021 RSI: 1ffff100181e8da7 RDI: ffffed00181e8e58^M
[   19.004806] RBP: ffff8800c0f47440 R08: 3830303833633930 R09: 3130383866666666^M
[   19.005626] R10: dffffc0000000000 R11: 0000000000001491 R12: ffff8800c0f47418^M
[   19.006452] R13: ffffea000307c8c0 R14: ffff88010444cc10 R15: ffff88010444cbf8^M
[   19.007277] FS:  00007ff6a26fb700(0000) GS:ffff88010a340000(0000) knlGS:0000000000000000^M
[   19.008424] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033^M
[   19.009092] CR2: 00007ff6a155267c CR3: 00000000cb301000 CR4: 00000000000406a0^M
[   19.009919] Call Trace:^M
[   19.012266] set_page_dirty (mm/page-writeback.c:2579)
[   19.020028] v9fs_write_end (fs/9p/vfs_addr.c:325)
[   19.022473] generic_perform_write (mm/filemap.c:2842)
[   19.024857] __generic_file_write_iter (mm/filemap.c:2957)
[   19.025830] generic_file_write_iter (./include/linux/fs.h:702 mm/filemap.c:2985)
[   19.028549] __do_readv_writev (./include/linux/fs.h:1734 fs/read_write.c:696 fs/read_write.c:862)
[   19.029924] do_readv_writev (fs/read_write.c:895)
[   19.034044] vfs_writev (fs/read_write.c:921)
[   19.035223] do_writev (fs/read_write.c:955)
[   19.036925] SyS_writev (fs/read_write.c:1024)
[   19.037297] do_syscall_64 (arch/x86/entry/common.c:284)
[   19.042085] entry_SYSCALL64_slow_path (arch/x86/entry/entry_64.S:249)                                                                                      [   19.042608] RIP: 0033:0x7ff6a200a8e9^M                                                                                                                     [   19.043015] RSP: 002b:00007fff78079608 EFLAGS: 00000246 ORIG_RAX: 0000000000000014^M
[   19.044253] RAX: ffffffffffffffda RBX: 0000000000000014 RCX: 00007ff6a200a8e9^M                                                                            [   19.045045] RDX: 0000000000000001 RSI: 0000000002337d60 RDI: 000000000000018b^M
[   19.045835] RBP: 00007ff6a2601000 R08: 000000482a1a83cf R09: fffdffffffffffff^M                                                                            [   19.046627] R10: 0012536735f82cf7 R11: 0000000000000246 R12: 0000000000000002^M                                                                            [   19.047413] R13: 00007ff6a2601048 R14: 00007ff6a26fb698 R15: 00007ff6a2601000^M                                                                            [ 19.048212] Code: 89 85 f0 fe ff ff e8 39 1b 20 00 8b 85 f0 fe ff ff eb 1a e8 2c bd 12 00 31 c0 eb 11 48 c7 c6 e0 c4 47 83 4c 89 ef e8 39 44 07 00 <0f> 0b 48 ba 00 00 00 00 00 fc ff df 48 c7 04 13 00 00 00 00 48 ^M                                                                                                     All code                                                                                                                                                      ========                                                                                                                                                         0:   89 85 f0 fe ff ff       mov    %eax,-0x110(%rbp)
   6:   e8 39 1b 20 00          callq  0x201b44 
   b:   8b 85 f0 fe ff ff       mov    -0x110(%rbp),%eax 
  11:   eb 1a                   jmp    0x2d 
  13:   e8 2c bd 12 00          callq  0x12bd44 
  18:   31 c0                   xor    %eax,%eax 
  1a:   eb 11                   jmp    0x2d 
  1c:   48 c7 c6 e0 c4 47 83    mov    $0xffffffff8347c4e0,%rsi
  23:   4c 89 ef                mov    %r13,%rdi
  26:   e8 39 44 07 00          callq  0x74464
  2b:*  0f 0b                   ud2             <-- trapping instruction
  2d:   48 ba 00 00 00 00 00    movabs $0xdffffc0000000000,%rdx
  34:   fc ff df
  37:   48 c7 04 13 00 00 00    movq   $0x0,(%rbx,%rdx,1)
  3e:   00
  3f:   48                      rex.W
        ...

Code starting with the faulting instruction
===========================================
   0:   0f 0b                   ud2
   2:   48 ba 00 00 00 00 00    movabs $0xdffffc0000000000,%rdx
   9:   fc ff df
   c:   48 c7 04 13 00 00 00    movq   $0x0,(%rbx,%rdx,1)
  13:   00
  14:   48                      rex.W
        ...
[   19.050311] RIP: __set_page_dirty_nobuffers+0x407/0x450 RSP: ffff8800c0f47318^M (??:?)

-- 

Thanks,
Sasha

[toc] | [next] | [standalone]


#1619859

FromJan Kara <jack@suse.cz>
Date2017-04-10 14:10 +0200
Message-ID<tuGiR-w4-7@gated-at.bofh.it>
In reply to#1619603
On Mon 10-04-17 02:22:33, alexander.levin@verizon.com wrote:
> On Fri, Dec 05, 2014 at 09:52:44AM -0500, Johannes Weiner wrote:
> > Tejun, while reviewing the code, spotted the following race condition
> > between the dirtying and truncation of a page:
> > 
> > __set_page_dirty_nobuffers()       __delete_from_page_cache()
> >   if (TestSetPageDirty(page))
> >                                      page->mapping = NULL
> > 				     if (PageDirty())
> > 				       dec_zone_page_state(page, NR_FILE_DIRTY);
> > 				       dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> >     if (page->mapping)
> >       account_page_dirtied(page)
> >         __inc_zone_page_state(page, NR_FILE_DIRTY);
> > 	__inc_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> > 
> > which results in an imbalance of NR_FILE_DIRTY and BDI_RECLAIMABLE.
> > 
> > Dirtiers usually lock out truncation, either by holding the page lock
> > directly, or in case of zap_pte_range(), by pinning the mapcount with
> > the page table lock held.  The notable exception to this rule, though,
> > is do_wp_page(), for which this race exists.  However, do_wp_page()
> > already waits for a locked page to unlock before setting the dirty
> > bit, in order to prevent a race where clear_page_dirty() misses the
> > page bit in the presence of dirty ptes.  Upgrade that wait to a fully
> > locked set_page_dirty() to also cover the situation explained above.
> > 
> > Afterwards, the code in set_page_dirty() dealing with a truncation
> > race is no longer needed.  Remove it.
> > 
> > Reported-by: Tejun Heo <tj@kernel.org>
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: <stable@vger.kernel.org>
> > Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> 
> Hi Johannes,
> 
> I'm seeing the following while fuzzing with trinity on linux-next (I've changed
> the WARN to a VM_BUG_ON_PAGE for some extra page info).

But this looks more like a bug in 9p which allows v9fs_write_end() to dirty
a !Uptodate page?

								Honza

> 
> [   18.991007] page:ffffea000307c8c0 count:3 mapcount:0 mapping:ffff88010444cbf8 index:0x1^M
> [   18.993051] flags: 0x1fffc0000000011(locked|dirty)^M
> [   18.993621] raw: 01fffc0000000011 ffff88010444cbf8 0000000000000001 00000003ffffffff^M
> [   18.994522] raw: dead000000000100 dead000000000200 0000000000000000 ffff880109c38008^M                                                                     [   18.995418] page dumped because: VM_BUG_ON_PAGE(!PagePrivate(page) && !PageUptodate(page))^M
> [   18.996381] page->mem_cgroup:ffff880109c38008^M                                                                                                            [   18.996935] ------------[ cut here ]------------^M                                                                                                         [   18.997483] kernel BUG at mm/page-writeback.c:2486!^M                                                                                                      [   18.998063] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN^M
> [   18.998756] Modules linked in:^M                                                                                                                           [   18.999129] CPU: 5 PID: 1388 Comm: trinity-c34 Not tainted 4.11.0-rc5-next-20170407-dirty #12^M                                                            [   19.000117] task: ffff880106ee5d40 task.stack: ffff8800c0f40000^M                                                                                          [   19.000828] RIP: 0010:__set_page_dirty_nobuffers (??:?)
> [   19.001491] RSP: 0018:ffff8800c0f47318 EFLAGS: 00010006^M
> [   19.002103] RAX: 0000000000000000 RBX: 1ffff100181e8e67 RCX: 0000000000000000^M
> [   19.002929] RDX: 0000000000000021 RSI: 1ffff100181e8da7 RDI: ffffed00181e8e58^M
> [   19.004806] RBP: ffff8800c0f47440 R08: 3830303833633930 R09: 3130383866666666^M
> [   19.005626] R10: dffffc0000000000 R11: 0000000000001491 R12: ffff8800c0f47418^M
> [   19.006452] R13: ffffea000307c8c0 R14: ffff88010444cc10 R15: ffff88010444cbf8^M
> [   19.007277] FS:  00007ff6a26fb700(0000) GS:ffff88010a340000(0000) knlGS:0000000000000000^M
> [   19.008424] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033^M
> [   19.009092] CR2: 00007ff6a155267c CR3: 00000000cb301000 CR4: 00000000000406a0^M
> [   19.009919] Call Trace:^M
> [   19.012266] set_page_dirty (mm/page-writeback.c:2579)
> [   19.020028] v9fs_write_end (fs/9p/vfs_addr.c:325)
> [   19.022473] generic_perform_write (mm/filemap.c:2842)
> [   19.024857] __generic_file_write_iter (mm/filemap.c:2957)
> [   19.025830] generic_file_write_iter (./include/linux/fs.h:702 mm/filemap.c:2985)
> [   19.028549] __do_readv_writev (./include/linux/fs.h:1734 fs/read_write.c:696 fs/read_write.c:862)
> [   19.029924] do_readv_writev (fs/read_write.c:895)
> [   19.034044] vfs_writev (fs/read_write.c:921)
> [   19.035223] do_writev (fs/read_write.c:955)
> [   19.036925] SyS_writev (fs/read_write.c:1024)
> [   19.037297] do_syscall_64 (arch/x86/entry/common.c:284)
> [   19.042085] entry_SYSCALL64_slow_path (arch/x86/entry/entry_64.S:249)                                                                                      [   19.042608] RIP: 0033:0x7ff6a200a8e9^M                                                                                                                     [   19.043015] RSP: 002b:00007fff78079608 EFLAGS: 00000246 ORIG_RAX: 0000000000000014^M
> [   19.044253] RAX: ffffffffffffffda RBX: 0000000000000014 RCX: 00007ff6a200a8e9^M                                                                            [   19.045045] RDX: 0000000000000001 RSI: 0000000002337d60 RDI: 000000000000018b^M
> [   19.045835] RBP: 00007ff6a2601000 R08: 000000482a1a83cf R09: fffdffffffffffff^M                                                                            [   19.046627] R10: 0012536735f82cf7 R11: 0000000000000246 R12: 0000000000000002^M                                                                            [   19.047413] R13: 00007ff6a2601048 R14: 00007ff6a26fb698 R15: 00007ff6a2601000^M                                                                            [ 19.048212] Code: 89 85 f0 fe ff ff e8 39 1b 20 00 8b 85 f0 fe ff ff eb 1a e8 2c bd 12 00 31 c0 eb 11 48 c7 c6 e0 c4 47 83 4c 89 ef e8 39 44 07 00 <0f> 0b 48 ba 00 00 00 00 00 fc ff df 48 c7 04 13 00 00 00 00 48 ^M                                                                                                     All code                                                                                                                                                      ========                                                                                                                                                         0:   89 85 f0 fe ff ff       mov    %eax,-0x110(%rbp)
>    6:   e8 39 1b 20 00          callq  0x201b44 
>    b:   8b 85 f0 fe ff ff       mov    -0x110(%rbp),%eax 
>   11:   eb 1a                   jmp    0x2d 
>   13:   e8 2c bd 12 00          callq  0x12bd44 
>   18:   31 c0                   xor    %eax,%eax 
>   1a:   eb 11                   jmp    0x2d 
>   1c:   48 c7 c6 e0 c4 47 83    mov    $0xffffffff8347c4e0,%rsi
>   23:   4c 89 ef                mov    %r13,%rdi
>   26:   e8 39 44 07 00          callq  0x74464
>   2b:*  0f 0b                   ud2             <-- trapping instruction
>   2d:   48 ba 00 00 00 00 00    movabs $0xdffffc0000000000,%rdx
>   34:   fc ff df
>   37:   48 c7 04 13 00 00 00    movq   $0x0,(%rbx,%rdx,1)
>   3e:   00
>   3f:   48                      rex.W
>         ...
> 
> Code starting with the faulting instruction
> ===========================================
>    0:   0f 0b                   ud2
>    2:   48 ba 00 00 00 00 00    movabs $0xdffffc0000000000,%rdx
>    9:   fc ff df
>    c:   48 c7 04 13 00 00 00    movq   $0x0,(%rbx,%rdx,1)
>   13:   00
>   14:   48                      rex.W
>         ...
> [   19.050311] RIP: __set_page_dirty_nobuffers+0x407/0x450 RSP: ffff8800c0f47318^M (??:?)
> 
> -- 
> 
> Thanks,
> Sasha
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]


#1620024

Fromalexander.levin@verizon.com
Date2017-04-10 17:20 +0200
Message-ID<tuJgK-2nZ-25@gated-at.bofh.it>
In reply to#1619859
On Mon, Apr 10, 2017 at 02:06:38PM +0200, Jan Kara wrote:
> On Mon 10-04-17 02:22:33, alexander.levin@verizon.com wrote:
> > On Fri, Dec 05, 2014 at 09:52:44AM -0500, Johannes Weiner wrote:
> > > Tejun, while reviewing the code, spotted the following race condition
> > > between the dirtying and truncation of a page:
> > > 
> > > __set_page_dirty_nobuffers()       __delete_from_page_cache()
> > >   if (TestSetPageDirty(page))
> > >                                      page->mapping = NULL
> > > 				     if (PageDirty())
> > > 				       dec_zone_page_state(page, NR_FILE_DIRTY);
> > > 				       dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> > >     if (page->mapping)
> > >       account_page_dirtied(page)
> > >         __inc_zone_page_state(page, NR_FILE_DIRTY);
> > > 	__inc_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> > > 
> > > which results in an imbalance of NR_FILE_DIRTY and BDI_RECLAIMABLE.
> > > 
> > > Dirtiers usually lock out truncation, either by holding the page lock
> > > directly, or in case of zap_pte_range(), by pinning the mapcount with
> > > the page table lock held.  The notable exception to this rule, though,
> > > is do_wp_page(), for which this race exists.  However, do_wp_page()
> > > already waits for a locked page to unlock before setting the dirty
> > > bit, in order to prevent a race where clear_page_dirty() misses the
> > > page bit in the presence of dirty ptes.  Upgrade that wait to a fully
> > > locked set_page_dirty() to also cover the situation explained above.
> > > 
> > > Afterwards, the code in set_page_dirty() dealing with a truncation
> > > race is no longer needed.  Remove it.
> > > 
> > > Reported-by: Tejun Heo <tj@kernel.org>
> > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > > Cc: <stable@vger.kernel.org>
> > > Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > 
> > Hi Johannes,
> > 
> > I'm seeing the following while fuzzing with trinity on linux-next (I've changed
> > the WARN to a VM_BUG_ON_PAGE for some extra page info).
> 
> But this looks more like a bug in 9p which allows v9fs_write_end() to dirty
> a !Uptodate page?

I thought that 77469c3f5 ("9p: saner ->write_end() on failing copy into
non-uptodate page") prevented from that happening, but that's actually the
change that's causing it (I ended up misreading it last night).

Will fix it as follows:

diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c 
index adaf6f6..be84c0c 100644 
--- a/fs/9p/vfs_addr.c 
+++ b/fs/9p/vfs_addr.c 
@@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct address_space *mapping, 
  
        p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); 
  
-       if (unlikely(copied < len && !PageUptodate(page))) { 
-               copied = 0; 
-               goto out; 
+       if (!PageUptodate(page)) { 
+               if (unlikely(copied < len)) { 
+                       copied = 0;
+                       goto out; 
+               } else { 
+                       SetPageUptodate(page); 
+               } 
        } 
        /* 
         * No need to use i_size_read() here, the i_size
 
-- 

Thanks,
Sasha

[toc] | [prev] | [next] | [standalone]


#1620150

FromJan Kara <jack@suse.cz>
Date2017-04-10 18:00 +0200
Message-ID<tuJTv-2D4-83@gated-at.bofh.it>
In reply to#1620024
On Mon 10-04-17 15:07:58, alexander.levin@verizon.com wrote:
> On Mon, Apr 10, 2017 at 02:06:38PM +0200, Jan Kara wrote:
> > On Mon 10-04-17 02:22:33, alexander.levin@verizon.com wrote:
> > > On Fri, Dec 05, 2014 at 09:52:44AM -0500, Johannes Weiner wrote:
> > > > Tejun, while reviewing the code, spotted the following race condition
> > > > between the dirtying and truncation of a page:
> > > > 
> > > > __set_page_dirty_nobuffers()       __delete_from_page_cache()
> > > >   if (TestSetPageDirty(page))
> > > >                                      page->mapping = NULL
> > > > 				     if (PageDirty())
> > > > 				       dec_zone_page_state(page, NR_FILE_DIRTY);
> > > > 				       dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> > > >     if (page->mapping)
> > > >       account_page_dirtied(page)
> > > >         __inc_zone_page_state(page, NR_FILE_DIRTY);
> > > > 	__inc_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> > > > 
> > > > which results in an imbalance of NR_FILE_DIRTY and BDI_RECLAIMABLE.
> > > > 
> > > > Dirtiers usually lock out truncation, either by holding the page lock
> > > > directly, or in case of zap_pte_range(), by pinning the mapcount with
> > > > the page table lock held.  The notable exception to this rule, though,
> > > > is do_wp_page(), for which this race exists.  However, do_wp_page()
> > > > already waits for a locked page to unlock before setting the dirty
> > > > bit, in order to prevent a race where clear_page_dirty() misses the
> > > > page bit in the presence of dirty ptes.  Upgrade that wait to a fully
> > > > locked set_page_dirty() to also cover the situation explained above.
> > > > 
> > > > Afterwards, the code in set_page_dirty() dealing with a truncation
> > > > race is no longer needed.  Remove it.
> > > > 
> > > > Reported-by: Tejun Heo <tj@kernel.org>
> > > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > > > Cc: <stable@vger.kernel.org>
> > > > Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > 
> > > Hi Johannes,
> > > 
> > > I'm seeing the following while fuzzing with trinity on linux-next (I've changed
> > > the WARN to a VM_BUG_ON_PAGE for some extra page info).
> > 
> > But this looks more like a bug in 9p which allows v9fs_write_end() to dirty
> > a !Uptodate page?
> 
> I thought that 77469c3f5 ("9p: saner ->write_end() on failing copy into
> non-uptodate page") prevented from that happening, but that's actually the
> change that's causing it (I ended up misreading it last night).
> 
> Will fix it as follows:

Yep, this looks good to me, although I'd find it more future-proof if we
had that SetPageUptodate() additionally guarded a by len == PAGE_SIZE
check.

								Honza

> 
> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c 
> index adaf6f6..be84c0c 100644 
> --- a/fs/9p/vfs_addr.c 
> +++ b/fs/9p/vfs_addr.c 
> @@ -310,9 +310,13 @@ static int v9fs_write_end(struct file *filp, struct address_space *mapping, 
>   
>         p9_debug(P9_DEBUG_VFS, "filp %p, mapping %p\n", filp, mapping); 
>   
> -       if (unlikely(copied < len && !PageUptodate(page))) { 
> -               copied = 0; 
> -               goto out; 
> +       if (!PageUptodate(page)) { 
> +               if (unlikely(copied < len)) { 
> +                       copied = 0;
> +                       goto out; 
> +               } else { 
> +                       SetPageUptodate(page); 
> +               } 
>         } 
>         /* 
>          * No need to use i_size_read() here, the i_size
>  
> -- 
> 
> Thanks,
> Sasha
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web