* [PATCH] xfs: validate quota log items during log recovery
@ 2009-03-03 17:54 Christoph Hellwig
2009-03-16 7:54 ` Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Christoph Hellwig @ 2009-03-03 17:54 UTC (permalink / raw)
To: xfs
Arkadiusz has been seeing really strange crashes in xfs_qm_dqcheck that
I can only explain by a log item beeing too smal to actually fit the
xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add
graceful checks for NULL or too small quota items to the log recovery
code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_log_recover.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log_recover.c 2009-03-02 04:15:11.410430892 +0100
+++ xfs/fs/xfs/xfs_log_recover.c 2009-03-02 04:16:29.649444226 +0100
@@ -1975,16 +1975,26 @@ xlog_recover_do_reg_buffer(
error = 0;
if (buf_f->blf_flags &
(XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) {
+ if (item->ri_buf[i].i_addr == NULL ||
+ item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) {
+ cmn_err(CE_ALERT,
+ "XFS: dquot too small (%d) in xlog_recover_do_reg_buffer.",
+ item->ri_buf[i].i_len);
+ goto next;
+ }
error = xfs_qm_dqcheck((xfs_disk_dquot_t *)
item->ri_buf[i].i_addr,
-1, 0, XFS_QMOPT_DOWARN,
"dquot_buf_recover");
+ if (error)
+ goto next;
}
- if (!error)
- memcpy(xfs_buf_offset(bp,
- (uint)bit << XFS_BLI_SHIFT), /* dest */
- item->ri_buf[i].i_addr, /* source */
- nbits<<XFS_BLI_SHIFT); /* length */
+
+ memcpy(xfs_buf_offset(bp,
+ (uint)bit << XFS_BLI_SHIFT), /* dest */
+ item->ri_buf[i].i_addr, /* source */
+ nbits<<XFS_BLI_SHIFT); /* length */
+ next:
i++;
bit += nbits;
}
@@ -2615,7 +2625,15 @@ xlog_recover_do_dquot_trans(
return (0);
recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr;
- ASSERT(recddq);
+
+ if (item->ri_buf[1].i_addr == NULL ||
+ item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) {
+ cmn_err(CE_ALERT,
+ "XFS: dquot too small (%d) in xlog_recover_do_dquot_trans.",
+ item->ri_buf[1].i_len);
+ return XFS_ERROR(EIO);
+ }
+
/*
* This type of quotas was turned off, so ignore this record.
*/
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] xfs: validate quota log items during log recovery 2009-03-03 17:54 [PATCH] xfs: validate quota log items during log recovery Christoph Hellwig @ 2009-03-16 7:54 ` Christoph Hellwig 2009-03-29 7:42 ` Christoph Hellwig 2009-04-04 11:59 ` Arkadiusz Miskiewicz 2009-05-26 16:07 ` Eric Sandeen 2 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2009-03-16 7:54 UTC (permalink / raw) To: xfs ping? On Tue, Mar 03, 2009 at 12:54:27PM -0500, Christoph Hellwig wrote: > Arkadiusz has been seeing really strange crashes in xfs_qm_dqcheck that > I can only explain by a log item beeing too smal to actually fit the > xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add > graceful checks for NULL or too small quota items to the log recovery > code. > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_log_recover.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_log_recover.c 2009-03-02 04:15:11.410430892 +0100 > +++ xfs/fs/xfs/xfs_log_recover.c 2009-03-02 04:16:29.649444226 +0100 > @@ -1975,16 +1975,26 @@ xlog_recover_do_reg_buffer( > error = 0; > if (buf_f->blf_flags & > (XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) { > + if (item->ri_buf[i].i_addr == NULL || > + item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) { > + cmn_err(CE_ALERT, > + "XFS: dquot too small (%d) in xlog_recover_do_reg_buffer.", > + item->ri_buf[i].i_len); > + goto next; > + } > error = xfs_qm_dqcheck((xfs_disk_dquot_t *) > item->ri_buf[i].i_addr, > -1, 0, XFS_QMOPT_DOWARN, > "dquot_buf_recover"); > + if (error) > + goto next; > } > - if (!error) > - memcpy(xfs_buf_offset(bp, > - (uint)bit << XFS_BLI_SHIFT), /* dest */ > - item->ri_buf[i].i_addr, /* source */ > - nbits<<XFS_BLI_SHIFT); /* length */ > + > + memcpy(xfs_buf_offset(bp, > + (uint)bit << XFS_BLI_SHIFT), /* dest */ > + item->ri_buf[i].i_addr, /* source */ > + nbits<<XFS_BLI_SHIFT); /* length */ > + next: > i++; > bit += nbits; > } > @@ -2615,7 +2625,15 @@ xlog_recover_do_dquot_trans( > return (0); > > recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr; > - ASSERT(recddq); > + > + if (item->ri_buf[1].i_addr == NULL || > + item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) { > + cmn_err(CE_ALERT, > + "XFS: dquot too small (%d) in xlog_recover_do_dquot_trans.", > + item->ri_buf[1].i_len); > + return XFS_ERROR(EIO); > + } > + > /* > * This type of quotas was turned off, so ignore this record. > */ > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs ---end quoted text--- _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: validate quota log items during log recovery 2009-03-16 7:54 ` Christoph Hellwig @ 2009-03-29 7:42 ` Christoph Hellwig 2009-05-26 9:11 ` Christoph Hellwig 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2009-03-29 7:42 UTC (permalink / raw) To: xfs ping^2? On Mon, Mar 16, 2009 at 03:54:07AM -0400, Christoph Hellwig wrote: > ping? > > On Tue, Mar 03, 2009 at 12:54:27PM -0500, Christoph Hellwig wrote: > > Arkadiusz has been seeing really strange crashes in xfs_qm_dqcheck that > > I can only explain by a log item beeing too smal to actually fit the > > xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add > > graceful checks for NULL or too small quota items to the log recovery > > code. > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > Index: xfs/fs/xfs/xfs_log_recover.c > > =================================================================== > > --- xfs.orig/fs/xfs/xfs_log_recover.c 2009-03-02 04:15:11.410430892 +0100 > > +++ xfs/fs/xfs/xfs_log_recover.c 2009-03-02 04:16:29.649444226 +0100 > > @@ -1975,16 +1975,26 @@ xlog_recover_do_reg_buffer( > > error = 0; > > if (buf_f->blf_flags & > > (XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) { > > + if (item->ri_buf[i].i_addr == NULL || > > + item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) { > > + cmn_err(CE_ALERT, > > + "XFS: dquot too small (%d) in xlog_recover_do_reg_buffer.", > > + item->ri_buf[i].i_len); > > + goto next; > > + } > > error = xfs_qm_dqcheck((xfs_disk_dquot_t *) > > item->ri_buf[i].i_addr, > > -1, 0, XFS_QMOPT_DOWARN, > > "dquot_buf_recover"); > > + if (error) > > + goto next; > > } > > - if (!error) > > - memcpy(xfs_buf_offset(bp, > > - (uint)bit << XFS_BLI_SHIFT), /* dest */ > > - item->ri_buf[i].i_addr, /* source */ > > - nbits<<XFS_BLI_SHIFT); /* length */ > > + > > + memcpy(xfs_buf_offset(bp, > > + (uint)bit << XFS_BLI_SHIFT), /* dest */ > > + item->ri_buf[i].i_addr, /* source */ > > + nbits<<XFS_BLI_SHIFT); /* length */ > > + next: > > i++; > > bit += nbits; > > } > > @@ -2615,7 +2625,15 @@ xlog_recover_do_dquot_trans( > > return (0); > > > > recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr; > > - ASSERT(recddq); > > + > > + if (item->ri_buf[1].i_addr == NULL || > > + item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) { > > + cmn_err(CE_ALERT, > > + "XFS: dquot too small (%d) in xlog_recover_do_dquot_trans.", > > + item->ri_buf[1].i_len); > > + return XFS_ERROR(EIO); > > + } > > + > > /* > > * This type of quotas was turned off, so ignore this record. > > */ > > > > _______________________________________________ > > xfs mailing list > > xfs@oss.sgi.com > > http://oss.sgi.com/mailman/listinfo/xfs > ---end quoted text--- > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs ---end quoted text--- _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: validate quota log items during log recovery 2009-03-29 7:42 ` Christoph Hellwig @ 2009-05-26 9:11 ` Christoph Hellwig 0 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2009-05-26 9:11 UTC (permalink / raw) To: xfs ping^3? On Sun, Mar 29, 2009 at 03:42:46AM -0400, Christoph Hellwig wrote: > ping^2? > > On Mon, Mar 16, 2009 at 03:54:07AM -0400, Christoph Hellwig wrote: > > ping? > > > > On Tue, Mar 03, 2009 at 12:54:27PM -0500, Christoph Hellwig wrote: > > > Arkadiusz has been seeing really strange crashes in xfs_qm_dqcheck that > > > I can only explain by a log item beeing too smal to actually fit the > > > xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add > > > graceful checks for NULL or too small quota items to the log recovery > > > code. > > > > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > > > Index: xfs/fs/xfs/xfs_log_recover.c > > > =================================================================== > > > --- xfs.orig/fs/xfs/xfs_log_recover.c 2009-03-02 04:15:11.410430892 +0100 > > > +++ xfs/fs/xfs/xfs_log_recover.c 2009-03-02 04:16:29.649444226 +0100 > > > @@ -1975,16 +1975,26 @@ xlog_recover_do_reg_buffer( > > > error = 0; > > > if (buf_f->blf_flags & > > > (XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) { > > > + if (item->ri_buf[i].i_addr == NULL || > > > + item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) { > > > + cmn_err(CE_ALERT, > > > + "XFS: dquot too small (%d) in xlog_recover_do_reg_buffer.", > > > + item->ri_buf[i].i_len); > > > + goto next; > > > + } > > > error = xfs_qm_dqcheck((xfs_disk_dquot_t *) > > > item->ri_buf[i].i_addr, > > > -1, 0, XFS_QMOPT_DOWARN, > > > "dquot_buf_recover"); > > > + if (error) > > > + goto next; > > > } > > > - if (!error) > > > - memcpy(xfs_buf_offset(bp, > > > - (uint)bit << XFS_BLI_SHIFT), /* dest */ > > > - item->ri_buf[i].i_addr, /* source */ > > > - nbits<<XFS_BLI_SHIFT); /* length */ > > > + > > > + memcpy(xfs_buf_offset(bp, > > > + (uint)bit << XFS_BLI_SHIFT), /* dest */ > > > + item->ri_buf[i].i_addr, /* source */ > > > + nbits<<XFS_BLI_SHIFT); /* length */ > > > + next: > > > i++; > > > bit += nbits; > > > } > > > @@ -2615,7 +2625,15 @@ xlog_recover_do_dquot_trans( > > > return (0); > > > > > > recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr; > > > - ASSERT(recddq); > > > + > > > + if (item->ri_buf[1].i_addr == NULL || > > > + item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) { > > > + cmn_err(CE_ALERT, > > > + "XFS: dquot too small (%d) in xlog_recover_do_dquot_trans.", > > > + item->ri_buf[1].i_len); > > > + return XFS_ERROR(EIO); > > > + } > > > + > > > /* > > > * This type of quotas was turned off, so ignore this record. > > > */ > > > > > > _______________________________________________ > > > xfs mailing list > > > xfs@oss.sgi.com > > > http://oss.sgi.com/mailman/listinfo/xfs > > ---end quoted text--- > > > > _______________________________________________ > > xfs mailing list > > xfs@oss.sgi.com > > http://oss.sgi.com/mailman/listinfo/xfs > ---end quoted text--- > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs ---end quoted text--- _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: validate quota log items during log recovery 2009-03-03 17:54 [PATCH] xfs: validate quota log items during log recovery Christoph Hellwig 2009-03-16 7:54 ` Christoph Hellwig @ 2009-04-04 11:59 ` Arkadiusz Miskiewicz 2009-05-26 16:07 ` Eric Sandeen 2 siblings, 0 replies; 8+ messages in thread From: Arkadiusz Miskiewicz @ 2009-04-04 11:59 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Tuesday 03 of March 2009, Christoph Hellwig wrote: > Arkadiusz has been seeing really strange crashes in xfs_qm_dqcheck that > I can only explain by a log item beeing too smal to actually fit the > xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add > graceful checks for NULL or too small quota items to the log recovery > code. Unfortunately this validation doesn't cover my case. I still got oops even with the patch applied 1). I also tried xfs debug enabled kernel 2) 1) oops with "[PATCH] xfs: validate quota log items during log recovery" [ 37.379658] Filesystem "dm-0": Disabling barriers, trial barrier write failed [ 37.445959] XFS mounting filesystem dm-0 [ 39.478398] Starting XFS recovery on filesystem: dm-0 (logdev: internal) [ 42.833274] BUG: unable to handle kernel paging request at fffffffffffffc00 [ 42.835651] IP: [<ffffffffa0101e5a>] xfs_qm_dqcheck+0x9ca/0x2270 [xfs] [ 42.835651] PGD 549067 PUD 54b067 PMD 0 [ 42.876885] Oops: 0002 [#1] SMP [ 42.876885] last sysfs file: /sys/devices/virtual/block/md3/dev [ 42.906879] CPU 2 [ 42.906879] Modules linked in: ext3 jbd mbcache raid456 async_xor async_memcpy async_tx xor raid1 dm_mod e1000 e1000e ipmi_devintf ipmi_si ipmi_msghandler 8021q garp stp xfs scsi_wait_scan sd_mod crc_t10dif mptsas mptscsih mptbase scsi_transport_sas scsi_mod raid10 md_mod [ 42.976880] Pid: 1718, comm: mount Not tainted 2.6.28.9-2 #1 [ 42.976880] RIP: 0010:[<ffffffffa0101e5a>] [<ffffffffa0101e5a>] xfs_qm_dqcheck+0x9ca/0x2270 [xfs] [ 42.976880] RSP: 0018:ffff88015b53fa58 EFLAGS: 00010256 [ 42.976880] RAX: 0000000000000000 RBX: ffff88015bcaee40 RCX: 0000000000000000 [ 42.976880] RDX: fffffffffffffc00 RSI: 0000000000000000 RDI: fffffffffffffc00 [ 42.976880] RBP: 0000000000000000 R08: 0000000000000000 R09: fffffffffffffc00 [ 42.976880] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88015bcaebc0 [ 42.976880] R13: ffff88015bcaeb40 R14: 0000000000001000 R15: ffff88015e55d000 [ 42.976880] FS: 00007f3de87f27d0(0000) GS:ffff88015fa4c180(0000) knlGS:0000000000000000 [ 43.176886] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b [ 43.176886] CR2: fffffffffffffc00 CR3: 000000015b501000 CR4: 00000000000006e0 [ 43.176886] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 43.176886] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 43.176886] Process mount (pid: 1718, threadinfo ffff88015b53e000, task ffff88015ccd9470) [ 43.276880] Stack: [ 43.276880] ffff88015bcaeb40 000000005d34a0c8 ffff88015b53fb98 ffffc200115e839c [ 43.313546] ffffc200115e83a8 0000000000000003 ffffc200115e9e00 ffffffffa010351c [ 43.313546] 0000000000039c4a 000000015b5ab000 ffff88015b5ab000 ffff88015b53fb88 [ 43.340212] Call Trace: [ 43.340212] [<ffffffffa010351c>] xfs_qm_dqcheck+0x208c/0x2270 [xfs] [ 43.340212] [<ffffffffa0103ed9>] xlog_get_bp+0x1e9/0x1750 [xfs] [ 43.340212] [<ffffffffa01256d0>] xfs_dir_file_operations+0x2290/0xe906 [xfs] [ 43.340212] [<ffffffffa01044e7>] xlog_get_bp+0x7f7/0x1750 [xfs] [ 43.340212] [<ffffffffa010454a>] xlog_get_bp+0x85a/0x1750 [xfs] [ 43.340212] [<ffffffffa01058ea>] xlog_recover+0x7a/0x90 [xfs] [ 43.340212] [<ffffffffa00ff706>] xfs_log_mount+0xa6/0xd30 [xfs] [ 43.340212] [<ffffffffa010816b>] xfs_mountfs+0x33b/0x680 [xfs] [ 43.340212] [<ffffffffa00ebb00>] xfs_filestream_lookup_ag+0x60/0x4f0 [xfs] [ 43.340212] [<ffffffffa0114e3b>] kmem_zalloc+0x2b/0x40 [xfs] [ 43.340212] [<ffffffffa01091af>] xfs_mru_cache_create+0x12f/0x160 [xfs] [ 43.340212] [<ffffffffa01216c2>] xfs_blkdev_get+0x492/0x660 [xfs] [ 43.340212] [<ffffffff802d7c54>] get_sb_bdev+0x174/0x1a0 [ 43.340212] [<ffffffffa0121460>] xfs_blkdev_get+0x230/0x660 [xfs] [ 43.340212] [<ffffffff802a6854>] kstrdup+0x54/0x70 [ 43.340212] [<ffffffff802d75e6>] vfs_kern_mount+0x86/0x250 [ 43.340212] [<ffffffff802d7813>] do_kern_mount+0x53/0x120 [ 43.340212] [<ffffffff802f14bd>] do_mount+0x2ed/0xa50 [ 43.340212] [<ffffffff802f1d19>] sys_mount+0xf9/0x110 [ 43.340212] [<ffffffff802031bb>] system_call_fastpath+0x16/0x1b [ 43.340212] Code: 49 8b 54 24 08 8b 42 18 85 c0 75 65 49 8b 45 28 48 8b 58 08 8b 6b 18 85 ed 0f 84 92 00 00 00 48 63 43 14 48 8b 53 20 48 c1 e0 04 <4c> 89 3c 02 48 63 43 14 48 8b 53 20 48 c1 e0 04 44 89 74 02 08 [ 43.340212] RIP [<ffffffffa0101e5a>] xfs_qm_dqcheck+0x9ca/0x2270 [xfs] [ 43.340212] RSP <ffff88015b53fa58> [ 43.340212] CR2: fffffffffffffc00 [ 43.340212] ---[ end trace 3fb966a92b4fc211 ]--- [ 60.803310] 0000:05:00.0: eth0: changing MTU from 1500 to 9000 2) ops without "[PATCH] xfs: validate quota log items during log recovery" but with xfs debug enabled kernel [ 37.768605] Filesystem "dm-0": Disabling barriers, trial barrier write failed [ 37.831256] XFS mounting filesystem dm-0 [ 39.575446] Starting XFS recovery on filesystem: dm-0 (logdev: internal) [ 42.482802] Assertion failed: item->ri_total > item->ri_cnt, file: fs/xfs/xfs_log_recover.c, line: 1452 [ 42.482841] ------------[ cut here ]------------ [ 42.486059] kernel BUG at fs/xfs/support/debug.c:81! [ 42.486059] invalid opcode: 0000 [#1] SMP [ 42.486059] last sysfs file: /sys/devices/virtual/block/md3/dev [ 42.486059] CPU 3 [ 42.486059] Modules linked in: ext3 jbd mbcache raid456 async_xor async_memcpy async_tx xor raid1 dm_mod e1000 e1000e ipmi_devintf ipmi_si ipmi_msghandler 8021q garp stp xfs scsi_wait_scan sd_mod crc_t10dif mptsas mptscsih mptbase scsi_transport_sas scsi_mod raid10 md_mod [ 42.486059] Pid: 1718, comm: mount Not tainted 2.6.28.7-1 #1 [ 42.486059] RIP: 0010:[<ffffffffa013ddca>] [<ffffffffa013ddca>] assfail+0x1a/0x20 [xfs] [ 42.486059] RSP: 0018:ffff88015bc7ba48 EFLAGS: 00010292 [ 42.486059] RAX: 000000000000006e RBX: ffff88015bc723c0 RCX: 0000000000000004 [ 42.486059] RDX: 0000000000000d0d RSI: 0000000000000046 RDI: ffffffff8084d310 [ 42.486059] RBP: ffffc200115e63a8 R08: 0000000000000000 R09: 0000000000000006 [ 42.486059] R10: ffff88015bc7b7e8 R11: 000000005bc7b8e8 R12: ffff88015bc72180 [ 42.486059] R13: ffff88015bc722c0 R14: 0000000000001000 R15: ffff88015e0fb000 [ 42.486059] FS: 00007ff1ac5557d0(0000) GS:ffff88015fa4c500(0000) knlGS:0000000000000000 [ 42.486059] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b [ 42.486059] CR2: 00000000006e3000 CR3: 000000015bc6d000 CR4: 00000000000006e0 [ 42.486059] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 42.486059] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 42.486059] Process mount (pid: 1718, threadinfo ffff88015bc7a000, task ffff88015d0ba210) [ 42.486059] Stack: [ 42.486059] ffffc200115e63a8 ffffffffa0119883 ffff88015bc72180 ffffc200115e639c [ 42.486059] ffffc200115e63a8 ffffc200115e7e00 0000000000000003 ffff88015bc7bb78 [ 42.486059] ffff88015b4b4000 ffffffffa011b47e ffff88015d455680 00000001a0132b9d [ 42.486059] Call Trace: [ 42.486059] [<ffffffffa0119883>] ? xlog_recover_add_to_trans+0xa3/0x1f0 [xfs] [ 42.486059] [<ffffffffa011b47e>] ? xlog_recover_process_data+0x23e/0x290 [xfs] [ 42.486059] [<ffffffffa011ba39>] ? xlog_do_recovery_pass+0x389/0x810 [xfs] [ 42.486059] [<ffffffff802308f0>] ? default_wake_function+0x0/0x10 [ 42.486059] [<ffffffffa014afa0>] ? xfs_dir_file_operations+0xbd20/0x24571 [xfs] [ 42.486059] [<ffffffffa011bf16>] ? xlog_do_log_recovery+0x56/0x100 [xfs] [ 42.486059] [<ffffffffa011bfe0>] ? xlog_do_recover+0x20/0x250 [xfs] [ 42.486059] [<ffffffffa011d6ca>] ? xlog_recover+0x7a/0x90 [xfs] [ 42.486059] [<ffffffffa01174ca>] ? xfs_log_mount+0xaa/0x1b0 [xfs] [ 42.486059] [<ffffffffa012085b>] ? xfs_mountfs+0x32b/0x6b0 [xfs] [ 42.486059] [<ffffffffa00fcf70>] ? xfs_fstrm_free_func+0x0/0xc0 [xfs] [ 42.486059] [<ffffffffa012fd7b>] ? kmem_zalloc+0x2b/0x40 [xfs] [ 42.486059] [<ffffffffa01211ef>] ? xfs_mru_cache_create+0x12f/0x160 [xfs] [ 42.486059] [<ffffffffa013d472>] ? xfs_fs_fill_super+0x262/0x430 [xfs] [ 42.486059] [<ffffffff802d7e64>] ? get_sb_bdev+0x174/0x1a0 [ 42.486059] [<ffffffffa013d210>] ? xfs_fs_fill_super+0x0/0x430 [xfs] [ 42.486059] [<ffffffff802a6964>] ? kstrdup+0x54/0x70 [ 42.486059] [<ffffffff802d77f6>] ? vfs_kern_mount+0x86/0x250 [ 42.486059] [<ffffffff802d7a23>] ? do_kern_mount+0x53/0x120 [ 42.486059] [<ffffffff802f150d>] ? do_mount+0x2ed/0xa50 [ 42.486059] [<ffffffff802f1d69>] ? sys_mount+0xf9/0x110 [ 42.486059] [<ffffffff802031bb>] ? system_call_fastpath+0x16/0x1b [ 42.486059] Code: 08 01 00 00 00 e8 77 df 26 e0 48 83 c4 18 c3 66 90 89 d1 48 83 ec 08 48 89 f2 31 c0 48 89 fe 48 c7 c7 e0 d2 14 a0 e8 57 4b 3f e0 <0f> 0b eb fe 66 90 41 55 41 54 49 89 f4 55 89 fd 48 c7 c7 80 19 [ 42.486059] RIP [<ffffffffa013ddca>] assfail+0x1a/0x20 [xfs] [ 42.486059] RSP <ffff88015bc7ba48> [ 43.497724] ---[ end trace 07b3fe479be2dbfb ]--- [ 59.200404] 0000:05:00.0: eth0: changing MTU from 1500 to 9000 -- Arkadiusz Miśkiewicz PLD/Linux Team arekm / maven.pl http://ftp.pld-linux.org/ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: validate quota log items during log recovery 2009-03-03 17:54 [PATCH] xfs: validate quota log items during log recovery Christoph Hellwig 2009-03-16 7:54 ` Christoph Hellwig 2009-04-04 11:59 ` Arkadiusz Miskiewicz @ 2009-05-26 16:07 ` Eric Sandeen 2009-05-27 9:17 ` Christoph Hellwig 2 siblings, 1 reply; 8+ messages in thread From: Eric Sandeen @ 2009-05-26 16:07 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Christoph Hellwig wrote: > Arkadiusz has been seeing really strange crashes in xfs_qm_dqcheck that > I can only explain by a log item beeing too smal to actually fit the ^^being too small^^ > xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add > graceful checks for NULL or too small quota items to the log recovery > code. > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_log_recover.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_log_recover.c 2009-03-02 04:15:11.410430892 +0100 > +++ xfs/fs/xfs/xfs_log_recover.c 2009-03-02 04:16:29.649444226 +0100 > @@ -1975,16 +1975,26 @@ xlog_recover_do_reg_buffer( > error = 0; > if (buf_f->blf_flags & > (XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) { > + if (item->ri_buf[i].i_addr == NULL || > + item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) { > + cmn_err(CE_ALERT, > + "XFS: dquot too small (%d) in xlog_recover_do_reg_buffer.", > + item->ri_buf[i].i_len); Shouldn't this differentiate between i_addr == NULL and i_len too small, though? While we're at it anyway... Maybe: + "XFS: dquot null addr (%p) or len too small (%d) in %s." + item->ri_buf[i].i_addr, item->ri_buf[i].i_len, __func__); ? (not hardcoding function name may be good too) > + goto next; > + } > error = xfs_qm_dqcheck((xfs_disk_dquot_t *) > item->ri_buf[i].i_addr, > -1, 0, XFS_QMOPT_DOWARN, > "dquot_buf_recover"); > + if (error) > + goto next; I guess we can't do much else, but what happens in the end, when we skip a buffer... > } > - if (!error) > - memcpy(xfs_buf_offset(bp, > - (uint)bit << XFS_BLI_SHIFT), /* dest */ > - item->ri_buf[i].i_addr, /* source */ > - nbits<<XFS_BLI_SHIFT); /* length */ > + > + memcpy(xfs_buf_offset(bp, > + (uint)bit << XFS_BLI_SHIFT), /* dest */ > + item->ri_buf[i].i_addr, /* source */ > + nbits<<XFS_BLI_SHIFT); /* length */ > + next: > i++; > bit += nbits; > } > @@ -2615,7 +2625,15 @@ xlog_recover_do_dquot_trans( > return (0); > > recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr; > - ASSERT(recddq); > + > + if (item->ri_buf[1].i_addr == NULL || > + item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) { > + cmn_err(CE_ALERT, > + "XFS: dquot too small (%d) in xlog_recover_do_dquot_trans.", > + item->ri_buf[1].i_len); Same deal here, should you differentiate on the error & use __func__ ? -Eric > + return XFS_ERROR(EIO); > + } > + > /* > * This type of quotas was turned off, so ignore this record. > */ > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: validate quota log items during log recovery 2009-05-26 16:07 ` Eric Sandeen @ 2009-05-27 9:17 ` Christoph Hellwig 2009-05-27 19:06 ` Eric Sandeen 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2009-05-27 9:17 UTC (permalink / raw) To: Eric Sandeen; +Cc: Christoph Hellwig, xfs On Tue, May 26, 2009 at 11:07:56AM -0500, Eric Sandeen wrote: > Christoph Hellwig wrote: > > Arkadiusz has been seeing really strange crashes in xfs_qm_dqcheck that > > I can only explain by a log item beeing too smal to actually fit the > ^^being too small^^ Thanks, corrected. > > + if (item->ri_buf[i].i_addr == NULL || > > + item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) { > > + cmn_err(CE_ALERT, > > + "XFS: dquot too small (%d) in xlog_recover_do_reg_buffer.", > > + item->ri_buf[i].i_len); > > Shouldn't this differentiate between i_addr == NULL and i_len too small, > though? While we're at it anyway... > > Maybe: I've split both into individual checks and used __func__ to print the function instead of hardconding it. > > error = xfs_qm_dqcheck((xfs_disk_dquot_t *) > > item->ri_buf[i].i_addr, > > -1, 0, XFS_QMOPT_DOWARN, > > "dquot_buf_recover"); > > + if (error) > > + goto next; > > I guess we can't do much else, but what happens in the end, when we skip > a buffer... Yeah, same action a a xfs_qm_dqcheck failure. Error handling here probably wants to be revisited, but that should be a separate patch. Updated patch below: Subject: xfs: validate quota log items during log recovery From: Christoph Hellwig <hch@lst.de> Arkadiusz has seen really strange crashes in xfs_qm_dqcheck that I can only explain by a log item being too smal to actually fit the xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add graceful checks for NULL or too small quota items to the log recovery code. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_log_recover.c =================================================================== --- xfs.orig/fs/xfs/xfs_log_recover.c 2009-05-27 10:40:03.752821404 +0200 +++ xfs/fs/xfs/xfs_log_recover.c 2009-05-27 10:43:23.740939498 +0200 @@ -1975,16 +1975,30 @@ xlog_recover_do_reg_buffer( error = 0; if (buf_f->blf_flags & (XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) { + if (item->ri_buf[i].i_addr == NULL) { + cmn_err(CE_ALERT, + "XFS: NULL dquot in %s.", __func__); + goto next; + } + if (item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) { + cmn_err(CE_ALERT, + "XFS: dquot too small (%d) in %s.", + item->ri_buf[i].i_len, __func__); + goto next; + } error = xfs_qm_dqcheck((xfs_disk_dquot_t *) item->ri_buf[i].i_addr, -1, 0, XFS_QMOPT_DOWARN, "dquot_buf_recover"); + if (error) + goto next; } - if (!error) - memcpy(xfs_buf_offset(bp, - (uint)bit << XFS_BLI_SHIFT), /* dest */ - item->ri_buf[i].i_addr, /* source */ - nbits<<XFS_BLI_SHIFT); /* length */ + + memcpy(xfs_buf_offset(bp, + (uint)bit << XFS_BLI_SHIFT), /* dest */ + item->ri_buf[i].i_addr, /* source */ + nbits<<XFS_BLI_SHIFT); /* length */ + next: i++; bit += nbits; } @@ -2615,7 +2629,19 @@ xlog_recover_do_dquot_trans( return (0); recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr; - ASSERT(recddq); + + if (item->ri_buf[1].i_addr == NULL) { + cmn_err(CE_ALERT, + "XFS: NULL dquot in %s.", __func__); + return XFS_ERROR(EIO); + } + if (item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) { + cmn_err(CE_ALERT, + "XFS: dquot too small (%d) in %s.", + item->ri_buf[1].i_len, __func__); + return XFS_ERROR(EIO); + } + /* * This type of quotas was turned off, so ignore this record. */ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: validate quota log items during log recovery 2009-05-27 9:17 ` Christoph Hellwig @ 2009-05-27 19:06 ` Eric Sandeen 0 siblings, 0 replies; 8+ messages in thread From: Eric Sandeen @ 2009-05-27 19:06 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Christoph Hellwig wrote: > Updated patch below: > > > Subject: xfs: validate quota log items during log recovery > From: Christoph Hellwig <hch@lst.de> > > Arkadiusz has seen really strange crashes in xfs_qm_dqcheck that > I can only explain by a log item being too smal to actually fit the ^^small ;) > xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add > graceful checks for NULL or too small quota items to the log recovery > code. > > > Signed-off-by: Christoph Hellwig <hch@lst.de> A bit more verbose now isn't it, but oh well :) Reviewed-by: Eric Sandeen <sandeen@sandeen.net> > Index: xfs/fs/xfs/xfs_log_recover.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_log_recover.c 2009-05-27 10:40:03.752821404 +0200 > +++ xfs/fs/xfs/xfs_log_recover.c 2009-05-27 10:43:23.740939498 +0200 > @@ -1975,16 +1975,30 @@ xlog_recover_do_reg_buffer( > error = 0; > if (buf_f->blf_flags & > (XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) { > + if (item->ri_buf[i].i_addr == NULL) { > + cmn_err(CE_ALERT, > + "XFS: NULL dquot in %s.", __func__); > + goto next; > + } > + if (item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) { > + cmn_err(CE_ALERT, > + "XFS: dquot too small (%d) in %s.", > + item->ri_buf[i].i_len, __func__); > + goto next; > + } > error = xfs_qm_dqcheck((xfs_disk_dquot_t *) > item->ri_buf[i].i_addr, > -1, 0, XFS_QMOPT_DOWARN, > "dquot_buf_recover"); > + if (error) > + goto next; > } > - if (!error) > - memcpy(xfs_buf_offset(bp, > - (uint)bit << XFS_BLI_SHIFT), /* dest */ > - item->ri_buf[i].i_addr, /* source */ > - nbits<<XFS_BLI_SHIFT); /* length */ > + > + memcpy(xfs_buf_offset(bp, > + (uint)bit << XFS_BLI_SHIFT), /* dest */ > + item->ri_buf[i].i_addr, /* source */ > + nbits<<XFS_BLI_SHIFT); /* length */ > + next: > i++; > bit += nbits; > } > @@ -2615,7 +2629,19 @@ xlog_recover_do_dquot_trans( > return (0); > > recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr; > - ASSERT(recddq); > + > + if (item->ri_buf[1].i_addr == NULL) { > + cmn_err(CE_ALERT, > + "XFS: NULL dquot in %s.", __func__); > + return XFS_ERROR(EIO); > + } > + if (item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) { > + cmn_err(CE_ALERT, > + "XFS: dquot too small (%d) in %s.", > + item->ri_buf[1].i_len, __func__); > + return XFS_ERROR(EIO); > + } > + > /* > * This type of quotas was turned off, so ignore this record. > */ > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-05-27 19:06 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-03-03 17:54 [PATCH] xfs: validate quota log items during log recovery Christoph Hellwig 2009-03-16 7:54 ` Christoph Hellwig 2009-03-29 7:42 ` Christoph Hellwig 2009-05-26 9:11 ` Christoph Hellwig 2009-04-04 11:59 ` Arkadiusz Miskiewicz 2009-05-26 16:07 ` Eric Sandeen 2009-05-27 9:17 ` Christoph Hellwig 2009-05-27 19:06 ` Eric Sandeen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox