[PATCH] f2fs: add a max block count for f2fs_map_blocks

[PATCH] f2fs: add a max block count for f2fs_map_blocks

[Yunlei He]

This patch adds a max block count for f2fs_map_blocks

Signed-off-by: Yunlei He <heyunlei@huawei.com>
Signed-off-by: Xue Liu <liuxueliu.liu@huawei.com>
---
 fs/f2fs/data.c | 4 ++++
 fs/f2fs/f2fs.h | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index e34b1bd..2a16c867 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -578,6 +578,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 	map->m_len = 0;
 	map->m_flags = 0;
 
+	/* We can handle the block number less than F2FS_MAX_BLOCKS */
+	if (unlikely(map->m_lblk >= F2FS_MAX_BLOCKS))
+		return -EUCLEAN;
+
 	/* it only supports block size == page size */
 	pgofs =	(pgoff_t)map->m_lblk;
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 19beabe..911c99b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -331,6 +331,9 @@ enum {
 
 #define F2FS_LINK_MAX	0xffffffff	/* maximum link count per file */
 
+
+#define F2FS_MAX_BLOCKS 0x3F015AFF      /* maximum block count per file */
+
 #define MAX_DIR_RA_PAGES	4	/* maximum ra pages of dir */
 
 /* vector size for gang look-up from extent cache that consists of radix tree */
-- 
1.9.1


------------------------------------------------------------------------------

Re: [PATCH] f2fs: add a max block count for f2fs_map_blocks

[Chao Yu]

Hi,

> -----Original Message-----
> From: Yunlei He [mailto:heyunlei@huawei.com]
> Sent: Friday, December 25, 2015 4:48 PM
> To: linux-f2fs-devel@lists.sourceforge.net; jaegeuk@kernel.org; chao2.yu@samsung.com
> Cc: bintian.wang@huawei.com; Yunlei He; Xue Liu
> Subject: [PATCH] f2fs: add a max block count for f2fs_map_blocks
> 
> This patch adds a max block count for f2fs_map_blocks

Maximum file size should be limited by sb->s_maxbytes which was inited
in max_file_size(), so logical block index should not exceed the value
calculated with maxbytes, why would we limit logical block index with
another value?

Thanks,

> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> Signed-off-by: Xue Liu <liuxueliu.liu@huawei.com>
> ---
>  fs/f2fs/data.c | 4 ++++
>  fs/f2fs/f2fs.h | 3 +++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index e34b1bd..2a16c867 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -578,6 +578,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  	map->m_len = 0;
>  	map->m_flags = 0;
> 
> +	/* We can handle the block number less than F2FS_MAX_BLOCKS */
> +	if (unlikely(map->m_lblk >= F2FS_MAX_BLOCKS))
> +		return -EUCLEAN;
> +
>  	/* it only supports block size == page size */
>  	pgofs =	(pgoff_t)map->m_lblk;
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 19beabe..911c99b 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -331,6 +331,9 @@ enum {
> 
>  #define F2FS_LINK_MAX	0xffffffff	/* maximum link count per file */
> 
> +
> +#define F2FS_MAX_BLOCKS 0x3F015AFF      /* maximum block count per file */
> +
>  #define MAX_DIR_RA_PAGES	4	/* maximum ra pages of dir */
> 
>  /* vector size for gang look-up from extent cache that consists of radix tree */
> --
> 1.9.1



------------------------------------------------------------------------------

Re: [PATCH] f2fs: add a max block count for f2fs_map_blocks

[liuxue]

On 2015/12/25 17:25, Chao Yu wrote:
> Hi,
>
>> -----Original Message-----
>> From: Yunlei He [mailto:heyunlei@huawei.com]
>> Sent: Friday, December 25, 2015 4:48 PM
>> To: linux-f2fs-devel@lists.sourceforge.net; jaegeuk@kernel.org; chao2.yu@samsung.com
>> Cc: bintian.wang@huawei.com; Yunlei He; Xue Liu
>> Subject: [PATCH] f2fs: add a max block count for f2fs_map_blocks
>>
>> This patch adds a max block count for f2fs_map_blocks
>
> Maximum file size should be limited by sb->s_maxbytes which was inited
> in max_file_size(), so logical block index should not exceed the value
> calculated with maxbytes, why would we limit logical block index with
> another value?
>
> Thanks,
>

Hi,
Trinity test program will send a block number as parameter into ioctl_fibmap, which will be used in
get_node_path(), when the block number large than f2fs max blocks, it will trigger kernel bug.
So we judge the block number in f2fs_map_blocks(), which reference ext4:

file: fs/ext4/inode.c
function: ext4_map_blocks()

     /* We can handle the block number less than EXT_MAX_BLOCKS */
     if (unlikely(map->m_lblk >= EXT_MAX_BLOCKS))
         return -EFSCORRUPTED;


>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> Signed-off-by: Xue Liu <liuxueliu.liu@huawei.com>
>> ---
>>   fs/f2fs/data.c | 4 ++++
>>   fs/f2fs/f2fs.h | 3 +++
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index e34b1bd..2a16c867 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -578,6 +578,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>>   	map->m_len = 0;
>>   	map->m_flags = 0;
>>
>> +	/* We can handle the block number less than F2FS_MAX_BLOCKS */
>> +	if (unlikely(map->m_lblk >= F2FS_MAX_BLOCKS))
>> +		return -EUCLEAN;
>> +
>>   	/* it only supports block size == page size */
>>   	pgofs =	(pgoff_t)map->m_lblk;
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 19beabe..911c99b 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -331,6 +331,9 @@ enum {
>>
>>   #define F2FS_LINK_MAX	0xffffffff	/* maximum link count per file */
>>
>> +
>> +#define F2FS_MAX_BLOCKS 0x3F015AFF      /* maximum block count per file */
>> +
>>   #define MAX_DIR_RA_PAGES	4	/* maximum ra pages of dir */
>>
>>   /* vector size for gang look-up from extent cache that consists of radix tree */
>> --
>> 1.9.1
>
>
>
> .
>



------------------------------------------------------------------------------

Re: [PATCH] f2fs: add a max block count for f2fs_map_blocks

[Chao Yu]

Hi,

On 12/25/15 6:17 PM, liuxue wrote:
> On 2015/12/25 17:25, Chao Yu wrote:
>> Hi,
>>
>>> -----Original Message-----
>>> From: Yunlei He [mailto:heyunlei@huawei.com]
>>> Sent: Friday, December 25, 2015 4:48 PM
>>> To: linux-f2fs-devel@lists.sourceforge.net; jaegeuk@kernel.org; chao2.yu@samsung.com
>>> Cc: bintian.wang@huawei.com; Yunlei He; Xue Liu
>>> Subject: [PATCH] f2fs: add a max block count for f2fs_map_blocks
>>>
>>> This patch adds a max block count for f2fs_map_blocks
>>
>> Maximum file size should be limited by sb->s_maxbytes which was inited
>> in max_file_size(), so logical block index should not exceed the value
>> calculated with maxbytes, why would we limit logical block index with
>> another value?
>>
>> Thanks,
>>
> 
> Hi,
> Trinity test program will send a block number as parameter into ioctl_fibmap, which will be used in
> get_node_path(), when the block number large than f2fs max blocks, it will trigger kernel bug.
> So we judge the block number in f2fs_map_blocks(), which reference ext4:

Thanks for the explanation, and this makes sense to me, it's better to add above
message in commit log.

> 
> file: fs/ext4/inode.c
> function: ext4_map_blocks()
> 
>      /* We can handle the block number less than EXT_MAX_BLOCKS */
>      if (unlikely(map->m_lblk >= EXT_MAX_BLOCKS))
>          return -EFSCORRUPTED;
> 
> 
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> Signed-off-by: Xue Liu <liuxueliu.liu@huawei.com>
>>> ---
>>>   fs/f2fs/data.c | 4 ++++
>>>   fs/f2fs/f2fs.h | 3 +++
>>>   2 files changed, 7 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index e34b1bd..2a16c867 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -578,6 +578,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,

As you mentioned, this bug will only be triggered in bmap, so how about checking
upper boundary in get_data_block_bmap? Since we could remain bug_on in
get_node_path for other call paths to check whether this is a bug of VFS or not.

>>>   	map->m_len = 0;
>>>   	map->m_flags = 0;
>>>
>>> +	/* We can handle the block number less than F2FS_MAX_BLOCKS */
>>> +	if (unlikely(map->m_lblk >= F2FS_MAX_BLOCKS))
>>> +		return -EUCLEAN;

EFBIG ?

>>> +
>>>   	/* it only supports block size == page size */
>>>   	pgofs =	(pgoff_t)map->m_lblk;
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 19beabe..911c99b 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -331,6 +331,9 @@ enum {
>>>
>>>   #define F2FS_LINK_MAX	0xffffffff	/* maximum link count per file */
>>>
>>> +
>>> +#define F2FS_MAX_BLOCKS 0x3F015AFF      /* maximum block count per file */

Can you introduce F2FS_MAX_BLOCKS with calculation detail of constant
'0x3F015AFF', moreover need to exclude reserved space for inline xattr.

Thanks,

>>> +
>>>   #define MAX_DIR_RA_PAGES	4	/* maximum ra pages of dir */
>>>
>>>   /* vector size for gang look-up from extent cache that consists of radix tree */
>>> --
>>> 1.9.1
>>
>>
>>
>> .
>
> 
> 
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 

------------------------------------------------------------------------------

Re: [PATCH] f2fs: add a max block count for f2fs_map_blocks

[He YunLei]

On 2015/12/26 21:21, Chao Yu wrote:
> Hi,
>
> On 12/25/15 6:17 PM, liuxue wrote:
>> On 2015/12/25 17:25, Chao Yu wrote:
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: Yunlei He [mailto:heyunlei@huawei.com]
>>>> Sent: Friday, December 25, 2015 4:48 PM
>>>> To: linux-f2fs-devel@lists.sourceforge.net; jaegeuk@kernel.org; chao2.yu@samsung.com
>>>> Cc: bintian.wang@huawei.com; Yunlei He; Xue Liu
>>>> Subject: [PATCH] f2fs: add a max block count for f2fs_map_blocks
>>>>
>>>> This patch adds a max block count for f2fs_map_blocks
>>>
>>> Maximum file size should be limited by sb->s_maxbytes which was inited
>>> in max_file_size(), so logical block index should not exceed the value
>>> calculated with maxbytes, why would we limit logical block index with
>>> another value?
>>>
>>> Thanks,
>>>
>>
>> Hi,
>> Trinity test program will send a block number as parameter into ioctl_fibmap, which will be used in
>> get_node_path(), when the block number large than f2fs max blocks, it will trigger kernel bug.
>> So we judge the block number in f2fs_map_blocks(), which reference ext4:
>
> Thanks for the explanation, and this makes sense to me, it's better to add above
> message in commit log.
>
>>
>> file: fs/ext4/inode.c
>> function: ext4_map_blocks()
>>
>>       /* We can handle the block number less than EXT_MAX_BLOCKS */
>>       if (unlikely(map->m_lblk >= EXT_MAX_BLOCKS))
>>           return -EFSCORRUPTED;
>>
>>
>>>>
>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>> Signed-off-by: Xue Liu <liuxueliu.liu@huawei.com>
>>>> ---
>>>>    fs/f2fs/data.c | 4 ++++
>>>>    fs/f2fs/f2fs.h | 3 +++
>>>>    2 files changed, 7 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index e34b1bd..2a16c867 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -578,6 +578,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>
> As you mentioned, this bug will only be triggered in bmap, so how about checking
> upper boundary in get_data_block_bmap? Since we could remain bug_on in
> get_node_path for other call paths to check whether this is a bug of VFS or not.
>
yeah, change here maybe influence dio and fiemap.
>>>>    	map->m_len = 0;
>>>>    	map->m_flags = 0;
>>>>
>>>> +	/* We can handle the block number less than F2FS_MAX_BLOCKS */
>>>> +	if (unlikely(map->m_lblk >= F2FS_MAX_BLOCKS))
>>>> +		return -EUCLEAN;
>
> EFBIG ?

yeah, EFBIG is better.
>
>>>> +
>>>>    	/* it only supports block size == page size */
>>>>    	pgofs =	(pgoff_t)map->m_lblk;
>>>>
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 19beabe..911c99b 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -331,6 +331,9 @@ enum {
>>>>
>>>>    #define F2FS_LINK_MAX	0xffffffff	/* maximum link count per file */
>>>>
>>>> +
>>>> +#define F2FS_MAX_BLOCKS 0x3F015AFF      /* maximum block count per file */
>
> Can you introduce F2FS_MAX_BLOCKS with calculation detail of constant
> '0x3F015AFF', moreover need to exclude reserved space for inline xattr.
>
> Thanks,

I will send version v2 use max_file_size() function.

Thanks,

>
>>>> +
>>>>    #define MAX_DIR_RA_PAGES	4	/* maximum ra pages of dir */
>>>>
>>>>    /* vector size for gang look-up from extent cache that consists of radix tree */
>>>> --
>>>> 1.9.1
>>>
>>>
>>>
>>> .
>>
>>
>>
>>
>> ------------------------------------------------------------------------------
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>
>
> .
>


------------------------------------------------------------------------------