Re: [PATCH 1/1] f2fs: fix validation of the block count in sanity_check_raw_super

2018-12-24 Thread Chao Yu
On 2018/12/22 19:08, Martin Blumenstingl wrote:
> On Sat, Dec 22, 2018 at 8:00 AM kbuild test robot  wrote:
>>
>> Hi Martin,
>>
>> Thank you for the patch! Perhaps something to improve:
>>
>> [auto build test WARNING on f2fs/dev-test]
>> [also build test WARNING on v4.20-rc7 next-20181221]
>> [if your patch is applied to the wrong git tree, please drop us a note to 
>> help improve the system]
>>
>> url:
>> https://github.com/0day-ci/linux/commits/Martin-Blumenstingl/f2fs-fix-sanity_check_raw_super-on-big-endian-machines/20181222-142616
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git 
>> dev-test
>> config: i386-randconfig-x006-201850 (attached as .config)
>> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
>> reproduce:
>> # save the attached .config to linux build tree
>> make ARCH=i386
>>
>> All warnings (new ones prefixed by >>):
>>
>>fs/f2fs/super.c: In function 'sanity_check_raw_super':
 fs/f2fs/super.c:2498:46: warning: format '%u' expects argument of type 
 'unsigned int', but argument 5 has type 'long long unsigned int' 
 [-Wformat=]
>>"Wrong segment_count / block_count (%u > %u)",
>> ~^
>> %llu
> I sent a v2 with this fixed and Chao Yu's reviewed by included

Sorry, I didn't catch previous print issue on v1, anyway, I'm okay with v2. :)

Thanks,

> 
> .
> 



Re: [PATCH 1/1] f2fs: fix validation of the block count in sanity_check_raw_super

2018-12-22 Thread Martin Blumenstingl
On Sat, Dec 22, 2018 at 8:00 AM kbuild test robot  wrote:
>
> Hi Martin,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on f2fs/dev-test]
> [also build test WARNING on v4.20-rc7 next-20181221]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Martin-Blumenstingl/f2fs-fix-sanity_check_raw_super-on-big-endian-machines/20181222-142616
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git 
> dev-test
> config: i386-randconfig-x006-201850 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All warnings (new ones prefixed by >>):
>
>fs/f2fs/super.c: In function 'sanity_check_raw_super':
> >> fs/f2fs/super.c:2498:46: warning: format '%u' expects argument of type 
> >> 'unsigned int', but argument 5 has type 'long long unsigned int' 
> >> [-Wformat=]
>"Wrong segment_count / block_count (%u > %u)",
> ~^
> %llu
I sent a v2 with this fixed and Chao Yu's reviewed by included


Re: [PATCH 1/1] f2fs: fix validation of the block count in sanity_check_raw_super

2018-12-22 Thread kbuild test robot
Hi Martin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on f2fs/dev-test]
[also build test WARNING on v4.20-rc7 next-20181221]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Martin-Blumenstingl/f2fs-fix-sanity_check_raw_super-on-big-endian-machines/20181222-142616
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git 
dev-test
config: i386-randconfig-a3-12211242 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   fs/f2fs/super.c: In function 'sanity_check_raw_super':
>> fs/f2fs/super.c:2499:4: warning: format '%u' expects argument of type 
>> 'unsigned int', but argument 5 has type '__le64' [-Wformat=]
   segment_count, le64_to_cpu(raw_super->block_count));
   ^

