Release candidate for e2fsprogs 1.47.2 is available

Hi all,

There is a release candidate for e2fsprogs 1.47.2 available at:

https://www.kernel.org/pub/linux/kernel/people/tytso/e2fsprogs/testing/v1.47.2-rc1/e2fsprogs-1.47.2-rc1.tar.gz

It is also uploaded to Debian unstable, for folks who want to try that out.

A known issue is that f_clear_orphan_file is failing on s390x, powerpc, and ppc64 which is why Debian packages haven't been built on those architectures:

https://buildd.debian.org/status/package.php?p=e2fsprogs

Please give it a try, and let me know if you run into any other problems.

Many thanks!!

- Ted

IOWs, the big endian architectures.

Hmmm, why does ext2fs_do_orphan_file_block_csum claim to return a __u32 yet the actual return statement returns an __le32:

	return ext2fs_cpu_to_le32(crc);

Particularly because ext2fs_orphan_file_block_csum_verify then compares the returned __le32 value to a __u32 that was converted from the ondisk structure:

	return ext2fs_le32_to_cpu(tail->ob_checksum) == crc;

AFAICT ext2fs_do_orphan_file_block_csum should not do that conversion, and mkorphan_proc should be doing:

	tail->ob_checksum = cpu_to_le32(
			ext2fs_do_orphan_file_block_csum(...));

But I'm not that familiar with this code.

--D

Yeah, I noticed this when I was taking look a bit earlier. The fundamental problem is indeed that ext2fs_orphan_file_block_csum() is returning an on-disk checksum, and on little endian systems, this confusion is harmless, but it's a problem on big endian systems. It doesn't help that in the e2fsprogs's headers, we have:

struct ext4_orphan_block_tail {
	__u32 ob_magic;
	__u32 ob_checksum;
};

... but in the kernel'sheader files we have:

struct ext4_orphan_block_tail {
	__le32 ob_magic;
	__le32 ob_checksum;
};

... and although e2fsprogs had been set up to use sparse as a static code checker some number of years ago, no one has used it in a while, and right now compiling with make C=1 generates a huge amount of noise. (For example, sparse is now warning about unused functions which are delcared "static inline" in header files, and short of just squeching all unusedFunction warnings, there doesn't seem to be an obvious way to get it to shut up about inline functions.

So to fix things on big endian systems, we'll need to clean up the ambiguities. The simplest way to fix things is just to do this:

(sid_ppc64-dchroot)tytso@perotto:~/e2fsprogs/e2fsprogs$ git diff

index 913eb9a0..14b83b74 100644
@@ -271,5 +271,5 @@ int ext2fs_orphan_file_block_csum_verify(ext2_filsys fs, ext2_ino_t ino,
        if (retval)
                return 0;
        tail = ext2fs_orphan_block_tail(fs, buf);
-       return ext2fs_le32_to_cpu(tail->ob_checksum) == crc;
+       return tail->ob_checksum == crc;
 }

But it's not clear this is the cleanest way to fix things. I tend to agree with your suggestion as probably the better one, and that we should be making things explicit with the use of __le32 annotations.

I should also try to see what needs to be done to make using "make C=1" more useful....

- Ted