Release candidate for e2fsprogs 1.47.2 is available

Release candidate for e2fsprogs 1.47.2 is available

[Theodore Ts'o]

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

Re: Release candidate for e2fsprogs 1.47.2 is available

[Darrick J. Wong]

On Fri, Nov 29, 2024 at 08:34:29PM -0500, Theodore Ts'o wrote:
> 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

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

> Please give it a try, and let me know if you run into any other
> problems.
> 
> Many thanks!!
> 
> 					- Ted
> 

Re: Release candidate for e2fsprogs 1.47.2 is available

[Theodore Ts'o]

On Sat, Nov 30, 2024 at 12:36:07PM -0800, Darrick J. Wong wrote:
> > 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
> 
> 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);

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
diff --git a/lib/ext2fs/orphan.c b/lib/ext2fs/orphan.c
index 913eb9a0..14b83b74 100644
--- a/lib/ext2fs/orphan.c
+++ b/lib/ext2fs/orphan.c
@@ -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