vim +2499 fs/f2fs/super.c

  2382  
  2383  static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
  2384  struct buffer_head *bh)
  2385  {
  2386  block_t segment_count, segs_per_sec, secs_per_zone;
  2387  block_t total_sections, blocks_per_seg;
  2388  struct f2fs_super_block *raw_super = (struct f2fs_super_block *)
  2389  (bh->b_data + 
F2FS_SUPER_OFFSET);
  2390  struct super_block *sb = sbi->sb;
  2391  unsigned int blocksize;
  2392  size_t crc_offset = 0;
  2393  __u32 crc = 0;
  2394  
  2395  /* Check checksum_offset and crc in superblock */
  2396  if (__F2FS_HAS_FEATURE(raw_super, F2FS_FEATURE_SB_CHKSUM)) {
  2397  crc_offset = le32_to_cpu(raw_super->checksum_offset);
  2398  if (crc_offset !=
  2399  offsetof(struct f2fs_super_block, crc)) {
  2400  f2fs_msg(sb, KERN_INFO,
  2401  "Invalid SB checksum offset: %zu",
  2402  crc_offset);
  2403  return 1;
  2404  }
  2405  crc = le32_to_cpu(raw_super->crc);
  2406  if (!f2fs_crc_valid(sbi, crc, raw_super, crc_offset)) {
  2407  f2fs_msg(sb, KERN_INFO,
  2408  "Invalid SB checksum value: %u", crc);
  2409  return 1;
  2410  }
  2411  }
  2412  
  2413  if (F2FS_SUPER_MAGIC != le32_to_cpu(raw_super->magic)) {
  2414  f2fs_msg(sb, KERN_INFO,
  2415  "Magic Mismatch, valid(0x%x) - read(0x%x)",
  2416  F2FS_SUPER_MAGIC, 
le32_to_cpu(raw_super->magic));
  2417  return 1;
  2418  }
  2419  
  2420  /* Currently, support only 4KB page cache size */
  2421  if (F2FS_BLKSIZE != PAGE_SIZE) {
  2422  f2fs_msg(sb, KERN_INFO,
  2423  "Invalid page_cache_size (%lu), supports only 
4KB\n",
  2424  PAGE_SIZE);
  2425  return 1;
  2426  }
  2427  
  2428  /* Currently, support only 4KB block size */
  2429  blocksize = 1 << le32_to_cpu(raw_super->log_blocksize);
  2430  if (blocksize != F2FS_BLKSIZE) {
  2431  f2fs_msg(sb, KERN_INFO,
  2432  "Invalid blocksize (%u), supports only 4KB\n",
  2433  blocksize);
  2434  return 1;
  2435  }
  2436  
  2437  /* check log blocks per segment */
  2438  if (le32_to_cpu(raw_super->log_blocks_per_seg) != 9) {
  2439  f2fs_msg(sb, KERN_INFO,
  2440  "Invalid log blocks per segment (%u)\n",
  2441  le32_to_cpu(raw_super->log_blocks_per_seg));
  2442  return 1;
  2443  }
  2444  
  2445  /* Currently, support 512/1024/2048/4096 bytes sector size */
  2446  if (le32_to_cpu(raw_super->log_sectorsize) >
  2447  F2FS_MAX_LOG_SECTOR_SIZE ||
  2448  le32_to_cpu(raw_super->log_sectorsize) <
  2449  F2FS_MIN_LOG_SECTOR_SIZE) {
  2450  f2fs_msg(sb, KERN_INFO, "Invalid log sectorsize (%u)",
  2451  le32_to_cpu(raw_super->log_sectorsize));
  2452  return 1;
  2453  }
  2454  if (le32_to_cpu(raw_super->log_sectors_per_block) +
  2455  le32_to_cpu(raw_super->log_sectorsize) !=
  2456  F2FS_MAX_LOG_SECTOR_SIZE) {
  2457  f2fs_msg(sb, KERN_INFO,
  2458  "Invalid log sectors per block(%u) log 
sectorsize(%u)",
  2459  

Re: [PATCH 1/1] f2fs: fix validation of the block count in sanity_check_raw_super

2018-12-22 Thread kbuild test robot
Hi Martin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on f2fs/dev-test]
[also build test WARNING on v4.20-rc7 next-20181221]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Martin-Blumenstingl/f2fs-fix-sanity_check_raw_super-on-big-endian-machines/20181222-142616
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git 
dev-test
config: i386-randconfig-x006-201850 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   fs/f2fs/super.c: In function 'sanity_check_raw_super':
>> fs/f2fs/super.c:2498:46: warning: format '%u' expects argument of type 
>> 'unsigned int', but argument 5 has type 'long long unsigned int' [-Wformat=]
   "Wrong segment_count / block_count (%u > %u)",
~^
%llu

vim +2498 fs/f2fs/super.c

9a59b62fd Chao Yu 2015-12-15  2382  
df728b0f6 Jaegeuk Kim 2016-03-23  2383  static int 
sanity_check_raw_super(struct f2fs_sb_info *sbi,
fd694733d Jaegeuk Kim 2016-03-20  2384  
struct buffer_head *bh)
aff063e26 Jaegeuk Kim 2012-11-02  2385  {
0cfe75c5b Jaegeuk Kim 2018-04-27  2386  block_t segment_count, 
segs_per_sec, secs_per_zone;
0cfe75c5b Jaegeuk Kim 2018-04-27  2387  block_t total_sections, 
blocks_per_seg;
fd694733d Jaegeuk Kim 2016-03-20  2388  struct f2fs_super_block 
*raw_super = (struct f2fs_super_block *)
fd694733d Jaegeuk Kim 2016-03-20  2389  
(bh->b_data + F2FS_SUPER_OFFSET);
df728b0f6 Jaegeuk Kim 2016-03-23  2390  struct super_block *sb 
= sbi->sb;
aff063e26 Jaegeuk Kim 2012-11-02  2391  unsigned int blocksize;
d440c52d3 Junling Zheng   2018-09-28  2392  size_t crc_offset = 0;
d440c52d3 Junling Zheng   2018-09-28  2393  __u32 crc = 0;
d440c52d3 Junling Zheng   2018-09-28  2394  
d440c52d3 Junling Zheng   2018-09-28  2395  /* Check 
checksum_offset and crc in superblock */
7beb01f74 Chao Yu 2018-10-24  2396  if 
(__F2FS_HAS_FEATURE(raw_super, F2FS_FEATURE_SB_CHKSUM)) {
d440c52d3 Junling Zheng   2018-09-28  2397  crc_offset = 
le32_to_cpu(raw_super->checksum_offset);
d440c52d3 Junling Zheng   2018-09-28  2398  if (crc_offset 
!=
d440c52d3 Junling Zheng   2018-09-28  2399  
offsetof(struct f2fs_super_block, crc)) {
d440c52d3 Junling Zheng   2018-09-28  2400  
f2fs_msg(sb, KERN_INFO,
d440c52d3 Junling Zheng   2018-09-28  2401  
"Invalid SB checksum offset: %zu",
d440c52d3 Junling Zheng   2018-09-28  2402  
crc_offset);
d440c52d3 Junling Zheng   2018-09-28  2403  return 
1;
d440c52d3 Junling Zheng   2018-09-28  2404  }
d440c52d3 Junling Zheng   2018-09-28  2405  crc = 
le32_to_cpu(raw_super->crc);
d440c52d3 Junling Zheng   2018-09-28  2406  if 
(!f2fs_crc_valid(sbi, crc, raw_super, crc_offset)) {
d440c52d3 Junling Zheng   2018-09-28  2407  
f2fs_msg(sb, KERN_INFO,
d440c52d3 Junling Zheng   2018-09-28  2408  
"Invalid SB checksum value: %u", crc);
d440c52d3 Junling Zheng   2018-09-28  2409  return 
1;
d440c52d3 Junling Zheng   2018-09-28  2410  }
d440c52d3 Junling Zheng   2018-09-28  2411  }
aff063e26 Jaegeuk Kim 2012-11-02  2412  
a07ef7843 Namjae Jeon 2012-12-30  2413  if (F2FS_SUPER_MAGIC != 
le32_to_cpu(raw_super->magic)) {
a07ef7843 Namjae Jeon 2012-12-30  2414  f2fs_msg(sb, 
KERN_INFO,
a07ef7843 Namjae Jeon 2012-12-30  2415  "Magic 
Mismatch, valid(0x%x) - read(0x%x)",
a07ef7843 Namjae Jeon 2012-12-30  2416  
F2FS_SUPER_MAGIC, le32_to_cpu(raw_super->magic));
aff063e26 Jaegeuk Kim 2012-11-02  2417  return 1;
a07ef7843 Namjae Jeon 2012-12-30  2418  }
aff063e26 Jaegeuk Kim 2012-11-02  2419  
5c9b46929 majianpeng  2013-02-01  2420  /* Currently, support 
only 4KB page cache size */
09cbfeaf1 Kirill A. Shutemov  2016-04-01  2421  if (F2FS_BLKSIZE != 
PAGE_SIZE) {
5c9b46929 majianpeng  2013-02-01  2422  f2fs_msg(sb, 
KERN_INFO,
14d7e9de0 majianpeng  2013-02-01  2423  
"Invalid page_cache_size (%lu), supports only 4KB\n",
09cbfeaf1 Kirill 

Re: [PATCH 1/1] f2fs: fix validation of the block count in sanity_check_raw_super

2018-12-21 Thread Chao Yu
On 2018/12/22 3:28, Martin Blumenstingl wrote:
> Treat "block_count" from struct f2fs_super_block as 64-bit little endian
> value in sanity_check_raw_super() because struct f2fs_super_block
> declares "block_count" as "__le64".
> 
> This fixes a bug where the superblock validation fails on big endian
> devices with the following error:
>   F2FS-fs (sda1): Wrong segment_count / block_count (61439 > 0)
>   F2FS-fs (sda1): Can't find valid F2FS filesystem in 1th superblock
>   F2FS-fs (sda1): Wrong segment_count / block_count (61439 > 0)
>   F2FS-fs (sda1): Can't find valid F2FS filesystem in 2th superblock
> As result of this the partition cannot be mounted.
> 
> With this patch applied the superblock validation works fine and the
> partition can be mounted again:
>   F2FS-fs (sda1): Mounted with checkpoint version = 7c84
> 
> My little endian x86-64 hardware was able to mount the partition without
> this fix.
> To confirm that mounting f2fs filesystems works on big endian machines
> again I tested this on a 32-bit MIPS big endian (lantiq) device.
> 
> Fixes: 0cfe75c5b01199 ("f2fs: enhance sanity_check_raw_super() to avoid 
> potential overflows")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Martin Blumenstingl 

Reviewed-by: Chao Yu 

Thanks,



[PATCH 1/1] f2fs: fix validation of the block count in sanity_check_raw_super

2018-12-21 Thread Martin Blumenstingl
Treat "block_count" from struct f2fs_super_block as 64-bit little endian
value in sanity_check_raw_super() because struct f2fs_super_block
declares "block_count" as "__le64".

This fixes a bug where the superblock validation fails on big endian
devices with the following error:
  F2FS-fs (sda1): Wrong segment_count / block_count (61439 > 0)
  F2FS-fs (sda1): Can't find valid F2FS filesystem in 1th superblock
  F2FS-fs (sda1): Wrong segment_count / block_count (61439 > 0)
  F2FS-fs (sda1): Can't find valid F2FS filesystem in 2th superblock
As result of this the partition cannot be mounted.

With this patch applied the superblock validation works fine and the
partition can be mounted again:
  F2FS-fs (sda1): Mounted with checkpoint version = 7c84

My little endian x86-64 hardware was able to mount the partition without
this fix.
To confirm that mounting f2fs filesystems works on big endian machines
again I tested this on a 32-bit MIPS big endian (lantiq) device.

Fixes: 0cfe75c5b01199 ("f2fs: enhance sanity_check_raw_super() to avoid 
potential overflows")
Cc: sta...@vger.kernel.org
Signed-off-by: Martin Blumenstingl 
---
 fs/f2fs/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index af58b2cc21b8..3a65d5af122a 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2496,10 +2496,10 @@ static int sanity_check_raw_super(struct f2fs_sb_info 
*sbi,
return 1;
}
 
-   if (segment_count > (le32_to_cpu(raw_super->block_count) >> 9)) {
+   if (segment_count > (le64_to_cpu(raw_super->block_count) >> 9)) {
f2fs_msg(sb, KERN_INFO,
"Wrong segment_count / block_count (%u > %u)",
-   segment_count, le32_to_cpu(raw_super->block_count));
+   segment_count, le64_to_cpu(raw_super->block_count));
return 1;
}
 
-- 
2.20.1