Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
Can the current modification method be confirmed? 在 2019/9/16 6:00, Richard Weinberger 写道: > I need to give this another thought
Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
在 2019/9/16 6:00, Richard Weinberger 写道: > On Fri, Aug 16, 2019 at 10:01 AM chengzhihao wrote: >> >>> ubifs_assert(c, p < c->gap_lebs + c->lst.idx_lebs); >> >> I've done 50 problem reproduces on different flash devices and made sure >> that the assertion was not triggered. See record.txt for details. > > Thanks for letting me know. :) > I need to give this another thought, then we can apply it for -rc2. > ACK. :)
Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
On Fri, Aug 16, 2019 at 10:01 AM chengzhihao wrote: > > > ubifs_assert(c, p < c->gap_lebs + c->lst.idx_lebs); > > I've done 50 problem reproduces on different flash devices and made sure that > the assertion was not triggered. See record.txt for details. Thanks for letting me know. :) I need to give this another thought, then we can apply it for -rc2.
答复: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
> ubifs_assert(c, p < c->gap_lebs + c->lst.idx_lebs); I've done 50 problem reproduces on different flash devices and made sure that the assertion was not triggered. See record.txt for details. -邮件原件- 发件人: chengzhihao 发送时间: 2019年8月14日 9:20 收件人: 'Richard Weinberger' 抄送: Richard Weinberger ; Sascha Hauer ; Artem Bityutskiy ; zhangyi (F) ; linux-...@lists.infradead.org; LKML 主题: 答复: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps Sure, I'll do more tests on different machines to check the assertion. I'm trying to understand when this assertion will be triggered. Although I haven't found this assertion be triggered so far in several tests on x86_64(qemu). -邮件原件- 发件人: Richard Weinberger [mailto:richard.weinber...@gmail.com] 发送时间: 2019年8月14日 5:44 收件人: chengzhihao 抄送: Richard Weinberger ; Sascha Hauer ; Artem Bityutskiy ; zhangyi (F) ; linux-...@lists.infradead.org; LKML 主题: Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps On Tue, Jul 30, 2019 at 3:21 AM chengzhihao wrote: > > OK, that's fine, and I will continue to understand more implementation code > related to this part. I think we can go with the realloc() approach for now. Can you please check whether the assert() triggers? -- Thanks, //richard No Log Config 1 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 11, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 2 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 9 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 3 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 9 nandsim: 16MiB, PEB size 16KiB, page size 512KiB, VID offset 0, fastmap enabled, volume size 11MiB 4 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 11, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 5 c->lst.idx_lebs[origin] = 5, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 6 c->lst.idx_lebs[origin] = 5, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 8 nandsim: 16MiB, PEB size 16KiB, page size 512KiB, VID offset 0, fastmap enabled, volume size 11MiB 7 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 11, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 8 c->lst.idx_lebs[origin] = 5, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 8 nandsim: 16MiB, PEB size 16KiB, page size 512KiB, VID offset 0, fastmap enabled, volume size 11MiB 9 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 11, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 10 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 11, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 11 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 12 c->lst.idx_lebs[origin] = 3, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 10 nandsim: 16MiB, PEB size 16KiB, page size 512KiB, VID offset 0, fastmap enabled, volume size 11MiB 13 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 11, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 14 c->lst.idx_lebs[origin] = 5, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 15 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 11, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 16 c->lst.idx_lebs[origin] = 5, c->lst.idx_lebs[curr] = 12, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 17 c->lst.idx_lebs[origin] = 4, c->lst.idx_lebs[curr] = 11, p - c->gap_lebs = 8 mtdram: 16MiB, PEB size 16KiB, fastmap enabled, volume size 11MiB 18 c->lst.idx_lebs[origin] = 6, c->lst.idx_lebs[curr] = 13, p - c
答复: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
Sure, I'll do more tests on different machines to check the assertion. I'm trying to understand when this assertion will be triggered. Although I haven't found this assertion be triggered so far in several tests on x86_64(qemu). -邮件原件- 发件人: Richard Weinberger [mailto:richard.weinber...@gmail.com] 发送时间: 2019年8月14日 5:44 收件人: chengzhihao 抄送: Richard Weinberger ; Sascha Hauer ; Artem Bityutskiy ; zhangyi (F) ; linux-...@lists.infradead.org; LKML 主题: Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps On Tue, Jul 30, 2019 at 3:21 AM chengzhihao wrote: > > OK, that's fine, and I will continue to understand more implementation code > related to this part. I think we can go with the realloc() approach for now. Can you please check whether the assert() triggers? -- Thanks, //richard
Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
On Tue, Jul 30, 2019 at 3:21 AM chengzhihao wrote: > > OK, that's fine, and I will continue to understand more implementation code > related to this part. I think we can go with the realloc() approach for now. Can you please check whether the assert() triggers? -- Thanks, //richard
答复: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
OK, that's fine, and I will continue to understand more implementation code related to this part. - Thanks, Cheng zhihao -邮件原件- 发件人: Richard Weinberger [mailto:richard.weinber...@gmail.com] 发送时间: 2019年7月30日 0:52 收件人: chengzhihao 抄送: Richard Weinberger ; Sascha Hauer ; Artem Bityutskiy ; zhangyi (F) ; linux-...@lists.infradead.org; LKML 主题: Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps On Sat, Jul 20, 2019 at 8:00 AM Zhihao Cheng wrote: > > Running stress-test test_2 in mtd-utils on ubi device, sometimes we > can get following oops message: > > BUG: unable to handle page fault for address: 0140 > #PF: supervisor read access in kernel mode > #PF: error_code(0x) - not-present page > PGD 280a067 P4D 280a067 PUD 0 > Oops: [#1] SMP > CPU: 0 PID: 60 Comm: kworker/u16:1 Kdump: loaded Not tainted 5.2.0 #13 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0 > -0-ga698c8995f-prebuilt.qemu.org 04/01/2014 > Workqueue: writeback wb_workfn (flush-ubifs_0_0) > RIP: 0010:rb_next_postorder+0x2e/0xb0 > Code: 80 db 03 01 48 85 ff 0f 84 97 00 00 00 48 8b 17 48 83 05 bc 80 db > 03 01 48 83 e2 fc 0f 84 82 00 00 00 48 83 05 b2 80 db 03 01 <48> 3b 7a > 10 48 89 d0 74 02 f3 c3 48 8b 52 08 48 83 05 a3 80 db 03 > RSP: 0018:c9887758 EFLAGS: 00010202 > RAX: 888129ae4700 RBX: 888138b08400 RCX: 8081 > RDX: 0130 RSI: 80800024 RDI: 888138b08400 > RBP: 888138b08400 R08: ea0004a6b920 R09: > R10: c9887740 R11: 0001 R12: 888128d48000 > R13: 0800 R14: 011e R15: 07c8 > FS: () GS:88813ba0() > knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 0140 CR3: 00013789d000 CR4: 06f0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > Call Trace: > destroy_old_idx+0x5d/0xa0 [ubifs] > ubifs_tnc_start_commit+0x4fe/0x1380 [ubifs] > do_commit+0x3eb/0x830 [ubifs] > ubifs_run_commit+0xdc/0x1c0 [ubifs] > > Above Oops are due to the slab-out-of-bounds happened in do-while of > function layout_in_gaps indirectly called by ubifs_tnc_start_commit. > In function layout_in_gaps, there is a do-while loop placing index > nodes into the gaps created by obsolete index nodes in non-empty index > LEBs until rest index nodes can totally be placed into pre-allocated > empty LEBs. @c->gap_lebs points to a memory area(integer array) which > records LEB numbers used by 'in-the-gaps' method. Whenever a fitable > index LEB is found, corresponding lnum will be incrementally written > into the memory area pointed by @c->gap_lebs. The size > ((@c->lst.idx_lebs + 1) * sizeof(int)) of memory area is allocated > before do-while loop and can not be changed in the loop. But > @c->lst.idx_lebs could be increased by function ubifs_change_lp > (called by > layout_leb_in_gaps->ubifs_find_dirty_idx_leb->get_idx_gc_leb) during > the loop. So, sometimes oob happens when number of cycles in do-while > loop exceeds the original value of @c->lst.idx_lebs. See detail in > https://bugzilla.kernel.org/show_bug.cgi?id=204229. > This patch fixes oob in layout_in_gaps. > > Signed-off-by: Zhihao Cheng > --- > fs/ubifs/tnc_commit.c | 34 +++--- > 1 file changed, 27 insertions(+), 7 deletions(-) > > diff --git a/fs/ubifs/tnc_commit.c b/fs/ubifs/tnc_commit.c index > a384a0f..234be1c 100644 > --- a/fs/ubifs/tnc_commit.c > +++ b/fs/ubifs/tnc_commit.c > @@ -212,7 +212,7 @@ static int is_idx_node_in_use(struct ubifs_info > *c, union ubifs_key *key, > /** > * layout_leb_in_gaps - layout index nodes using in-the-gaps method. > * @c: UBIFS file-system description object > - * @p: return LEB number here > + * @p: return LEB number in @c->gap_lebs[p] > * > * This function lays out new index nodes for dirty znodes using in-the-gaps > * method of TNC commit. > @@ -221,7 +221,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union > ubifs_key *key, > * This function returns the number of index nodes written into the gaps, or > a > * negative error code on failure. > */ > -static int layout_leb_in_gaps(struct ubifs_info *c, int *p) > +static int layout_leb_in_gaps(struct ubifs_info *c, int p) > { > struct ubifs_scan_leb *sleb; > struct ubifs_scan_node *snod; > @@ -236,7 +236,7 @@ static int layout_leb_in_gaps(struct ubifs_info *c, int > *p) > * filled, however we do
Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
On Mon, Jul 29, 2019 at 6:51 PM Richard Weinberger wrote: > > - ubifs_assert(c, p < c->gap_lebs + c->lst.idx_lebs); > > + ubifs_assert(c, p < c->lst.idx_lebs); I wonder, doesn't this assert trigger too? -- Thanks, //richard
Re: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
On Sat, Jul 20, 2019 at 8:00 AM Zhihao Cheng wrote: > > Running stress-test test_2 in mtd-utils on ubi device, sometimes we can > get following oops message: > > BUG: unable to handle page fault for address: 0140 > #PF: supervisor read access in kernel mode > #PF: error_code(0x) - not-present page > PGD 280a067 P4D 280a067 PUD 0 > Oops: [#1] SMP > CPU: 0 PID: 60 Comm: kworker/u16:1 Kdump: loaded Not tainted 5.2.0 #13 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0 > -0-ga698c8995f-prebuilt.qemu.org 04/01/2014 > Workqueue: writeback wb_workfn (flush-ubifs_0_0) > RIP: 0010:rb_next_postorder+0x2e/0xb0 > Code: 80 db 03 01 48 85 ff 0f 84 97 00 00 00 48 8b 17 48 83 05 bc 80 db > 03 01 48 83 e2 fc 0f 84 82 00 00 00 48 83 05 b2 80 db 03 01 <48> 3b 7a > 10 48 89 d0 74 02 f3 c3 48 8b 52 08 48 83 05 a3 80 db 03 > RSP: 0018:c9887758 EFLAGS: 00010202 > RAX: 888129ae4700 RBX: 888138b08400 RCX: 8081 > RDX: 0130 RSI: 80800024 RDI: 888138b08400 > RBP: 888138b08400 R08: ea0004a6b920 R09: > R10: c9887740 R11: 0001 R12: 888128d48000 > R13: 0800 R14: 011e R15: 07c8 > FS: () GS:88813ba0() > knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 0140 CR3: 00013789d000 CR4: 06f0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > Call Trace: > destroy_old_idx+0x5d/0xa0 [ubifs] > ubifs_tnc_start_commit+0x4fe/0x1380 [ubifs] > do_commit+0x3eb/0x830 [ubifs] > ubifs_run_commit+0xdc/0x1c0 [ubifs] > > Above Oops are due to the slab-out-of-bounds happened in do-while of > function layout_in_gaps indirectly called by ubifs_tnc_start_commit. In > function layout_in_gaps, there is a do-while loop placing index nodes > into the gaps created by obsolete index nodes in non-empty index LEBs > until rest index nodes can totally be placed into pre-allocated empty > LEBs. @c->gap_lebs points to a memory area(integer array) which records > LEB numbers used by 'in-the-gaps' method. Whenever a fitable index LEB > is found, corresponding lnum will be incrementally written into the > memory area pointed by @c->gap_lebs. The size > ((@c->lst.idx_lebs + 1) * sizeof(int)) of memory area is allocated before > do-while loop and can not be changed in the loop. But @c->lst.idx_lebs > could be increased by function ubifs_change_lp (called by > layout_leb_in_gaps->ubifs_find_dirty_idx_leb->get_idx_gc_leb) during the > loop. So, sometimes oob happens when number of cycles in do-while loop > exceeds the original value of @c->lst.idx_lebs. See detail in > https://bugzilla.kernel.org/show_bug.cgi?id=204229. > This patch fixes oob in layout_in_gaps. > > Signed-off-by: Zhihao Cheng > --- > fs/ubifs/tnc_commit.c | 34 +++--- > 1 file changed, 27 insertions(+), 7 deletions(-) > > diff --git a/fs/ubifs/tnc_commit.c b/fs/ubifs/tnc_commit.c > index a384a0f..234be1c 100644 > --- a/fs/ubifs/tnc_commit.c > +++ b/fs/ubifs/tnc_commit.c > @@ -212,7 +212,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union > ubifs_key *key, > /** > * layout_leb_in_gaps - layout index nodes using in-the-gaps method. > * @c: UBIFS file-system description object > - * @p: return LEB number here > + * @p: return LEB number in @c->gap_lebs[p] > * > * This function lays out new index nodes for dirty znodes using in-the-gaps > * method of TNC commit. > @@ -221,7 +221,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union > ubifs_key *key, > * This function returns the number of index nodes written into the gaps, or > a > * negative error code on failure. > */ > -static int layout_leb_in_gaps(struct ubifs_info *c, int *p) > +static int layout_leb_in_gaps(struct ubifs_info *c, int p) > { > struct ubifs_scan_leb *sleb; > struct ubifs_scan_node *snod; > @@ -236,7 +236,7 @@ static int layout_leb_in_gaps(struct ubifs_info *c, int > *p) > * filled, however we do not check there at present. > */ > return lnum; /* Error code */ > - *p = lnum; > + c->gap_lebs[p] = lnum; > dbg_gc("LEB %d", lnum); > /* > * Scan the index LEB. We use the generic scan for this even though > @@ -355,7 +355,7 @@ static int get_leb_cnt(struct ubifs_info *c, int cnt) > */ > static int layout_in_gaps(struct ubifs_info *c, int cnt) > { > - int err, leb_needed_cnt, written, *p; > + int err, leb_needed_cnt, written, p = 0, old_idx_lebs, *gap_lebs; > > dbg_gc("%d znodes to write", cnt); > > @@ -364,9 +364,9 @@ static int layout_in_gaps(struct ubifs_info *c, int cnt) > if (!c->gap_lebs) > return -ENOMEM; >
Re: 答复: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
- Ursprüngliche Mail - > Von: "chengzhihao1" > An: "richard" , "Sascha Hauer" , > "Artem Bityutskiy" , "yi > zhang" > CC: "linux-mtd" , "linux-kernel" > > Gesendet: Samstag, 27. Juli 2019 13:09:59 > Betreff: 答复: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps > ping I had no time to look at this yet. It is on my list. Thanks, //richard
答复: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
ping -邮件原件- 发件人: chengzhihao 发送时间: 2019年7月20日 14:05 收件人: rich...@nod.at; s.ha...@pengutronix.de; dedeki...@gmail.com; zhangyi (F) 抄送: linux-...@lists.infradead.org; linux-kernel@vger.kernel.org; chengzhihao 主题: [PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps Running stress-test test_2 in mtd-utils on ubi device, sometimes we can get following oops message: BUG: unable to handle page fault for address: 0140 #PF: supervisor read access in kernel mode #PF: error_code(0x) - not-present page PGD 280a067 P4D 280a067 PUD 0 Oops: [#1] SMP CPU: 0 PID: 60 Comm: kworker/u16:1 Kdump: loaded Not tainted 5.2.0 #13 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0 -0-ga698c8995f-prebuilt.qemu.org 04/01/2014 Workqueue: writeback wb_workfn (flush-ubifs_0_0) RIP: 0010:rb_next_postorder+0x2e/0xb0 Code: 80 db 03 01 48 85 ff 0f 84 97 00 00 00 48 8b 17 48 83 05 bc 80 db 03 01 48 83 e2 fc 0f 84 82 00 00 00 48 83 05 b2 80 db 03 01 <48> 3b 7a 10 48 89 d0 74 02 f3 c3 48 8b 52 08 48 83 05 a3 80 db 03 RSP: 0018:c9887758 EFLAGS: 00010202 RAX: 888129ae4700 RBX: 888138b08400 RCX: 8081 RDX: 0130 RSI: 80800024 RDI: 888138b08400 RBP: 888138b08400 R08: ea0004a6b920 R09: R10: c9887740 R11: 0001 R12: 888128d48000 R13: 0800 R14: 011e R15: 07c8 FS: () GS:88813ba0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 0140 CR3: 00013789d000 CR4: 06f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: destroy_old_idx+0x5d/0xa0 [ubifs] ubifs_tnc_start_commit+0x4fe/0x1380 [ubifs] do_commit+0x3eb/0x830 [ubifs] ubifs_run_commit+0xdc/0x1c0 [ubifs] Above Oops are due to the slab-out-of-bounds happened in do-while of function layout_in_gaps indirectly called by ubifs_tnc_start_commit. In function layout_in_gaps, there is a do-while loop placing index nodes into the gaps created by obsolete index nodes in non-empty index LEBs until rest index nodes can totally be placed into pre-allocated empty LEBs. @c->gap_lebs points to a memory area(integer array) which records LEB numbers used by 'in-the-gaps' method. Whenever a fitable index LEB is found, corresponding lnum will be incrementally written into the memory area pointed by @c->gap_lebs. The size ((@c->lst.idx_lebs + 1) * sizeof(int)) of memory area is allocated before do-while loop and can not be changed in the loop. But @c->lst.idx_lebs could be increased by function ubifs_change_lp (called by layout_leb_in_gaps->ubifs_find_dirty_idx_leb->get_idx_gc_leb) during the loop. So, sometimes oob happens when number of cycles in do-while loop exceeds the original value of @c->lst.idx_lebs. See detail in https://bugzilla.kernel.org/show_bug.cgi?id=204229. This patch fixes oob in layout_in_gaps. Signed-off-by: Zhihao Cheng --- fs/ubifs/tnc_commit.c | 34 +++--- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/fs/ubifs/tnc_commit.c b/fs/ubifs/tnc_commit.c index a384a0f..234be1c 100644 --- a/fs/ubifs/tnc_commit.c +++ b/fs/ubifs/tnc_commit.c @@ -212,7 +212,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union ubifs_key *key, /** * layout_leb_in_gaps - layout index nodes using in-the-gaps method. * @c: UBIFS file-system description object - * @p: return LEB number here + * @p: return LEB number in @c->gap_lebs[p] * * This function lays out new index nodes for dirty znodes using in-the-gaps * method of TNC commit. @@ -221,7 +221,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union ubifs_key *key, * This function returns the number of index nodes written into the gaps, or a * negative error code on failure. */ -static int layout_leb_in_gaps(struct ubifs_info *c, int *p) +static int layout_leb_in_gaps(struct ubifs_info *c, int p) { struct ubifs_scan_leb *sleb; struct ubifs_scan_node *snod; @@ -236,7 +236,7 @@ static int layout_leb_in_gaps(struct ubifs_info *c, int *p) * filled, however we do not check there at present. */ return lnum; /* Error code */ - *p = lnum; + c->gap_lebs[p] = lnum; dbg_gc("LEB %d", lnum); /* * Scan the index LEB. We use the generic scan for this even though @@ -355,7 +355,7 @@ static int get_leb_cnt(struct ubifs_info *c, int cnt) */ static int layout_in_gaps(struct ubifs_info *c, int cnt) { - int err, leb_needed_cnt, written, *p; + int err, leb_needed_cnt, written, p = 0, old_idx_lebs, *gap_lebs; dbg_gc("%d znodes to write", cnt); @@ -364,9 +364,9 @@ st
[PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
Running stress-test test_2 in mtd-utils on ubi device, sometimes we can get following oops message: BUG: unable to handle page fault for address: 0140 #PF: supervisor read access in kernel mode #PF: error_code(0x) - not-present page PGD 280a067 P4D 280a067 PUD 0 Oops: [#1] SMP CPU: 0 PID: 60 Comm: kworker/u16:1 Kdump: loaded Not tainted 5.2.0 #13 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0 -0-ga698c8995f-prebuilt.qemu.org 04/01/2014 Workqueue: writeback wb_workfn (flush-ubifs_0_0) RIP: 0010:rb_next_postorder+0x2e/0xb0 Code: 80 db 03 01 48 85 ff 0f 84 97 00 00 00 48 8b 17 48 83 05 bc 80 db 03 01 48 83 e2 fc 0f 84 82 00 00 00 48 83 05 b2 80 db 03 01 <48> 3b 7a 10 48 89 d0 74 02 f3 c3 48 8b 52 08 48 83 05 a3 80 db 03 RSP: 0018:c9887758 EFLAGS: 00010202 RAX: 888129ae4700 RBX: 888138b08400 RCX: 8081 RDX: 0130 RSI: 80800024 RDI: 888138b08400 RBP: 888138b08400 R08: ea0004a6b920 R09: R10: c9887740 R11: 0001 R12: 888128d48000 R13: 0800 R14: 011e R15: 07c8 FS: () GS:88813ba0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 0140 CR3: 00013789d000 CR4: 06f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: destroy_old_idx+0x5d/0xa0 [ubifs] ubifs_tnc_start_commit+0x4fe/0x1380 [ubifs] do_commit+0x3eb/0x830 [ubifs] ubifs_run_commit+0xdc/0x1c0 [ubifs] Above Oops are due to the slab-out-of-bounds happened in do-while of function layout_in_gaps indirectly called by ubifs_tnc_start_commit. In function layout_in_gaps, there is a do-while loop placing index nodes into the gaps created by obsolete index nodes in non-empty index LEBs until rest index nodes can totally be placed into pre-allocated empty LEBs. @c->gap_lebs points to a memory area(integer array) which records LEB numbers used by 'in-the-gaps' method. Whenever a fitable index LEB is found, corresponding lnum will be incrementally written into the memory area pointed by @c->gap_lebs. The size ((@c->lst.idx_lebs + 1) * sizeof(int)) of memory area is allocated before do-while loop and can not be changed in the loop. But @c->lst.idx_lebs could be increased by function ubifs_change_lp (called by layout_leb_in_gaps->ubifs_find_dirty_idx_leb->get_idx_gc_leb) during the loop. So, sometimes oob happens when number of cycles in do-while loop exceeds the original value of @c->lst.idx_lebs. See detail in https://bugzilla.kernel.org/show_bug.cgi?id=204229. This patch fixes oob in layout_in_gaps. Signed-off-by: Zhihao Cheng --- fs/ubifs/tnc_commit.c | 34 +++--- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/fs/ubifs/tnc_commit.c b/fs/ubifs/tnc_commit.c index a384a0f..234be1c 100644 --- a/fs/ubifs/tnc_commit.c +++ b/fs/ubifs/tnc_commit.c @@ -212,7 +212,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union ubifs_key *key, /** * layout_leb_in_gaps - layout index nodes using in-the-gaps method. * @c: UBIFS file-system description object - * @p: return LEB number here + * @p: return LEB number in @c->gap_lebs[p] * * This function lays out new index nodes for dirty znodes using in-the-gaps * method of TNC commit. @@ -221,7 +221,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union ubifs_key *key, * This function returns the number of index nodes written into the gaps, or a * negative error code on failure. */ -static int layout_leb_in_gaps(struct ubifs_info *c, int *p) +static int layout_leb_in_gaps(struct ubifs_info *c, int p) { struct ubifs_scan_leb *sleb; struct ubifs_scan_node *snod; @@ -236,7 +236,7 @@ static int layout_leb_in_gaps(struct ubifs_info *c, int *p) * filled, however we do not check there at present. */ return lnum; /* Error code */ - *p = lnum; + c->gap_lebs[p] = lnum; dbg_gc("LEB %d", lnum); /* * Scan the index LEB. We use the generic scan for this even though @@ -355,7 +355,7 @@ static int get_leb_cnt(struct ubifs_info *c, int cnt) */ static int layout_in_gaps(struct ubifs_info *c, int cnt) { - int err, leb_needed_cnt, written, *p; + int err, leb_needed_cnt, written, p = 0, old_idx_lebs, *gap_lebs; dbg_gc("%d znodes to write", cnt); @@ -364,9 +364,9 @@ static int layout_in_gaps(struct ubifs_info *c, int cnt) if (!c->gap_lebs) return -ENOMEM; - p = c->gap_lebs; + old_idx_lebs = c->lst.idx_lebs; do { - ubifs_assert(c, p < c->gap_lebs + c->lst.idx_lebs); + ubifs_assert(c, p < c->lst.idx_lebs); written = layout_leb_in_gaps(c, p);
[PATCH] ubifs: ubifs_tnc_start_commit: Fix OOB in layout_in_gaps
From: Zhihao Cheng Running stress-test test_2 in mtd-utils on ubi device, sometimes we can get follwing oops message: BUG: unable to handle page fault for address: 0140 #PF: supervisor read access in kernel mode #PF: error_code(0x) - not-present page PGD 280a067 P4D 280a067 PUD 0 Oops: [#1] SMP CPU: 0 PID: 60 Comm: kworker/u16:1 Kdump: loaded Not tainted 5.2.0 #13 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0 -0-ga698c8995f-prebuilt.qemu.org 04/01/2014 Workqueue: writeback wb_workfn (flush-ubifs_0_0) RIP: 0010:rb_next_postorder+0x2e/0xb0 Code: 80 db 03 01 48 85 ff 0f 84 97 00 00 00 48 8b 17 48 83 05 bc 80 db 03 01 48 83 e2 fc 0f 84 82 00 00 00 48 83 05 b2 80 db 03 01 <48> 3b 7a 10 48 89 d0 74 02 f3 c3 48 8b 52 08 48 83 05 a3 80 db 03 RSP: 0018:c9887758 EFLAGS: 00010202 RAX: 888129ae4700 RBX: 888138b08400 RCX: 8081 RDX: 0130 RSI: 80800024 RDI: 888138b08400 RBP: 888138b08400 R08: ea0004a6b920 R09: R10: c9887740 R11: 0001 R12: 888128d48000 R13: 0800 R14: 011e R15: 07c8 FS: () GS:88813ba0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 0140 CR3: 00013789d000 CR4: 06f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: destroy_old_idx+0x5d/0xa0 [ubifs] ubifs_tnc_start_commit+0x4fe/0x1380 [ubifs] do_commit+0x3eb/0x830 [ubifs] ubifs_run_commit+0xdc/0x1c0 [ubifs] Above Oops are due to the slab-out-of-bounds happened in do-while of function layout_in_gaps indirectly called by ubifs_tnc_start_commit. In function layout_in_gaps, there is a do-while loop placing index nodes into the gaps created by obsolete index nodes in non-empty index LEBs until rest index nodes can totally be placed into pre-allocated empty LEBs. @c->gap_lebs points to a memory area(integer array) which records LEB numbers used by 'in-the-gaps' method. Whenever a fitable index LEB is found, corresponding lnum will be incrementally written into the memory area pointed by @c->gap_lebs. The size ((@c->lst.idx_lebs + 1) * sizeof(int)) of memory area is allocated before do-while loop and can not be changed in the loop. But @c->lst.idx_lebs could be increased by function ubifs_change_lp (called by layout_leb_in_gaps->ubifs_find_dirty_idx_leb->get_idx_gc_leb) during the loop. So, sometimes oob happens when number of cycles in do-while loop exceeds the original value of @c->lst.idx_lebs. See detail in https://bugzilla.kernel.org/show_bug.cgi?id=204229. This patch fixes oob in layout_in_gaps. Signed-off-by: Zhihao Cheng --- fs/ubifs/tnc_commit.c | 30 +++--- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/fs/ubifs/tnc_commit.c b/fs/ubifs/tnc_commit.c index a384a0f..9fc6b8e 100644 --- a/fs/ubifs/tnc_commit.c +++ b/fs/ubifs/tnc_commit.c @@ -212,7 +212,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union ubifs_key *key, /** * layout_leb_in_gaps - layout index nodes using in-the-gaps method. * @c: UBIFS file-system description object - * @p: return LEB number here + * @p: return LEB number in @c->gap_lebs[p] * * This function lays out new index nodes for dirty znodes using in-the-gaps * method of TNC commit. @@ -221,7 +221,7 @@ static int is_idx_node_in_use(struct ubifs_info *c, union ubifs_key *key, * This function returns the number of index nodes written into the gaps, or a * negative error code on failure. */ -static int layout_leb_in_gaps(struct ubifs_info *c, int *p) +static int layout_leb_in_gaps(struct ubifs_info *c, int p) { struct ubifs_scan_leb *sleb; struct ubifs_scan_node *snod; @@ -236,7 +236,7 @@ static int layout_leb_in_gaps(struct ubifs_info *c, int *p) * filled, however we do not check there at present. */ return lnum; /* Error code */ - *p = lnum; + c->gap_lebs[p] = lnum; dbg_gc("LEB %d", lnum); /* * Scan the index LEB. We use the generic scan for this even though @@ -355,7 +355,8 @@ static int get_leb_cnt(struct ubifs_info *c, int cnt) */ static int layout_in_gaps(struct ubifs_info *c, int cnt) { - int err, leb_needed_cnt, written, *p; + int err, leb_needed_cnt, written, p = 0, old_idx_lebs; + old_idx_lebs = c->lst.idx_lebs; dbg_gc("%d znodes to write", cnt); @@ -364,9 +365,8 @@ static int layout_in_gaps(struct ubifs_info *c, int cnt) if (!c->gap_lebs) return -ENOMEM; - p = c->gap_lebs; do { - ubifs_assert(c, p < c->gap_lebs + c->lst.idx_lebs); + ubifs_assert(c, p < c->lst.idx_lebs); written = layout_leb_in_gaps(c, p);