* 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90
@ 2008-07-30 19:54 Alexey Dobriyan
2008-07-30 20:09 ` Linus Torvalds
0 siblings, 1 reply; 7+ messages in thread
From: Alexey Dobriyan @ 2008-07-30 19:54 UTC (permalink / raw)
To: torvalds, akpm, npiggin; +Cc: linux-kernel
Steps to reproduce:
# while true; do ./ftest03; done
ftest03 from LTP 20080603
BUG: unable to handle kernel paging request at ffff88017c72a008
IP: [<ffffffff8026190e>] iov_iter_advance+0x2e/0x90
PGD 202063 PUD b067 PMD 17def8163 PTE 800000017c72a160
Oops: 0000 [1] PREEMPT SMP DEBUG_PAGEALLOC
CPU 0
Modules linked in: af_packet ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 xt_state iptable_filter iptable_raw ip_tables x_tables nf_conntrack_irc nf_conntrack fuse usblp usbcore
Pid: 3546, comm: ftest03 Not tainted 2.6.27-rc1 #2
RIP: 0010:[<ffffffff8026190e>] [<ffffffff8026190e>] iov_iter_advance+0x2e/0x90
RSP: 0018:ffff88017c75fad8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000800 RCX: 0000000000000000
RDX: 0000000000000080 RSI: 0000000000000000 RDI: ffff88017c75fb78
RBP: ffff88017c75fad8 R08: ffff88017c72a000 R09: 0000000000000000
R10: 0000000000000004 R11: 0000000000000001 R12: 0000000000000800
R13: 000000000006e800 R14: ffff88017f6b7ac8 R15: 0000000000000800
FS: 00007f490298d6f0(0000) GS:ffffffff8051f780(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffff88017c72a008 CR3: 000000017c631000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process ftest03 (pid: 3546, threadinfo ffff88017c75e000, task ffff88017c51e540)
Stack: ffff88017c75fbd8 ffffffff80263452 000000004890c442 0000000000000246
000000007faae000 ffff88017c75fd98 000000000006e800 ffff88017c75fd18
ffff88017efefe00 ffff88017f6b7ac8 ffffffff80422fc0 ffff88017f6b78e0
Call Trace:
[<ffffffff80263452>] generic_file_buffered_write+0x1e2/0x710
[<ffffffff8040cfd0>] ? _spin_unlock+0x30/0x60
[<ffffffff80263e0f>] __generic_file_aio_write_nolock+0x29f/0x450
[<ffffffff80264026>] generic_file_aio_write+0x66/0xd0
[<ffffffff802c9506>] ext3_file_write+0x26/0xc0
[<ffffffff80264250>] ? generic_file_aio_read+0x0/0x670
[<ffffffff802c94e0>] ? ext3_file_write+0x0/0xc0
[<ffffffff8028921b>] do_sync_readv_writev+0xeb/0x130
[<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff802449c0>] ? autoremove_wake_function+0x0/0x40
[<ffffffff80289055>] ? rw_copy_check_uvector+0x95/0x130
[<ffffffff80289953>] do_readv_writev+0xc3/0x120
[<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff802527b5>] ? trace_hardirqs_on_caller+0xd5/0x160
[<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff802899e9>] vfs_writev+0x39/0x60
[<ffffffff80289d60>] sys_writev+0x50/0x90
[<ffffffff8020b65b>] system_call_fastpath+0x16/0x1b
Code: 77 18 48 89 e5 72 11 48 83 7f 08 01 75 11 48 01 77 10 48 29 77 18 c9 c3 0f 0b 0f 1f 00 eb fb 4c 8b 07 48 8b 4f 10 48 85 f6 75 17 <49> 83 78 08 00 75 07 48 83 7f 18 00 75 09 4c 89 07 48 89 4f 10
RIP [<ffffffff8026190e>] iov_iter_advance+0x2e/0x90
RSP <ffff88017c75fad8>
CR2: ffff88017c72a008
0xffffffff8026190e is in iov_iter_advance (mm/filemap.c:1882).
1877
1878 /*
1879 * The !iov->iov_len check ensures we skip over unlikely
1880 * zero-length segments (without overruning the iovec).
1881 */
1882 ===> while (bytes || unlikely(!iov->iov_len && i->count)) {
1883 int copy;
1884
1885 copy = min(bytes, iov->iov_len - base);
1886 BUG_ON(!i->count || i->count < copy);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90 2008-07-30 19:54 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90 Alexey Dobriyan @ 2008-07-30 20:09 ` Linus Torvalds 2008-07-30 21:37 ` Alexey Dobriyan 2008-07-31 2:29 ` Nick Piggin 0 siblings, 2 replies; 7+ messages in thread From: Linus Torvalds @ 2008-07-30 20:09 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: akpm, Nick Piggin, Linux Kernel Mailing List On Wed, 30 Jul 2008, Alexey Dobriyan wrote: > > Steps to reproduce: > > # while true; do ./ftest03; done > > ftest03 from LTP 20080603 Hmm. The oops disassembles to -12: 4c 8b 07 mov (%rdi),%r8 -9: 48 8b 4f 10 mov 0x10(%rdi),%rcx -5: 48 85 f6 test %rsi,%rsi -2: 75 17 jne 0x42 0: 49 83 78 08 00 cmpq $0x0,0x8(%r8) <--- 5: 75 07 jne 0xe 7: 48 83 7f 18 00 cmpq $0x0,0x18(%rdi) c: 75 09 jne 0x17 So it looks like we just overflowed %r8 to a new page and you presumably have DEBUG_PAGEALLOC on. (And yes, I see in the oops that you do) > RIP [<ffffffff8026190e>] iov_iter_advance+0x2e/0x90 > Call Trace: > [<ffffffff80263452>] generic_file_buffered_write+0x1e2/0x710 > [<ffffffff8040cfd0>] ? _spin_unlock+0x30/0x60 > [<ffffffff80263e0f>] __generic_file_aio_write_nolock+0x29f/0x450 > [<ffffffff80264026>] generic_file_aio_write+0x66/0xd0 > [<ffffffff802c9506>] ext3_file_write+0x26/0xc0 > [<ffffffff80264250>] ? generic_file_aio_read+0x0/0x670 > [<ffffffff802c94e0>] ? ext3_file_write+0x0/0xc0 > [<ffffffff8028921b>] do_sync_readv_writev+0xeb/0x130 > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > [<ffffffff802449c0>] ? autoremove_wake_function+0x0/0x40 > [<ffffffff80289055>] ? rw_copy_check_uvector+0x95/0x130 > [<ffffffff80289953>] do_readv_writev+0xc3/0x120 > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > [<ffffffff802527b5>] ? trace_hardirqs_on_caller+0xd5/0x160 > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > [<ffffffff802899e9>] vfs_writev+0x39/0x60 > [<ffffffff80289d60>] sys_writev+0x50/0x90 > [<ffffffff8020b65b>] system_call_fastpath+0x16/0x1b > > 0xffffffff8026190e is in iov_iter_advance (mm/filemap.c:1882). > 1877 > 1878 /* > 1879 * The !iov->iov_len check ensures we skip over unlikely > 1880 * zero-length segments (without overruning the iovec). > 1881 */ > 1882 ===> while (bytes || unlikely(!iov->iov_len && i->count)) { And yes, that oopsing op would be the one that loads 'iov->iov_len'. So it very much looks like iov_iter_advance() advances past the end of the iov array. We've had issues like that before. And I bet it's due to a combination of Nick's commit f7009264c519603b8ec67c881bd368a56703cfc9 ("iov_iter_advance() fix") and 124d3b7041f9a0ca7c43a6293e1cae4576c32fd5 ("fix writev regression: pan hanging unkillable and un-straceable"). It's simply _not_ acceptable to look at iov->iov_len when 'bytes' has gone down to zero, because there may be no 'iov' left! Nick? That said, I do think that we have another issue with iovec's - I think we should strive to always pass in the number of iovec's when we pass a pointer to an iovec, in addition to the bytes. The sad part is that 'iov_iter_advance' actually -has- the count, but it's the byte count remaining, not the iovec's remaining. In this particular case, the trivial fix _may_ be to just change the order of testing iov->iov_len && i->count, but I really think we should also count actual iov entries and pass them around (and keep them updated). So does this (hacky, ugly) patch fix it for you? Linus --- mm/filemap.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 42bbc69..d97d1ad 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1879,7 +1879,7 @@ void iov_iter_advance(struct iov_iter *i, size_t bytes) * The !iov->iov_len check ensures we skip over unlikely * zero-length segments (without overruning the iovec). */ - while (bytes || unlikely(!iov->iov_len && i->count)) { + while (bytes || unlikely(i->count && !iov->iov_len)) { int copy; copy = min(bytes, iov->iov_len - base); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90 2008-07-30 20:09 ` Linus Torvalds @ 2008-07-30 21:37 ` Alexey Dobriyan 2008-07-30 21:44 ` Linus Torvalds 2008-07-31 2:29 ` Nick Piggin 1 sibling, 1 reply; 7+ messages in thread From: Alexey Dobriyan @ 2008-07-30 21:37 UTC (permalink / raw) To: Linus Torvalds; +Cc: akpm, Nick Piggin, Linux Kernel Mailing List On Wed, Jul 30, 2008 at 01:09:21PM -0700, Linus Torvalds wrote: > On Wed, 30 Jul 2008, Alexey Dobriyan wrote: > > > > Steps to reproduce: > > > > # while true; do ./ftest03; done > > > > ftest03 from LTP 20080603 > > Hmm. The oops disassembles to > > -12: 4c 8b 07 mov (%rdi),%r8 > -9: 48 8b 4f 10 mov 0x10(%rdi),%rcx > -5: 48 85 f6 test %rsi,%rsi > -2: 75 17 jne 0x42 > 0: 49 83 78 08 00 cmpq $0x0,0x8(%r8) <--- > 5: 75 07 jne 0xe > 7: 48 83 7f 18 00 cmpq $0x0,0x18(%rdi) > c: 75 09 jne 0x17 > > So it looks like we just overflowed %r8 to a new page and you presumably > have DEBUG_PAGEALLOC on. > > (And yes, I see in the oops that you do) > > > RIP [<ffffffff8026190e>] iov_iter_advance+0x2e/0x90 > > Call Trace: > > [<ffffffff80263452>] generic_file_buffered_write+0x1e2/0x710 > > [<ffffffff8040cfd0>] ? _spin_unlock+0x30/0x60 > > [<ffffffff80263e0f>] __generic_file_aio_write_nolock+0x29f/0x450 > > [<ffffffff80264026>] generic_file_aio_write+0x66/0xd0 > > [<ffffffff802c9506>] ext3_file_write+0x26/0xc0 > > [<ffffffff80264250>] ? generic_file_aio_read+0x0/0x670 > > [<ffffffff802c94e0>] ? ext3_file_write+0x0/0xc0 > > [<ffffffff8028921b>] do_sync_readv_writev+0xeb/0x130 > > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > > [<ffffffff802449c0>] ? autoremove_wake_function+0x0/0x40 > > [<ffffffff80289055>] ? rw_copy_check_uvector+0x95/0x130 > > [<ffffffff80289953>] do_readv_writev+0xc3/0x120 > > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > > [<ffffffff802527b5>] ? trace_hardirqs_on_caller+0xd5/0x160 > > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > > [<ffffffff802899e9>] vfs_writev+0x39/0x60 > > [<ffffffff80289d60>] sys_writev+0x50/0x90 > > [<ffffffff8020b65b>] system_call_fastpath+0x16/0x1b > > > > 0xffffffff8026190e is in iov_iter_advance (mm/filemap.c:1882). > > 1877 > > 1878 /* > > 1879 * The !iov->iov_len check ensures we skip over unlikely > > 1880 * zero-length segments (without overruning the iovec). > > 1881 */ > > 1882 ===> while (bytes || unlikely(!iov->iov_len && i->count)) { > > And yes, that oopsing op would be the one that loads 'iov->iov_len'. > > So it very much looks like iov_iter_advance() advances past the end of the > iov array. We've had issues like that before. And I bet it's due to a > combination of Nick's commit f7009264c519603b8ec67c881bd368a56703cfc9 > ("iov_iter_advance() fix") and 124d3b7041f9a0ca7c43a6293e1cae4576c32fd5 > ("fix writev regression: pan hanging unkillable and un-straceable"). > > It's simply _not_ acceptable to look at iov->iov_len when 'bytes' has gone > down to zero, because there may be no 'iov' left! > > Nick? > > That said, I do think that we have another issue with iovec's - I think we > should strive to always pass in the number of iovec's when we pass a > pointer to an iovec, in addition to the bytes. The sad part is that > 'iov_iter_advance' actually -has- the count, but it's the byte count > remaining, not the iovec's remaining. > > In this particular case, the trivial fix _may_ be to just change the order > of testing iov->iov_len && i->count, but I really think we should also > count actual iov entries and pass them around (and keep them updated). > > So does this (hacky, ugly) patch fix it for you? You forgot "untested". And, yes, it helps. > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1879,7 +1879,7 @@ void iov_iter_advance(struct iov_iter *i, size_t bytes) > * The !iov->iov_len check ensures we skip over unlikely > * zero-length segments (without overruning the iovec). > */ > - while (bytes || unlikely(!iov->iov_len && i->count)) { > + while (bytes || unlikely(i->count && !iov->iov_len)) { > int copy; > > copy = min(bytes, iov->iov_len - base); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90 2008-07-30 21:37 ` Alexey Dobriyan @ 2008-07-30 21:44 ` Linus Torvalds 2008-07-30 22:02 ` Linus Torvalds 0 siblings, 1 reply; 7+ messages in thread From: Linus Torvalds @ 2008-07-30 21:44 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: akpm, Nick Piggin, Linux Kernel Mailing List On Thu, 31 Jul 2008, Alexey Dobriyan wrote: > > > > So does this (hacky, ugly) patch fix it for you? > > You forgot "untested". I'm trying to teach people to just take my patches for granted as being untested. I think there is more shock value when I actually claim one of my patches is actually _tested_, so I guess I'll just emphasize that part in case it ever happens (and ok, it did happen for the cpu_mask thing). > And, yes, it helps. Ok, I'll commit it as obvious, ugly as it may be. And hope that Nick or some other god of iov_iter will think about trying to make these things more reliable and easier to use. Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90 2008-07-30 21:44 ` Linus Torvalds @ 2008-07-30 22:02 ` Linus Torvalds 2008-07-30 22:32 ` Alexey Dobriyan 0 siblings, 1 reply; 7+ messages in thread From: Linus Torvalds @ 2008-07-30 22:02 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: akpm, Nick Piggin, Linux Kernel Mailing List, stable On Wed, 30 Jul 2008, Linus Torvalds wrote: > > Ok, I'll commit it as obvious, ugly as it may be. And hope that Nick or > some other god of iov_iter will think about trying to make these things > more reliable and easier to use. Oh, and I think it's stable material. Unless I misread something, this area hasn't changed since 2.6.25. But somebody should double-check me, that just sounds wrong. stable added to cc, with patch appended. Linus --- commit 94ad374a0751f40d25e22e036c37f7263569d24c Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Wed Jul 30 14:45:12 2008 -0700 Fix off-by-one error in iov_iter_advance() The iov_iter_advance() function would look at the iov->iov_len entry even though it might have iterated over the whole array, and iov was pointing past the end. This would cause DEBUG_PAGEALLOC to trigger a kernel page fault if the allocation was at the end of a page, and the next page was unallocated. The quick fix is to just change the order of the tests: check that there is any iovec data left before we check the iov entry itself. Thanks to Alexey Dobriyan for finding this case, and testing the fix. Reported-and-tested-by: Alexey Dobriyan <adobriyan@gmail.com> Cc: Nick Piggin <npiggin@suse.de> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: <stable@kernel.org> [2.6.25.x, 2.6.26.x] Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- mm/filemap.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 42bbc69..d97d1ad 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1879,7 +1879,7 @@ void iov_iter_advance(struct iov_iter *i, size_t bytes) * The !iov->iov_len check ensures we skip over unlikely * zero-length segments (without overruning the iovec). */ - while (bytes || unlikely(!iov->iov_len && i->count)) { + while (bytes || unlikely(i->count && !iov->iov_len)) { int copy; copy = min(bytes, iov->iov_len - base); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90 2008-07-30 22:02 ` Linus Torvalds @ 2008-07-30 22:32 ` Alexey Dobriyan 0 siblings, 0 replies; 7+ messages in thread From: Alexey Dobriyan @ 2008-07-30 22:32 UTC (permalink / raw) To: Linus Torvalds; +Cc: akpm, Nick Piggin, Linux Kernel Mailing List, stable On Wed, Jul 30, 2008 at 03:02:32PM -0700, Linus Torvalds wrote: > On Wed, 30 Jul 2008, Linus Torvalds wrote: > > > > Ok, I'll commit it as obvious, ugly as it may be. And hope that Nick or > > some other god of iov_iter will think about trying to make these things > > more reliable and easier to use. > > Oh, and I think it's stable material. Unless I misread something, this > area hasn't changed since 2.6.25. But somebody should double-check me, > that just sounds wrong. 2.6.26 fails the same way and patch also seems to help. > commit 94ad374a0751f40d25e22e036c37f7263569d24c > Author: Linus Torvalds <torvalds@linux-foundation.org> > Date: Wed Jul 30 14:45:12 2008 -0700 > > Fix off-by-one error in iov_iter_advance() > > The iov_iter_advance() function would look at the iov->iov_len entry > even though it might have iterated over the whole array, and iov was > pointing past the end. This would cause DEBUG_PAGEALLOC to trigger a > kernel page fault if the allocation was at the end of a page, and the > next page was unallocated. > > The quick fix is to just change the order of the tests: check that there > is any iovec data left before we check the iov entry itself. > > Thanks to Alexey Dobriyan for finding this case, and testing the fix. > > Reported-and-tested-by: Alexey Dobriyan <adobriyan@gmail.com> > Cc: Nick Piggin <npiggin@suse.de> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: <stable@kernel.org> [2.6.25.x, 2.6.26.x] > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > --- > mm/filemap.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index 42bbc69..d97d1ad 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1879,7 +1879,7 @@ void iov_iter_advance(struct iov_iter *i, size_t bytes) > * The !iov->iov_len check ensures we skip over unlikely > * zero-length segments (without overruning the iovec). > */ > - while (bytes || unlikely(!iov->iov_len && i->count)) { > + while (bytes || unlikely(i->count && !iov->iov_len)) { > int copy; > > copy = min(bytes, iov->iov_len - base); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90 2008-07-30 20:09 ` Linus Torvalds 2008-07-30 21:37 ` Alexey Dobriyan @ 2008-07-31 2:29 ` Nick Piggin 1 sibling, 0 replies; 7+ messages in thread From: Nick Piggin @ 2008-07-31 2:29 UTC (permalink / raw) To: Linus Torvalds Cc: Alexey Dobriyan, akpm, Nick Piggin, Linux Kernel Mailing List On Thursday 31 July 2008 06:09, Linus Torvalds wrote: > On Wed, 30 Jul 2008, Alexey Dobriyan wrote: > > Steps to reproduce: > > > > # while true; do ./ftest03; done > > > > ftest03 from LTP 20080603 > > Hmm. The oops disassembles to > > -12: 4c 8b 07 mov (%rdi),%r8 > -9: 48 8b 4f 10 mov 0x10(%rdi),%rcx > -5: 48 85 f6 test %rsi,%rsi > -2: 75 17 jne 0x42 > 0: 49 83 78 08 00 cmpq $0x0,0x8(%r8) <--- > 5: 75 07 jne 0xe > 7: 48 83 7f 18 00 cmpq $0x0,0x18(%rdi) > c: 75 09 jne 0x17 > > So it looks like we just overflowed %r8 to a new page and you presumably > have DEBUG_PAGEALLOC on. > > (And yes, I see in the oops that you do) > > > RIP [<ffffffff8026190e>] iov_iter_advance+0x2e/0x90 > > Call Trace: > > [<ffffffff80263452>] generic_file_buffered_write+0x1e2/0x710 > > [<ffffffff8040cfd0>] ? _spin_unlock+0x30/0x60 > > [<ffffffff80263e0f>] __generic_file_aio_write_nolock+0x29f/0x450 > > [<ffffffff80264026>] generic_file_aio_write+0x66/0xd0 > > [<ffffffff802c9506>] ext3_file_write+0x26/0xc0 > > [<ffffffff80264250>] ? generic_file_aio_read+0x0/0x670 > > [<ffffffff802c94e0>] ? ext3_file_write+0x0/0xc0 > > [<ffffffff8028921b>] do_sync_readv_writev+0xeb/0x130 > > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > > [<ffffffff802449c0>] ? autoremove_wake_function+0x0/0x40 > > [<ffffffff80289055>] ? rw_copy_check_uvector+0x95/0x130 > > [<ffffffff80289953>] do_readv_writev+0xc3/0x120 > > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > > [<ffffffff802527b5>] ? trace_hardirqs_on_caller+0xd5/0x160 > > [<ffffffff8025284d>] ? trace_hardirqs_on+0xd/0x10 > > [<ffffffff802899e9>] vfs_writev+0x39/0x60 > > [<ffffffff80289d60>] sys_writev+0x50/0x90 > > [<ffffffff8020b65b>] system_call_fastpath+0x16/0x1b > > > > 0xffffffff8026190e is in iov_iter_advance (mm/filemap.c:1882). > > 1877 > > 1878 /* > > 1879 * The !iov->iov_len check ensures we skip over > > unlikely 1880 * zero-length segments (without > > overruning the iovec). 1881 */ > > 1882 ===> while (bytes || unlikely(!iov->iov_len && > > i->count)) { > > And yes, that oopsing op would be the one that loads 'iov->iov_len'. > > So it very much looks like iov_iter_advance() advances past the end of the > iov array. We've had issues like that before. And I bet it's due to a > combination of Nick's commit f7009264c519603b8ec67c881bd368a56703cfc9 > ("iov_iter_advance() fix") and 124d3b7041f9a0ca7c43a6293e1cae4576c32fd5 > ("fix writev regression: pan hanging unkillable and un-straceable"). > > It's simply _not_ acceptable to look at iov->iov_len when 'bytes' has gone > down to zero, because there may be no 'iov' left! > > Nick? Thanks Linus, patch looks exactly right. > That said, I do think that we have another issue with iovec's - I think we > should strive to always pass in the number of iovec's when we pass a > pointer to an iovec, in addition to the bytes. The sad part is that > 'iov_iter_advance' actually -has- the count, but it's the byte count > remaining, not the iovec's remaining. > > In this particular case, the trivial fix _may_ be to just change the order > of testing iov->iov_len && i->count, but I really think we should also > count actual iov entries and pass them around (and keep them updated). > > So does this (hacky, ugly) patch fix it for you? > > Linus > > --- > mm/filemap.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index 42bbc69..d97d1ad 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1879,7 +1879,7 @@ void iov_iter_advance(struct iov_iter *i, size_t > bytes) * The !iov->iov_len check ensures we skip over unlikely > * zero-length segments (without overruning the iovec). > */ > - while (bytes || unlikely(!iov->iov_len && i->count)) { > + while (bytes || unlikely(i->count && !iov->iov_len)) { > int copy; > > copy = min(bytes, iov->iov_len - base); > -- ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-07-31 2:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-07-30 19:54 2.6.27-rc1: IP: iov_iter_advance+0x2e/0x90 Alexey Dobriyan 2008-07-30 20:09 ` Linus Torvalds 2008-07-30 21:37 ` Alexey Dobriyan 2008-07-30 21:44 ` Linus Torvalds 2008-07-30 22:02 ` Linus Torvalds 2008-07-30 22:32 ` Alexey Dobriyan 2008-07-31 2:29 ` Nick Piggin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®