On 02/14/2018 01:55 AM, David Sterba wrote:
On Tue, Feb 13, 2018 at 06:27:13PM +0800, Anand Jain wrote:
On 02/13/2018 05:01 PM, Qu Wenruo wrote:
On 2018年02月13日 11:00, Anand Jain wrote:
Fixes the endianness bug in the fs_info::super_copy by using its
btrfs_set_super...() function to set values in the SB, as these
functions manage the endianness compatibility nicely.

Signed-off-by: Anand Jain <anand.j...@oracle.com>

Also went through all btrfs_super_block SETGET functions, greping using
\><member name>, seems that there are still some left here:

fs/btrfs/sysfs.c:
In both btrfs_sectorsize_show() and btrfs_clone_alignment_show():
        return snprintf(buf, PAGE_SIZE, "%u\n",
                        fs_info->super_copy->sectorsize);

In btrfs_nodesize_show():
        return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize);

 Thinking again on the printf they are fine without the endianness
 functions, because it is printing the values from the memory to
 buf/stdout, which was previously read from the disk (which
 possibly written by the opposite endianness system). Here, there
 is no endianness changes that will be required for it to be written
 out using printf.

   Oh. Thanks. Will fix. Maybe it's a good idea to add sysfs fixes
   into a new patch.

I'd prefer a single patch as it fixes the same problem for one
structure, the context of use is not that important to justify 2
patches.

 Agree.

I went through the possible uses of superblock again and did not find
anything else than the update_super_roots and sysfs read handlers. There
are some direct uses of super block members, like label, sys_array, uuid
that are passed unconverted and must be accessed via the set/get
helpers.

 AFAIK byte arrays btrfs_super_block::(label, sys_chunk_array, fsid)
 are not affected by endianness, as memory access for the array starts
 from the zero in both endianness. And the lowest granularity affected
 by the endianness is a byte.

In some places the superblock is put to a temporary variable so simple
grep may miss these.

And what about cc this to stable kernel?
IIRC it's a very critical problem for btrfs.

If the filesystem is always used on a same endianity host, this will not
be a problem. Moving between opposite endianity hosts will report bogus
numbers in sysfs and the backup root would not be restored correctly.

As this is not common, I'd rate thats as a bugfix for stable, but "only"
a serious one.

 agree.

Maybe cc: sta...@vger.kernel.org # v3.2+?

   Thanks for the suggestion. Will do. Any idea what if the patch which
   applied on mainline ends up conflict on LTS, so write a separate patch
   to stable?

If the patch does not apply to some older stable branch, all involved
people get a mail from stable team and have an opportunity to send an
updated version of the patch.

Regarding the long-term branches, I would consider 4.4 and up. Anything
older is a plus but fixing merge conflicts is more likely there so I
think the respective maintainers would either fix it by themselves or
ask for help.

 ok.

 Thanks, Anand

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to