Re: udf: Suspicious values in udf_statfs()

["Pali Rohár" <pali.rohar@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3999 bytes --]

On Friday 17 January 2020 13:05:11 Jan Kara wrote:
> On Mon 13-01-20 13:08:51, Jan Kara wrote:
> > Hello,
> > 
> > On Sun 12-01-20 17:23:11, Pali Rohár wrote:
> > > I looked at udf_statfs() implementation and I see there two things which
> > > are probably incorrect:
> > > 
> > > First one:
> > > 
> > > 	buf->f_blocks = sbi->s_partmaps[sbi->s_partition].s_partition_len;
> > > 
> > > If sbi->s_partition points to Metadata partition then reported number
> > > of blocks seems to be incorrect. Similar like in udf_count_free().
> > 
> > Oh, right. This needs similar treatment like udf_count_free(). I'll fix it.
> > Thanks for spotting.
> 
> Patch for this is attached.

I was wrong. After reading UDF specification and kernel implementation
again I realized that there is a complete mess what "partition" means.

Sometimes it is Partition Map, sometimes it is Partition Descriptor,
sometimes it is index of Partition Map, sometimes index of Partition
Descriptor, sometimes it refers to data referenced by Partition
Descriptor and sometimes it refers to data referenced by Partition Map.

And "length" means length of any of above structure (either of map
structure, either of data pointed by map structure, either of descriptor
or either of data pointed by descriptor).

So to make it clear, member "s_partition_len" refers to length of data
pointed by Partition Descriptor, therefore length of physical partition.

As kernel probably does not support UDF fs with more then one Partition
Descriptor, whatever Partition Map we choose (s_partmaps[i] member) we
would always get same value in "s_partition_len" as it does not refer to
data of Partition Map, but rather to data of Partition Descriptor.

As both Metadata Partition Map and Type 1 Partition Map (or Sparable
Partition Map) shares same Partition Descriptor, this patch does not
change value of "f_blocks" member and therefore is not needed at all.
So current code should be correct.

But please, double check that I'm correct as "partition" naming in
probably every one variable is misleading.

Just to note, free space is calculated from Partition Map index
(not from Partition Descriptor index), therefore previous patch for
udf_count_free() is really needed and should be correct.

> From 2e831a1fb4fa4a6843e154edb1d9e80a1c610803 Mon Sep 17 00:00:00 2001
> From: Jan Kara <jack@suse.cz>
> Date: Fri, 17 Jan 2020 12:41:39 +0100
> Subject: [PATCH] udf: Handle metadata partition better for statfs(2)
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> When the filesystem uses Metadata partition, we need to look at the
> underlying partition to get real total number of blocks in the
> filesystem. Do the necessary remapping in udf_statfs().
> 
> Reported-by: Pali Rohár <pali.rohar@gmail.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/udf/super.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index f747bf72edbe..3bb9793db3aa 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -2395,11 +2395,17 @@ static int udf_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	struct udf_sb_info *sbi = UDF_SB(sb);
>  	struct logicalVolIntegrityDescImpUse *lvidiu;
>  	u64 id = huge_encode_dev(sb->s_bdev->bd_dev);
> +	unsigned int part = sbi->s_partition;
> +	int ptype = sbi->s_partmaps[part].s_partition_type;
>  
> +	if (ptype == UDF_METADATA_MAP25) {
> +		part = sbi->s_partmaps[part].s_type_specific.s_metadata.
> +							s_phys_partition_ref;
> +	}
>  	lvidiu = udf_sb_lvidiu(sb);
>  	buf->f_type = UDF_SUPER_MAGIC;
>  	buf->f_bsize = sb->s_blocksize;
> -	buf->f_blocks = sbi->s_partmaps[sbi->s_partition].s_partition_len;
> +	buf->f_blocks = sbi->s_partmaps[part].s_partition_len;
>  	buf->f_bfree = udf_count_free(sb);
>  	buf->f_bavail = buf->f_bfree;
>  	/*

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]