lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140505222333.GI8434@birch.djwong.org>
Date:	Mon, 5 May 2014 15:23:33 -0700
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	Lukáš Czerner <lczerner@...hat.com>
Cc:	tytso@....edu, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 05/37] debugfs: teach logdump to deal with 64bit revoke
 tables

On Fri, May 02, 2014 at 01:38:04PM +0200, Lukáš Czerner wrote:
> On Thu, 1 May 2014, Darrick J. Wong wrote:
> 
> > Date: Thu, 01 May 2014 16:12:55 -0700
> > From: Darrick J. Wong <darrick.wong@...cle.com>
> > To: tytso@....edu, darrick.wong@...cle.com
> > Cc: linux-ext4@...r.kernel.org
> > Subject: [PATCH 05/37] debugfs: teach logdump to deal with 64bit revoke tables
> > 
> > The logdump command doesn't know how to deal with revoke tables in
> > 64bit journals, so teach it to do this.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@...cle.com>
> > ---
> >  debugfs/logdump.c          |   20 ++++-
> >  tests/f_jnl_64bit/expect.0 |  171 --------------------------------------------
> >  2 files changed, 15 insertions(+), 176 deletions(-)
> > 
> > 
> > diff --git a/debugfs/logdump.c b/debugfs/logdump.c
> > index 2d0efaf..8b9dc5b 100644
> > --- a/debugfs/logdump.c
> > +++ b/debugfs/logdump.c
> > @@ -526,28 +526,38 @@ static void dump_revoke_block(FILE *out_file, char *buf,
> >  {
> >  	int			offset, max;
> >  	journal_revoke_header_t *header;
> > -	unsigned int		*entry, rblock;
> > +	unsigned int		*entry;
> > +	unsigned long long	*bentry, rblock;
> > +	int			tag_size = sizeof(*entry);
> >  
> >  	if (dump_all)
> >  		fprintf(out_file, "Dumping revoke block, sequence %u, at "
> >  			"block %u:\n", transaction, blocknr);
> >  
> > +	if (be32_to_cpu(jsb->s_feature_incompat) & JFS_FEATURE_INCOMPAT_64BIT)
> > +		tag_size = sizeof(*bentry);
> > +
> >  	header = (journal_revoke_header_t *) buf;
> >  	offset = sizeof(journal_revoke_header_t);
> >  	max = be32_to_cpu(header->r_count);
> >  
> >  	while (offset < max) {
> > -		entry = (unsigned int *) (buf + offset);
> > -		rblock = be32_to_cpu(*entry);
> > +		if (tag_size == sizeof(*entry)) {
> > +			entry = (unsigned int *) (buf + offset);
> > +			rblock = be32_to_cpu(*entry);
> > +		} else {
> > +			bentry = (unsigned long long *)(buf + offset);
> > +			rblock = ext2fs_be64_to_cpu(*bentry);
> > +		}
> 
> I wonder whether we really need to have bentry and entry since those
> are just pointers and should be of the same size regardless of what
> they are pointing at.
> 
> Would not it be better from the readability pov ? Otherwise it looks
> good.

One could eliminate the local variables by writing it as such:

if (...)
	rblock = be32_to_cpu(*((__u32 *)(buf + offset)));
else
	rblock = ext2fs_be64_to_cpu(*((__u64 *)(buf + offset)));

The parentheses are a little harder to figure out in the second version, but I
don't have a strong opinion either way.

--D
> 
> Thanks!
> -Lukas
> 
> >  		if (dump_all || rblock == block_to_dump) {
> > -			fprintf(out_file, "  Revoke FS block %u", rblock);
> > +			fprintf(out_file, "  Revoke FS block %llu", rblock);
> >  			if (dump_all)
> >  				fprintf(out_file, "\n");
> >  			else
> >  				fprintf(out_file," at block %u, sequence %u\n",
> >  					blocknr, transaction);
> >  		}
> > -		offset += 4;
> > +		offset += tag_size;
> >  	}
> >  }
> >  
> > diff --git a/tests/f_jnl_64bit/expect.0 b/tests/f_jnl_64bit/expect.0
> > index 2007f03..5cef2d8 100644
> > --- a/tests/f_jnl_64bit/expect.0
> > +++ b/tests/f_jnl_64bit/expect.0
> > @@ -1,189 +1,97 @@
> >  Journal starts at block 67, transaction 32
> >  Found expected sequence 32, type 5 (revoke table) at block 67
> >  Dumping revoke block, sequence 32, at block 67:
> > -  Revoke FS block 0
> >    Revoke FS block 1536
> > -  Revoke FS block 0
> >    Revoke FS block 1472
> > -  Revoke FS block 0
> >    Revoke FS block 1473
> > -  Revoke FS block 0
> >    Revoke FS block 1474
> > -  Revoke FS block 0
> >    Revoke FS block 1475
> > -  Revoke FS block 0
> >    Revoke FS block 1476
> > -  Revoke FS block 0
> >    Revoke FS block 1541
> > -  Revoke FS block 0
> >    Revoke FS block 1477
> > -  Revoke FS block 0
> >    Revoke FS block 1478
> > -  Revoke FS block 0
> >    Revoke FS block 1479
> > -  Revoke FS block 0
> >    Revoke FS block 1480
> > -  Revoke FS block 0
> >    Revoke FS block 1481
> > -  Revoke FS block 0
> >    Revoke FS block 1482
> > -  Revoke FS block 0
> >    Revoke FS block 1483
> > -  Revoke FS block 0
> >    Revoke FS block 1484
> > -  Revoke FS block 0
> >    Revoke FS block 1485
> > -  Revoke FS block 0
> >    Revoke FS block 1486
> > -  Revoke FS block 0
> >    Revoke FS block 1487
> > -  Revoke FS block 0
> >    Revoke FS block 1488
> > -  Revoke FS block 0
> >    Revoke FS block 1489
> > -  Revoke FS block 0
> >    Revoke FS block 1490
> > -  Revoke FS block 0
> >    Revoke FS block 1491
> > -  Revoke FS block 0
> >    Revoke FS block 1556
> > -  Revoke FS block 0
> >    Revoke FS block 1492
> > -  Revoke FS block 0
> >    Revoke FS block 1493
> > -  Revoke FS block 0
> >    Revoke FS block 1429
> > -  Revoke FS block 0
> >    Revoke FS block 1494
> > -  Revoke FS block 0
> >    Revoke FS block 1495
> > -  Revoke FS block 0
> >    Revoke FS block 1496
> > -  Revoke FS block 0
> >    Revoke FS block 1432
> > -  Revoke FS block 0
> >    Revoke FS block 1497
> > -  Revoke FS block 0
> >    Revoke FS block 1498
> > -  Revoke FS block 0
> >    Revoke FS block 1434
> > -  Revoke FS block 0
> >    Revoke FS block 1499
> > -  Revoke FS block 0
> >    Revoke FS block 1435
> > -  Revoke FS block 0
> >    Revoke FS block 1500
> > -  Revoke FS block 0
> >    Revoke FS block 1501
> > -  Revoke FS block 0
> >    Revoke FS block 1502
> > -  Revoke FS block 0
> >    Revoke FS block 1503
> > -  Revoke FS block 0
> >    Revoke FS block 1504
> > -  Revoke FS block 0
> >    Revoke FS block 1505
> > -  Revoke FS block 0
> >    Revoke FS block 1506
> > -  Revoke FS block 0
> >    Revoke FS block 1442
> > -  Revoke FS block 0
> >    Revoke FS block 1507
> > -  Revoke FS block 0
> >    Revoke FS block 1508
> > -  Revoke FS block 0
> >    Revoke FS block 1444
> > -  Revoke FS block 0
> >    Revoke FS block 1509
> > -  Revoke FS block 0
> >    Revoke FS block 1445
> > -  Revoke FS block 0
> >    Revoke FS block 1510
> > -  Revoke FS block 0
> >    Revoke FS block 1511
> > -  Revoke FS block 0
> >    Revoke FS block 1512
> > -  Revoke FS block 0
> >    Revoke FS block 1513
> > -  Revoke FS block 0
> >    Revoke FS block 1449
> > -  Revoke FS block 0
> >    Revoke FS block 1514
> > -  Revoke FS block 0
> >    Revoke FS block 1515
> > -  Revoke FS block 0
> >    Revoke FS block 1516
> > -  Revoke FS block 0
> >    Revoke FS block 1517
> > -  Revoke FS block 0
> >    Revoke FS block 1453
> > -  Revoke FS block 0
> >    Revoke FS block 1518
> > -  Revoke FS block 0
> >    Revoke FS block 1519
> > -  Revoke FS block 0
> >    Revoke FS block 1520
> > -  Revoke FS block 0
> >    Revoke FS block 1456
> > -  Revoke FS block 0
> >    Revoke FS block 1521
> > -  Revoke FS block 0
> >    Revoke FS block 1457
> > -  Revoke FS block 0
> >    Revoke FS block 1522
> > -  Revoke FS block 0
> >    Revoke FS block 1458
> > -  Revoke FS block 0
> >    Revoke FS block 1523
> > -  Revoke FS block 0
> >    Revoke FS block 1459
> > -  Revoke FS block 0
> >    Revoke FS block 1524
> > -  Revoke FS block 0
> >    Revoke FS block 1460
> > -  Revoke FS block 0
> >    Revoke FS block 1525
> > -  Revoke FS block 0
> >    Revoke FS block 1461
> > -  Revoke FS block 0
> >    Revoke FS block 1526
> > -  Revoke FS block 0
> >    Revoke FS block 1462
> > -  Revoke FS block 0
> >    Revoke FS block 1527
> > -  Revoke FS block 0
> >    Revoke FS block 1463
> > -  Revoke FS block 0
> >    Revoke FS block 1528
> > -  Revoke FS block 0
> >    Revoke FS block 1464
> > -  Revoke FS block 0
> >    Revoke FS block 1529
> > -  Revoke FS block 0
> >    Revoke FS block 1465
> > -  Revoke FS block 0
> >    Revoke FS block 1530
> > -  Revoke FS block 0
> >    Revoke FS block 1466
> > -  Revoke FS block 0
> >    Revoke FS block 1531
> > -  Revoke FS block 0
> >    Revoke FS block 1467
> > -  Revoke FS block 0
> >    Revoke FS block 1532
> > -  Revoke FS block 0
> >    Revoke FS block 1468
> > -  Revoke FS block 0
> >    Revoke FS block 1533
> > -  Revoke FS block 0
> >    Revoke FS block 1469
> > -  Revoke FS block 0
> >    Revoke FS block 1534
> > -  Revoke FS block 0
> >    Revoke FS block 1470
> > -  Revoke FS block 0
> >    Revoke FS block 1535
> > -  Revoke FS block 0
> >    Revoke FS block 1471
> >  Found expected sequence 32, type 1 (descriptor block) at block 68
> >  Dumping descriptor block, sequence 32, at block 68:
> > @@ -323,163 +231,84 @@ Dumping descriptor block, sequence 32, at block 150:
> >  Found expected sequence 32, type 2 (commit block) at block 201
> >  Found expected sequence 33, type 5 (revoke table) at block 202
> >  Dumping revoke block, sequence 33, at block 202:
> > -  Revoke FS block 0
> >    Revoke FS block 1600
> > -  Revoke FS block 0
> >    Revoke FS block 1601
> > -  Revoke FS block 0
> >    Revoke FS block 1537
> > -  Revoke FS block 0
> >    Revoke FS block 1602
> > -  Revoke FS block 0
> >    Revoke FS block 1538
> > -  Revoke FS block 0
> >    Revoke FS block 1603
> > -  Revoke FS block 0
> >    Revoke FS block 1539
> > -  Revoke FS block 0
> >    Revoke FS block 1604
> > -  Revoke FS block 0
> >    Revoke FS block 1540
> > -  Revoke FS block 0
> >    Revoke FS block 1605
> > -  Revoke FS block 0
> >    Revoke FS block 1606
> > -  Revoke FS block 0
> >    Revoke FS block 1542
> > -  Revoke FS block 0
> >    Revoke FS block 1607
> > -  Revoke FS block 0
> >    Revoke FS block 1543
> > -  Revoke FS block 0
> >    Revoke FS block 1608
> > -  Revoke FS block 0
> >    Revoke FS block 1544
> > -  Revoke FS block 0
> >    Revoke FS block 1609
> > -  Revoke FS block 0
> >    Revoke FS block 1545
> > -  Revoke FS block 0
> >    Revoke FS block 1610
> > -  Revoke FS block 0
> >    Revoke FS block 1546
> > -  Revoke FS block 0
> >    Revoke FS block 1611
> > -  Revoke FS block 0
> >    Revoke FS block 1547
> > -  Revoke FS block 0
> >    Revoke FS block 1612
> > -  Revoke FS block 0
> >    Revoke FS block 1548
> > -  Revoke FS block 0
> >    Revoke FS block 1613
> > -  Revoke FS block 0
> >    Revoke FS block 1549
> > -  Revoke FS block 0
> >    Revoke FS block 1614
> > -  Revoke FS block 0
> >    Revoke FS block 1550
> > -  Revoke FS block 0
> >    Revoke FS block 1615
> > -  Revoke FS block 0
> >    Revoke FS block 1551
> > -  Revoke FS block 0
> >    Revoke FS block 1616
> > -  Revoke FS block 0
> >    Revoke FS block 1552
> > -  Revoke FS block 0
> >    Revoke FS block 1617
> > -  Revoke FS block 0
> >    Revoke FS block 1553
> > -  Revoke FS block 0
> >    Revoke FS block 1554
> > -  Revoke FS block 0
> >    Revoke FS block 1555
> > -  Revoke FS block 0
> >    Revoke FS block 1557
> > -  Revoke FS block 0
> >    Revoke FS block 1558
> > -  Revoke FS block 0
> >    Revoke FS block 1559
> > -  Revoke FS block 0
> >    Revoke FS block 1560
> > -  Revoke FS block 0
> >    Revoke FS block 1561
> > -  Revoke FS block 0
> >    Revoke FS block 1562
> > -  Revoke FS block 0
> >    Revoke FS block 1563
> > -  Revoke FS block 0
> >    Revoke FS block 1564
> > -  Revoke FS block 0
> >    Revoke FS block 1565
> > -  Revoke FS block 0
> >    Revoke FS block 1566
> > -  Revoke FS block 0
> >    Revoke FS block 1567
> > -  Revoke FS block 0
> >    Revoke FS block 1568
> > -  Revoke FS block 0
> >    Revoke FS block 1569
> > -  Revoke FS block 0
> >    Revoke FS block 1570
> > -  Revoke FS block 0
> >    Revoke FS block 1571
> > -  Revoke FS block 0
> >    Revoke FS block 1572
> > -  Revoke FS block 0
> >    Revoke FS block 1573
> > -  Revoke FS block 0
> >    Revoke FS block 1574
> > -  Revoke FS block 0
> >    Revoke FS block 1575
> > -  Revoke FS block 0
> >    Revoke FS block 1576
> > -  Revoke FS block 0
> >    Revoke FS block 1577
> > -  Revoke FS block 0
> >    Revoke FS block 1578
> > -  Revoke FS block 0
> >    Revoke FS block 1579
> > -  Revoke FS block 0
> >    Revoke FS block 1580
> > -  Revoke FS block 0
> >    Revoke FS block 1581
> > -  Revoke FS block 0
> >    Revoke FS block 1582
> > -  Revoke FS block 0
> >    Revoke FS block 1583
> > -  Revoke FS block 0
> >    Revoke FS block 1584
> > -  Revoke FS block 0
> >    Revoke FS block 1585
> > -  Revoke FS block 0
> >    Revoke FS block 1586
> > -  Revoke FS block 0
> >    Revoke FS block 1587
> > -  Revoke FS block 0
> >    Revoke FS block 1588
> > -  Revoke FS block 0
> >    Revoke FS block 1589
> > -  Revoke FS block 0
> >    Revoke FS block 1590
> > -  Revoke FS block 0
> >    Revoke FS block 1591
> > -  Revoke FS block 0
> >    Revoke FS block 1592
> > -  Revoke FS block 0
> >    Revoke FS block 1593
> > -  Revoke FS block 0
> >    Revoke FS block 1594
> > -  Revoke FS block 0
> >    Revoke FS block 1595
> > -  Revoke FS block 0
> >    Revoke FS block 1596
> > -  Revoke FS block 0
> >    Revoke FS block 1597
> > -  Revoke FS block 0
> >    Revoke FS block 1598
> > -  Revoke FS block 0
> >    Revoke FS block 1599
> >  Found expected sequence 33, type 1 (descriptor block) at block 203
> >  Dumping descriptor block, sequence 33, at block 203:
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists