linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: "Darrick J. Wong" <djwong@us.ibm.com>
Cc: Theodore Ts'o <tytso@mit.edu>, linux-fsdevel@vger.kernel.org
Subject: [RFC] mess in jbd2_block_tag_csum_verify()
Date: Wed, 8 May 2013 16:51:30 +0100	[thread overview]
Message-ID: <20130508155130.GT25399@ZenIV.linux.org.uk> (raw)

You have
	calculated = jbd2_chksum(j, calculated, buf, j->j_blocksize);
	provided = be32_to_cpu(tag->t_checksum);

	return provided == cpu_to_be32(calculated);

in there, which makes no sense whatsoever.  First of all, you are
converting big-endian to native, then another native to big-endian
and compare results.  The bogosity aside, it's equivalent to simply
comparing tag->t_checksum with calculated - cpu_to_be32() is the
same mapping as be32_to_cpu() on all architectures and it's a one-to-one
mapping, at that.

Bogosity, of course, is that tag->t_checksum is apparently big-endian
and definitely a 16bit value.  How in hell is that check going to
yield true?  Note that you are asking for 16 bits out of crc32c result
to be zero, _NOT_ to be ignored.

Producer of that value shoves lower 16 bits of cpu_to_be32(crc) into the
on-disk structure.  Also a bloody bad idea, since the values on little-endian
and big-endian hosts will be different; move the disk from one box to
another and watch the mismatches...

What the hell is going on there?

             reply	other threads:[~2013-05-08 15:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-08 15:51 Al Viro [this message]
2013-05-08 16:04 ` [RFC] mess in jbd2_block_tag_csum_verify() Andreas Dilger
2013-05-08 16:11   ` Andreas Dilger
2013-05-08 16:45   ` Darrick J. Wong
2013-05-08 17:07     ` Al Viro
2013-05-08 21:55       ` Darrick J. Wong
2013-05-08 22:58         ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130508155130.GT25399@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=djwong@us.ibm.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).