From: Andrew Nayenko <resver@gmail.com>
To: util-linux@vger.kernel.org
Subject: Re: NTFS volume label not found due to endian conversion bug in libblkid
Date: Sat, 04 Dec 2010 14:22:02 +0300 [thread overview]
Message-ID: <4CFA245A.3020900@gmail.com> (raw)
In-Reply-To: <20101203143031.GJ3077@nb.net.home>
>> I've been having a problem with blkid not finding the volume label
>> on NTFS volumes on my big-endian (PowerPC) system. I tracked the
>> issue down to a bug in the endian conversion code for parsing the
>> MFT $Volume attributes in ntfs.c.
>
> Fixed. Thanks!
Another similar typo:
diff --git a/shlibs/blkid/src/superblocks/nilfs.c b/shlibs/blkid/src/superblocks/nilfs.c
index bf16918..1f8f3a6 100644
--- a/shlibs/blkid/src/superblocks/nilfs.c
+++ b/shlibs/blkid/src/superblocks/nilfs.c
@@ -84,7 +84,7 @@ static int probe_nilfs2(blkid_probe pr, const struct blkid_idmag *mag)
if (!sb)
return -1;
- bytes = le32_to_cpu(sb->s_bytes);
+ bytes = le16_to_cpu(sb->s_bytes);
crc = crc32(le32_to_cpu(sb->s_crc_seed), (unsigned char *)sb, sumoff);
crc = crc32(crc, sum, 4);
crc = crc32(crc, (unsigned char *)sb + sumoff + 4, bytes - sumoff - 4);
--
I believe there is a better approach to endianness handling. And compiler is our best
helper here. We just need to make it generate an error when we try to use a field with
wrong (or without any) endianness conversion. To achieve this, fields in structures
should be explicitly declared as little or big endian, e.g.:
struct sb {
.....
xle16_t s_bytes;
.....
};
The xle16_t type should be a complex one, not an integral:
typedef struct { uint16_t __leu16; } xle16_t;
Only appropriate conversion functions should know how to convert those types to
integral ones:
uint16_t xle16_to_cpu(xle16_t v) { return le16_to_cpu(v.__leu16); }
xle16_t xcpu_to_le16(uint16_t v) { xle16_t t = {cpu_to_le16(v)}; return t; }
That's it. Now compiler will enforce us to use the right conversion anytime we
access a field, i.e. the following statements will fail to compile:
uint16_t bytes = sb->s_bytes; /* no conversion */
uint32_t bytes = xle32_to_cpu(sb->s_bytes); /* wrong conversion */
I use this approach in a real life project (FS driver) and it proved to be a good
"bugkeeper".
I had a look at shlibs/blkid/src/superblocks/*.c and I think that most probers can
adopt this approach. But in some cases it can be problematic:
1) sometimes fields endianness can vary (e.g. befs, linux_raid) and we do not know
it at compile time;
2) some probers (zfs) prefer to convert fields to native endianness first and then
just use instead of in-place conversion at any access to the field.
--
Andrew Nayenko <resver@gmail.com>
parent reply other threads:[~2010-12-04 11:22 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <20101203143031.GJ3077@nb.net.home>]
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=4CFA245A.3020900@gmail.com \
--to=resver@gmail.com \
--cc=util-linux@vger.kernel.org \
/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