diff mbox series

ext2fs: Fix build of new superblock fields on BE machines

Message ID 20181121001127.1868-1-krisman@collabora.com
State Accepted, archived
Headers show
Series ext2fs: Fix build of new superblock fields on BE machines | expand

Commit Message

Gabriel Krisman Bertazi Nov. 21, 2018, 12:11 a.m. UTC
New superblock fields must be byte swapped before being accessed in Big
Endian machines.

Not tested on an actual BE machine.

Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---

I just noticed this today.  I don't see the patch that includes this
field already pushed to kernel.org, so if you want to squash this one to
that patch before pushing to make the result more bisectable in BE
machines, I am ok with that.

 lib/ext2fs/swapfs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Theodore Ts'o Nov. 21, 2018, 4:24 p.m. UTC | #1
On Tue, Nov 20, 2018 at 07:11:27PM -0500, Gabriel Krisman Bertazi wrote:
> New superblock fields must be byte swapped before being accessed in Big
> Endian machines.
> 
> Not tested on an actual BE machine.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> ---
> 
> I just noticed this today.  I don't see the patch that includes this
> field already pushed to kernel.org, so if you want to squash this one to
> that patch before pushing to make the result more bisectable in BE
> machines, I am ok with that.

Oops, sorry, I just pushed it out before I saw your note.  I'll add it
as a follow-on commit right away.  Fortunately we don't do too much
debugging on BE machines, so it's probably not worth breaking the git
rules and rewinding, even though it's highly unlikely anyone has done
a git pull in the last 10 minutes or so.

While we're at it, I should have noticed this earler before I acceptd
patch 2 in your series.  Could you also send a follow up patch which
adds a quick description of the fname_encoding feature to the ext4.5
man page?

Also, did your v2 patch series to e2fsprogs reflect any of the
comments I made late last night on the rest of the v1 patch series?

Thanks, regards,

						- Ted
Gabriel Krisman Bertazi Nov. 21, 2018, 6:27 p.m. UTC | #2
"Theodore Y. Ts'o" <tytso@mit.edu> writes:

> On Tue, Nov 20, 2018 at 07:11:27PM -0500, Gabriel Krisman Bertazi wrote:
>> New superblock fields must be byte swapped before being accessed in Big
>> Endian machines.
>> 
>> Not tested on an actual BE machine.
>> 
>> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
>> ---
>> 
>> I just noticed this today.  I don't see the patch that includes this
>> field already pushed to kernel.org, so if you want to squash this one to
>> that patch before pushing to make the result more bisectable in BE
>> machines, I am ok with that.
>


> Oops, sorry, I just pushed it out before I saw your note.  I'll add it
> as a follow-on commit right away.  Fortunately we don't do too much
> debugging on BE machines, so it's probably not worth breaking the git
> rules and rewinding, even though it's highly unlikely anyone has done
> a git pull in the last 10 minutes or so.
>
> While we're at it, I should have noticed this earler before I acceptd
> patch 2 in your series.  Could you also send a follow up patch which
> adds a quick description of the fname_encoding feature to the ext4.5
> man page?

Hi,

Thanks for pushing it and yes, I can send the follow up patch.

>
> Also, did your v2 patch series to e2fsprogs reflect any of the
> comments I made late last night on the rest of the v1 patch series?

No, it only reflects the comments you made on the night before, up to
patch 3/10, as I hadn't seen your new responses before submitting v2.

So, please disregard v2, and I will follow up with a new iteration
applying your new suggestions.

Thanks.
Theodore Ts'o Nov. 21, 2018, 11:19 p.m. UTC | #3
On Tue, Nov 20, 2018 at 07:11:27PM -0500, Gabriel Krisman Bertazi wrote:
> New superblock fields must be byte swapped before being accessed in Big
> Endian machines.
> 
> Not tested on an actual BE machine.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>

Thanks, applied.

					- Ted
diff mbox series

Patch

diff --git a/lib/ext2fs/swapfs.c b/lib/ext2fs/swapfs.c
index b168a2f20208..a1560045d073 100644
--- a/lib/ext2fs/swapfs.c
+++ b/lib/ext2fs/swapfs.c
@@ -129,8 +129,11 @@  void ext2fs_swap_super(struct ext2_super_block * sb)
 	sb->s_lpf_ino = ext2fs_swab32(sb->s_lpf_ino);
 	sb->s_prj_quota_inum = ext2fs_swab32(sb->s_prj_quota_inum);
 	sb->s_checksum_seed = ext2fs_swab32(sb->s_checksum_seed);
+	/* s_*_time_hi are __u8 and does not need swabbing */
+	sb->s_encoding = ext2fs_swab16(sb->s_encoding);
+	sb->s_encoding_flags = ext2fs_swab16(sb->s_encoding_flags);
 	/* catch when new fields are used from s_reserved */
-	EXT2FS_BUILD_BUG_ON(sizeof(sb->s_reserved) != 98 * sizeof(__le32));
+	EXT2FS_BUILD_BUG_ON(sizeof(sb->s_reserved) != 95 * sizeof(__le32));
 	sb->s_checksum = ext2fs_swab32(sb->s_checksum);
 }