Re: [PATCH 3.16 315/410] btrfs: use proper endianness accessors for super_copy

[Ben Hutchings]

On Fri, 2018-06-08 at 04:02 +0800, Anand Jain wrote:
> Hi Ben,
> 
>   This patch was reverted from the mainline and 4.14 stable as it only
>   added half of the endianness conversion and caused regression. This
>   patch should not be included.

Oh yes, sorry I didn't spot that.  I'll drop it.

Ben.

> On 06/07/2018 10:05 PM, Ben Hutchings wrote:
> > 3.16.57-rc1 review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Anand Jain <anand.jain@xxxxxxxxxx>
> > 
> > commit 3c181c12c431fe33b669410d663beb9cceefcd1b upstream.
> > 
> > The fs_info::super_copy is a byte copy of the on-disk structure and all
> > members must use the accessor macros/functions to obtain the right
> > value.  This was missing in update_super_roots and in sysfs readers.
> > 
> > Moving between opposite endianness hosts will report bogus numbers in
> > sysfs, and mount may fail as the root will not be restored correctly. If
> > the filesystem is always used on a same endian host, this will not be a
> > problem.
> > 
> > Fix this by using the btrfs_set_super...() functions to set
> > fs_info::super_copy values, and for the sysfs, use the cached
> > fs_info::nodesize/sectorsize values.
> > 
> > Fixes: df93589a17378 ("btrfs: export more from FS_INFO to sysfs")
> > Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
> > Reviewed-by: Liu Bo <bo.li.liu@xxxxxxxxxx>
> > Reviewed-by: David Sterba <dsterba@xxxxxxxx>
> > [ update changelog ]
> > Signed-off-by: David Sterba <dsterba@xxxxxxxx>
> > [bwh: Backported to 3.16:
> >   - btrfs_fs_info doesn't have cached nodesize or sectorsize fields, so use
> >     the accessor functions
> >   - Adjust context]
> > Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
> > ---
> > --- a/fs/btrfs/sysfs.c
> > +++ b/fs/btrfs/sysfs.c
> > @@ -406,7 +406,7 @@ static ssize_t btrfs_nodesize_show(struc
> >   {
> >   	struct btrfs_fs_info *fs_info = to_fs_info(kobj);
> >   
> > -	return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize);
> > +	return snprintf(buf, PAGE_SIZE, "%u\n", btrfs_super_nodesize(fs_info->super_copy));
> >   }
> >   
> >   BTRFS_ATTR_RW(nodesize, 0444, btrfs_nodesize_show, btrfs_no_store);
> > @@ -416,7 +416,7 @@ static ssize_t btrfs_sectorsize_show(str
> >   {
> >   	struct btrfs_fs_info *fs_info = to_fs_info(kobj);
> >   
> > -	return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->sectorsize);
> > +	return snprintf(buf, PAGE_SIZE, "%u\n", btrfs_super_sectorsize(fs_info->super_copy));
> >   }
> >   
> >   BTRFS_ATTR_RW(sectorsize, 0444, btrfs_sectorsize_show, btrfs_no_store);
> > @@ -426,7 +426,7 @@ static ssize_t btrfs_clone_alignment_sho
> >   {
> >   	struct btrfs_fs_info *fs_info = to_fs_info(kobj);
> >   
> > -	return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->sectorsize);
> > +	return snprintf(buf, PAGE_SIZE, "%u\n", btrfs_super_sectorsize(fs_info->super_copy));
> >   }
> >   
> >   BTRFS_ATTR_RW(clone_alignment, 0444, btrfs_clone_alignment_show, btrfs_no_store);
> > --- a/fs/btrfs/transaction.c
> > +++ b/fs/btrfs/transaction.c
> > @@ -1428,19 +1428,23 @@ static void update_super_roots(struct bt
> >   
> >   	super = root->fs_info->super_copy;
> >   
> > +	/* update latest btrfs_super_block::chunk_root refs */
> >   	root_item = &root->fs_info->chunk_root->root_item;
> > -	super->chunk_root = root_item->bytenr;
> > -	super->chunk_root_generation = root_item->generation;
> > -	super->chunk_root_level = root_item->level;
> > +	btrfs_set_super_chunk_root(super, root_item->bytenr);
> > +	btrfs_set_super_chunk_root_generation(super, root_item->generation);
> > +	btrfs_set_super_chunk_root_level(super, root_item->level);
> >   
> > +	/* update latest btrfs_super_block::root refs */
> >   	root_item = &root->fs_info->tree_root->root_item;
> > -	super->root = root_item->bytenr;
> > -	super->generation = root_item->generation;
> > -	super->root_level = root_item->level;
> > +	btrfs_set_super_root(super, root_item->bytenr);
> > +	btrfs_set_super_generation(super, root_item->generation);
> > +	btrfs_set_super_root_level(super, root_item->level);
> > +
> >   	if (btrfs_test_opt(root, SPACE_CACHE))
> > -		super->cache_generation = root_item->generation;
> > +		btrfs_set_super_cache_generation(super, root_item->generation);
> >   	if (root->fs_info->update_uuid_tree_gen)
> > -		super->uuid_tree_generation = root_item->generation;
> > +		btrfs_set_super_uuid_tree_generation(super,
> > +						     root_item->generation);
> >   }
> >   
> >   int btrfs_transaction_in_commit(struct btrfs_fs_info *info)
> > 
-- 
Ben Hutchings
When in doubt, use brute force. - Ken Thompson

Attachment: signature.asc
Description: This is a digitally signed message part