* [PATCH 0/3] fs/ntfs3: Refactoring of super.c
@ 2021-09-27 15:46 Konstantin Komarov
2021-09-27 15:47 ` [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed Konstantin Komarov
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Konstantin Komarov @ 2021-09-27 15:46 UTC (permalink / raw)
To: ntfs3; +Cc: linux-kernel, linux-fsdevel
Fix memory leak in ntfs_discard.
Reject mount so we won't corrupt fs.
Refactor ntfs_init_from_boot function.
Konstantin Komarov (3):
fs/ntfs3: Fix memory leak if fill_super failed
fs/ntfs3: Reject mount if boot's cluster size < media sector size
fs/ntfs3: Refactoring of ntfs_init_from_boot
fs/ntfs3/ntfs_fs.h | 2 --
fs/ntfs3/super.c | 36 +++++++++++++++++++++++++-----------
2 files changed, 25 insertions(+), 13 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed 2021-09-27 15:46 [PATCH 0/3] fs/ntfs3: Refactoring of super.c Konstantin Komarov @ 2021-09-27 15:47 ` Konstantin Komarov 2021-09-27 18:40 ` Kari Argillander ` (2 more replies) 2021-09-27 15:48 ` [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size < media sector size Konstantin Komarov 2021-09-27 15:48 ` [PATCH 3/3] fs/ntfs3: Refactoring of ntfs_init_from_boot Konstantin Komarov 2 siblings, 3 replies; 13+ messages in thread From: Konstantin Komarov @ 2021-09-27 15:47 UTC (permalink / raw) To: ntfs3; +Cc: linux-kernel, linux-fsdevel Restore fc->s_fs_info to free memory allocated in ntfs_init_fs_context. Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> --- fs/ntfs3/super.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c index 800897777eb0..7099d9b1f3aa 100644 --- a/fs/ntfs3/super.c +++ b/fs/ntfs3/super.c @@ -1308,6 +1308,9 @@ int ntfs_discard(struct ntfs_sb_info *sbi, CLST lcn, CLST len) if (err == -EOPNOTSUPP) sbi->flags |= NTFS_FLAGS_NODISCARD; + /* Restore fc->s_fs_info to free memory allocated in ntfs_init_fs_context. */ + fc->s_fs_info = sbi; + return err; } -- 2.33.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed 2021-09-27 15:47 ` [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed Konstantin Komarov @ 2021-09-27 18:40 ` Kari Argillander 2021-10-01 3:56 ` kernel test robot 2021-11-05 23:48 ` kernel test robot 2 siblings, 0 replies; 13+ messages in thread From: Kari Argillander @ 2021-09-27 18:40 UTC (permalink / raw) To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, linux-fsdevel On Mon, Sep 27, 2021 at 06:47:14PM +0300, Konstantin Komarov wrote: > Restore fc->s_fs_info to free memory allocated in ntfs_init_fs_context. > > Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> > --- > fs/ntfs3/super.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c > index 800897777eb0..7099d9b1f3aa 100644 > --- a/fs/ntfs3/super.c > +++ b/fs/ntfs3/super.c > @@ -1308,6 +1308,9 @@ int ntfs_discard(struct ntfs_sb_info *sbi, CLST lcn, CLST len) > if (err == -EOPNOTSUPP) > sbi->flags |= NTFS_FLAGS_NODISCARD; > > + /* Restore fc->s_fs_info to free memory allocated in ntfs_init_fs_context. */ > + fc->s_fs_info = sbi; Won't build and I do not understand what this does in ntfs_discard. > + > return err; > } > > -- > 2.33.0 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed 2021-09-27 15:47 ` [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed Konstantin Komarov @ 2021-10-01 3:56 ` kernel test robot 2021-10-01 3:56 ` kernel test robot 2021-11-05 23:48 ` kernel test robot 2 siblings, 0 replies; 13+ messages in thread From: kernel test robot @ 2021-10-01 3:56 UTC (permalink / raw) To: Konstantin Komarov, ntfs3; +Cc: kbuild-all, linux-kernel, linux-fsdevel [-- Attachment #1: Type: text/plain, Size: 3349 bytes --] Hi Konstantin, I love your patch! Yet something to improve: [auto build test ERROR on linux/master] [also build test ERROR on linus/master v5.15-rc3 next-20210921] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Konstantin-Komarov/fs-ntfs3-Refactoring-of-super-c/20210929-210855 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5816b3e6577eaa676ceb00a848f0fd65fe2adc29 config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/82c276fadc515e07a29db5855fd04a0c5e5f2358 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Konstantin-Komarov/fs-ntfs3-Refactoring-of-super-c/20210929-210855 git checkout 82c276fadc515e07a29db5855fd04a0c5e5f2358 # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): fs/ntfs3/super.c: In function 'ntfs_discard': >> fs/ntfs3/super.c:1429:9: error: 'fc' undeclared (first use in this function); did you mean 'fd'? 1429 | fc->s_fs_info = sbi; | ^~ | fd fs/ntfs3/super.c:1429:9: note: each undeclared identifier is reported only once for each function it appears in vim +1429 fs/ntfs3/super.c 1390 1391 /* 1392 * ntfs_discard - Issue a discard request (trim for SSD). 1393 */ 1394 int ntfs_discard(struct ntfs_sb_info *sbi, CLST lcn, CLST len) 1395 { 1396 int err; 1397 u64 lbo, bytes, start, end; 1398 struct super_block *sb; 1399 1400 if (sbi->used.next_free_lcn == lcn + len) 1401 sbi->used.next_free_lcn = lcn; 1402 1403 if (sbi->flags & NTFS_FLAGS_NODISCARD) 1404 return -EOPNOTSUPP; 1405 1406 if (!sbi->options.discard) 1407 return -EOPNOTSUPP; 1408 1409 lbo = (u64)lcn << sbi->cluster_bits; 1410 bytes = (u64)len << sbi->cluster_bits; 1411 1412 /* Align up 'start' on discard_granularity. */ 1413 start = (lbo + sbi->discard_granularity - 1) & 1414 sbi->discard_granularity_mask_inv; 1415 /* Align down 'end' on discard_granularity. */ 1416 end = (lbo + bytes) & sbi->discard_granularity_mask_inv; 1417 1418 sb = sbi->sb; 1419 if (start >= end) 1420 return 0; 1421 1422 err = blkdev_issue_discard(sb->s_bdev, start >> 9, (end - start) >> 9, 1423 GFP_NOFS, 0); 1424 1425 if (err == -EOPNOTSUPP) 1426 sbi->flags |= NTFS_FLAGS_NODISCARD; 1427 1428 /* Restore fc->s_fs_info to free memory allocated in ntfs_init_fs_context. */ > 1429 fc->s_fs_info = sbi; 1430 1431 return err; 1432 } 1433 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 68942 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed @ 2021-10-01 3:56 ` kernel test robot 0 siblings, 0 replies; 13+ messages in thread From: kernel test robot @ 2021-10-01 3:56 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 3439 bytes --] Hi Konstantin, I love your patch! Yet something to improve: [auto build test ERROR on linux/master] [also build test ERROR on linus/master v5.15-rc3 next-20210921] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Konstantin-Komarov/fs-ntfs3-Refactoring-of-super-c/20210929-210855 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5816b3e6577eaa676ceb00a848f0fd65fe2adc29 config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/82c276fadc515e07a29db5855fd04a0c5e5f2358 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Konstantin-Komarov/fs-ntfs3-Refactoring-of-super-c/20210929-210855 git checkout 82c276fadc515e07a29db5855fd04a0c5e5f2358 # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): fs/ntfs3/super.c: In function 'ntfs_discard': >> fs/ntfs3/super.c:1429:9: error: 'fc' undeclared (first use in this function); did you mean 'fd'? 1429 | fc->s_fs_info = sbi; | ^~ | fd fs/ntfs3/super.c:1429:9: note: each undeclared identifier is reported only once for each function it appears in vim +1429 fs/ntfs3/super.c 1390 1391 /* 1392 * ntfs_discard - Issue a discard request (trim for SSD). 1393 */ 1394 int ntfs_discard(struct ntfs_sb_info *sbi, CLST lcn, CLST len) 1395 { 1396 int err; 1397 u64 lbo, bytes, start, end; 1398 struct super_block *sb; 1399 1400 if (sbi->used.next_free_lcn == lcn + len) 1401 sbi->used.next_free_lcn = lcn; 1402 1403 if (sbi->flags & NTFS_FLAGS_NODISCARD) 1404 return -EOPNOTSUPP; 1405 1406 if (!sbi->options.discard) 1407 return -EOPNOTSUPP; 1408 1409 lbo = (u64)lcn << sbi->cluster_bits; 1410 bytes = (u64)len << sbi->cluster_bits; 1411 1412 /* Align up 'start' on discard_granularity. */ 1413 start = (lbo + sbi->discard_granularity - 1) & 1414 sbi->discard_granularity_mask_inv; 1415 /* Align down 'end' on discard_granularity. */ 1416 end = (lbo + bytes) & sbi->discard_granularity_mask_inv; 1417 1418 sb = sbi->sb; 1419 if (start >= end) 1420 return 0; 1421 1422 err = blkdev_issue_discard(sb->s_bdev, start >> 9, (end - start) >> 9, 1423 GFP_NOFS, 0); 1424 1425 if (err == -EOPNOTSUPP) 1426 sbi->flags |= NTFS_FLAGS_NODISCARD; 1427 1428 /* Restore fc->s_fs_info to free memory allocated in ntfs_init_fs_context. */ > 1429 fc->s_fs_info = sbi; 1430 1431 return err; 1432 } 1433 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 68942 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed 2021-09-27 15:47 ` [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed Konstantin Komarov @ 2021-11-05 23:48 ` kernel test robot 2021-10-01 3:56 ` kernel test robot 2021-11-05 23:48 ` kernel test robot 2 siblings, 0 replies; 13+ messages in thread From: kernel test robot @ 2021-11-05 23:48 UTC (permalink / raw) To: Konstantin Komarov; +Cc: llvm, kbuild-all [-- Attachment #1: Type: text/plain, Size: 3216 bytes --] Hi Konstantin, I love your patch! Yet something to improve: [auto build test ERROR on linux/master] [also build test ERROR on linus/master v5.15 next-20211105] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Konstantin-Komarov/fs-ntfs3-Refactoring-of-super-c/20210929-210855 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5816b3e6577eaa676ceb00a848f0fd65fe2adc29 config: i386-randconfig-a014-20210929 (attached as .config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project dc6e8dfdfe7efecfda318d43a06fae18b40eb498) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/82c276fadc515e07a29db5855fd04a0c5e5f2358 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Konstantin-Komarov/fs-ntfs3-Refactoring-of-super-c/20210929-210855 git checkout 82c276fadc515e07a29db5855fd04a0c5e5f2358 # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> fs/ntfs3/super.c:1429:2: error: use of undeclared identifier 'fc' fc->s_fs_info = sbi; ^ 1 error generated. vim +/fc +1429 fs/ntfs3/super.c 1390 1391 /* 1392 * ntfs_discard - Issue a discard request (trim for SSD). 1393 */ 1394 int ntfs_discard(struct ntfs_sb_info *sbi, CLST lcn, CLST len) 1395 { 1396 int err; 1397 u64 lbo, bytes, start, end; 1398 struct super_block *sb; 1399 1400 if (sbi->used.next_free_lcn == lcn + len) 1401 sbi->used.next_free_lcn = lcn; 1402 1403 if (sbi->flags & NTFS_FLAGS_NODISCARD) 1404 return -EOPNOTSUPP; 1405 1406 if (!sbi->options.discard) 1407 return -EOPNOTSUPP; 1408 1409 lbo = (u64)lcn << sbi->cluster_bits; 1410 bytes = (u64)len << sbi->cluster_bits; 1411 1412 /* Align up 'start' on discard_granularity. */ 1413 start = (lbo + sbi->discard_granularity - 1) & 1414 sbi->discard_granularity_mask_inv; 1415 /* Align down 'end' on discard_granularity. */ 1416 end = (lbo + bytes) & sbi->discard_granularity_mask_inv; 1417 1418 sb = sbi->sb; 1419 if (start >= end) 1420 return 0; 1421 1422 err = blkdev_issue_discard(sb->s_bdev, start >> 9, (end - start) >> 9, 1423 GFP_NOFS, 0); 1424 1425 if (err == -EOPNOTSUPP) 1426 sbi->flags |= NTFS_FLAGS_NODISCARD; 1427 1428 /* Restore fc->s_fs_info to free memory allocated in ntfs_init_fs_context. */ > 1429 fc->s_fs_info = sbi; 1430 1431 return err; 1432 } 1433 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 37248 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed @ 2021-11-05 23:48 ` kernel test robot 0 siblings, 0 replies; 13+ messages in thread From: kernel test robot @ 2021-11-05 23:48 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 3304 bytes --] Hi Konstantin, I love your patch! Yet something to improve: [auto build test ERROR on linux/master] [also build test ERROR on linus/master v5.15 next-20211105] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Konstantin-Komarov/fs-ntfs3-Refactoring-of-super-c/20210929-210855 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5816b3e6577eaa676ceb00a848f0fd65fe2adc29 config: i386-randconfig-a014-20210929 (attached as .config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project dc6e8dfdfe7efecfda318d43a06fae18b40eb498) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/82c276fadc515e07a29db5855fd04a0c5e5f2358 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Konstantin-Komarov/fs-ntfs3-Refactoring-of-super-c/20210929-210855 git checkout 82c276fadc515e07a29db5855fd04a0c5e5f2358 # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> fs/ntfs3/super.c:1429:2: error: use of undeclared identifier 'fc' fc->s_fs_info = sbi; ^ 1 error generated. vim +/fc +1429 fs/ntfs3/super.c 1390 1391 /* 1392 * ntfs_discard - Issue a discard request (trim for SSD). 1393 */ 1394 int ntfs_discard(struct ntfs_sb_info *sbi, CLST lcn, CLST len) 1395 { 1396 int err; 1397 u64 lbo, bytes, start, end; 1398 struct super_block *sb; 1399 1400 if (sbi->used.next_free_lcn == lcn + len) 1401 sbi->used.next_free_lcn = lcn; 1402 1403 if (sbi->flags & NTFS_FLAGS_NODISCARD) 1404 return -EOPNOTSUPP; 1405 1406 if (!sbi->options.discard) 1407 return -EOPNOTSUPP; 1408 1409 lbo = (u64)lcn << sbi->cluster_bits; 1410 bytes = (u64)len << sbi->cluster_bits; 1411 1412 /* Align up 'start' on discard_granularity. */ 1413 start = (lbo + sbi->discard_granularity - 1) & 1414 sbi->discard_granularity_mask_inv; 1415 /* Align down 'end' on discard_granularity. */ 1416 end = (lbo + bytes) & sbi->discard_granularity_mask_inv; 1417 1418 sb = sbi->sb; 1419 if (start >= end) 1420 return 0; 1421 1422 err = blkdev_issue_discard(sb->s_bdev, start >> 9, (end - start) >> 9, 1423 GFP_NOFS, 0); 1424 1425 if (err == -EOPNOTSUPP) 1426 sbi->flags |= NTFS_FLAGS_NODISCARD; 1427 1428 /* Restore fc->s_fs_info to free memory allocated in ntfs_init_fs_context. */ > 1429 fc->s_fs_info = sbi; 1430 1431 return err; 1432 } 1433 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 37248 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size < media sector size 2021-09-27 15:46 [PATCH 0/3] fs/ntfs3: Refactoring of super.c Konstantin Komarov 2021-09-27 15:47 ` [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed Konstantin Komarov @ 2021-09-27 15:48 ` Konstantin Komarov 2021-09-27 18:56 ` Kari Argillander 2021-09-27 15:48 ` [PATCH 3/3] fs/ntfs3: Refactoring of ntfs_init_from_boot Konstantin Komarov 2 siblings, 1 reply; 13+ messages in thread From: Konstantin Komarov @ 2021-09-27 15:48 UTC (permalink / raw) To: ntfs3; +Cc: linux-kernel, linux-fsdevel If we continue to work in this case, then we can corrupt fs. Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> --- fs/ntfs3/super.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c index 7099d9b1f3aa..193f9a98f6ab 100644 --- a/fs/ntfs3/super.c +++ b/fs/ntfs3/super.c @@ -763,9 +763,14 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, sbi->mft.lbo = mlcn << sbi->cluster_bits; sbi->mft.lbo2 = mlcn2 << sbi->cluster_bits; + /* Compare boot's cluster and sector. */ if (sbi->cluster_size < sbi->sector_size) goto out; + /* Compare boot's cluster and media sector. */ + if (sbi->cluster_size < sector_size) + goto out; /* No way to use ntfs_get_block in this case. */ + sbi->cluster_mask = sbi->cluster_size - 1; sbi->cluster_mask_inv = ~(u64)sbi->cluster_mask; sbi->record_size = record_size = boot->record_size < 0 -- 2.33.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size < media sector size 2021-09-27 15:48 ` [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size < media sector size Konstantin Komarov @ 2021-09-27 18:56 ` Kari Argillander 2021-09-28 17:21 ` Konstantin Komarov 0 siblings, 1 reply; 13+ messages in thread From: Kari Argillander @ 2021-09-27 18:56 UTC (permalink / raw) To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, linux-fsdevel On Mon, Sep 27, 2021 at 06:48:00PM +0300, Konstantin Komarov wrote: > If we continue to work in this case, then we can corrupt fs. > Should have fixes tag. > Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> > --- > fs/ntfs3/super.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c > index 7099d9b1f3aa..193f9a98f6ab 100644 > --- a/fs/ntfs3/super.c > +++ b/fs/ntfs3/super.c > @@ -763,9 +763,14 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, > sbi->mft.lbo = mlcn << sbi->cluster_bits; > sbi->mft.lbo2 = mlcn2 << sbi->cluster_bits; > > + /* Compare boot's cluster and sector. */ Pretty random obvious comment and I do not know what this does in this patch. > if (sbi->cluster_size < sbi->sector_size) > goto out; > > + /* Compare boot's cluster and media sector. */ > + if (sbi->cluster_size < sector_size) > + goto out; /* No way to use ntfs_get_block in this case. */ Usually comment should not go after line. If you take chunk from patch 3/3 then this is not issue. > + > sbi->cluster_mask = sbi->cluster_size - 1; > sbi->cluster_mask_inv = ~(u64)sbi->cluster_mask; > sbi->record_size = record_size = boot->record_size < 0 > -- > 2.33.0 > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size < media sector size 2021-09-27 18:56 ` Kari Argillander @ 2021-09-28 17:21 ` Konstantin Komarov 2021-09-28 17:58 ` Kari Argillander 0 siblings, 1 reply; 13+ messages in thread From: Konstantin Komarov @ 2021-09-28 17:21 UTC (permalink / raw) To: Kari Argillander; +Cc: ntfs3, linux-kernel, linux-fsdevel On 27.09.2021 21:56, Kari Argillander wrote: > On Mon, Sep 27, 2021 at 06:48:00PM +0300, Konstantin Komarov wrote: >> If we continue to work in this case, then we can corrupt fs. >> > > Should have fixes tag. > The bug is in initial commit. Do I need to write Fixes: 82cae269cfa95 "fs/ntfs3: Add initialization of super block" ? >> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> >> --- >> fs/ntfs3/super.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c >> index 7099d9b1f3aa..193f9a98f6ab 100644 >> --- a/fs/ntfs3/super.c >> +++ b/fs/ntfs3/super.c >> @@ -763,9 +763,14 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, >> sbi->mft.lbo = mlcn << sbi->cluster_bits; >> sbi->mft.lbo2 = mlcn2 << sbi->cluster_bits; >> >> + /* Compare boot's cluster and sector. */ > > Pretty random obvious comment and I do not know what this does in this > patch. > >> if (sbi->cluster_size < sbi->sector_size) >> goto out; >> >> + /* Compare boot's cluster and media sector. */ >> + if (sbi->cluster_size < sector_size) >> + goto out; /* No way to use ntfs_get_block in this case. */ > > Usually comment should not go after line. If you take chunk from patch > 3/3 then this is not issue. > >> + >> sbi->cluster_mask = sbi->cluster_size - 1; >> sbi->cluster_mask_inv = ~(u64)sbi->cluster_mask; >> sbi->record_size = record_size = boot->record_size < 0 >> -- >> 2.33.0 >> >> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size < media sector size 2021-09-28 17:21 ` Konstantin Komarov @ 2021-09-28 17:58 ` Kari Argillander 0 siblings, 0 replies; 13+ messages in thread From: Kari Argillander @ 2021-09-28 17:58 UTC (permalink / raw) To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, linux-fsdevel On Tue, Sep 28, 2021 at 08:21:54PM +0300, Konstantin Komarov wrote: > > > On 27.09.2021 21:56, Kari Argillander wrote: > > On Mon, Sep 27, 2021 at 06:48:00PM +0300, Konstantin Komarov wrote: > >> If we continue to work in this case, then we can corrupt fs. > >> > > > > Should have fixes tag. > > > > The bug is in initial commit. > Do I need to write > Fixes: 82cae269cfa95 "fs/ntfs3: Add initialization of super block" > ? Yes, but format is not right. It needs to be 12 letter sha and need brackets. Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block") Just add these to your gitconfig [core] abbrev = 12 [pretty] fixes = Fixes: %h (\"%s\") And after that you can use git show -s --pretty=fixes <sha> You can see also that others have used this same fixes line in commits: b8155e95de38 ("fs/ntfs3: Fix error handling in indx_insert_into_root()") 8c83a4851da1 ("fs/ntfs3: Potential NULL dereference in hdr_find_split()") 04810f000afd ("fs/ntfs3: Fix error code in indx_add_allocate()") 1263eddfea99 ("fs/ntfs3: Remove unused including <linux/version.h>") 8c01308b6d6b ("fs/ntfs3: Remove unused variable cnt in ntfs_security_init()") 71eeb6ace80b ("fs/ntfs3: Fix integer overflow in multiplication") The reason for fixes tag is that automatic tools can more easily cherry-pick things. Kernel stable branches example big these automatically, but there is also other companys which big these. Also it is sometimes nice to check commit which introduce this bug. Also some organization or study might use these for some statics. > > >> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> > >> --- > >> fs/ntfs3/super.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c > >> index 7099d9b1f3aa..193f9a98f6ab 100644 > >> --- a/fs/ntfs3/super.c > >> +++ b/fs/ntfs3/super.c > >> @@ -763,9 +763,14 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, > >> sbi->mft.lbo = mlcn << sbi->cluster_bits; > >> sbi->mft.lbo2 = mlcn2 << sbi->cluster_bits; > >> > >> + /* Compare boot's cluster and sector. */ > > > > Pretty random obvious comment and I do not know what this does in this > > patch. > > > >> if (sbi->cluster_size < sbi->sector_size) > >> goto out; > >> > >> + /* Compare boot's cluster and media sector. */ > >> + if (sbi->cluster_size < sector_size) > >> + goto out; /* No way to use ntfs_get_block in this case. */ > > > > Usually comment should not go after line. If you take chunk from patch > > 3/3 then this is not issue. > > > >> + > >> sbi->cluster_mask = sbi->cluster_size - 1; > >> sbi->cluster_mask_inv = ~(u64)sbi->cluster_mask; > >> sbi->record_size = record_size = boot->record_size < 0 > >> -- > >> 2.33.0 > >> > >> > >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] fs/ntfs3: Refactoring of ntfs_init_from_boot 2021-09-27 15:46 [PATCH 0/3] fs/ntfs3: Refactoring of super.c Konstantin Komarov 2021-09-27 15:47 ` [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed Konstantin Komarov 2021-09-27 15:48 ` [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size < media sector size Konstantin Komarov @ 2021-09-27 15:48 ` Konstantin Komarov 2021-09-27 18:52 ` Kari Argillander 2 siblings, 1 reply; 13+ messages in thread From: Konstantin Komarov @ 2021-09-27 15:48 UTC (permalink / raw) To: ntfs3; +Cc: linux-kernel, linux-fsdevel Remove ntfs_sb_info members sector_size and sector_bits. Print details why mount failed. Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> --- fs/ntfs3/ntfs_fs.h | 2 -- fs/ntfs3/super.c | 32 +++++++++++++++++++------------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h index 6731b5d9e2d8..38b7c1a9dc52 100644 --- a/fs/ntfs3/ntfs_fs.h +++ b/fs/ntfs3/ntfs_fs.h @@ -211,10 +211,8 @@ struct ntfs_sb_info { u32 blocks_per_cluster; // cluster_size / sb->s_blocksize u32 record_size; - u32 sector_size; u32 index_size; - u8 sector_bits; u8 cluster_bits; u8 record_bits; diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c index 193f9a98f6ab..5fe9484c6781 100644 --- a/fs/ntfs3/super.c +++ b/fs/ntfs3/super.c @@ -682,7 +682,7 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, struct ntfs_sb_info *sbi = sb->s_fs_info; int err; u32 mb, gb, boot_sector_size, sct_per_clst, record_size; - u64 sectors, clusters, fs_size, mlcn, mlcn2; + u64 sectors, clusters, mlcn, mlcn2; struct NTFS_BOOT *boot; struct buffer_head *bh; struct MFT_REC *rec; @@ -740,20 +740,20 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, goto out; } - sbi->sector_size = boot_sector_size; - sbi->sector_bits = blksize_bits(boot_sector_size); - fs_size = (sectors + 1) << sbi->sector_bits; + sbi->volume.size = sectors * boot_sector_size; - gb = format_size_gb(fs_size, &mb); + gb = format_size_gb(sbi->volume.size + boot_sector_size, &mb); /* * - Volume formatted and mounted with the same sector size. * - Volume formatted 4K and mounted as 512. * - Volume formatted 512 and mounted as 4K. */ - if (sbi->sector_size != sector_size) { - ntfs_warn(sb, - "Different NTFS' sector size and media sector size"); + if (boot_sector_size != sector_size) { + ntfs_warn( + sb, + "Different NTFS' sector size (%u) and media sector size (%u)", + boot_sector_size, sector_size); dev_size += sector_size - 1; } @@ -764,12 +764,19 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, sbi->mft.lbo2 = mlcn2 << sbi->cluster_bits; /* Compare boot's cluster and sector. */ - if (sbi->cluster_size < sbi->sector_size) + if (sbi->cluster_size < boot_sector_size) goto out; /* Compare boot's cluster and media sector. */ - if (sbi->cluster_size < sector_size) - goto out; /* No way to use ntfs_get_block in this case. */ + if (sbi->cluster_size < sector_size) { + /* No way to use ntfs_get_block in this case. */ + ntfs_err( + sb, + "Failed to mount 'cause NTFS's cluster size (%u) is " + "less than media sector size (%u)", + sbi->cluster_size, sector_size); + goto out; + } sbi->cluster_mask = sbi->cluster_size - 1; sbi->cluster_mask_inv = ~(u64)sbi->cluster_mask; @@ -794,10 +801,9 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, : (u32)boot->index_size << sbi->cluster_bits; sbi->volume.ser_num = le64_to_cpu(boot->serial_num); - sbi->volume.size = sectors << sbi->sector_bits; /* Warning if RAW volume. */ - if (dev_size < fs_size) { + if (dev_size < sbi->volume.size + boot_sector_size) { u32 mb0, gb0; gb0 = format_size_gb(dev_size, &mb0); -- 2.33.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] fs/ntfs3: Refactoring of ntfs_init_from_boot 2021-09-27 15:48 ` [PATCH 3/3] fs/ntfs3: Refactoring of ntfs_init_from_boot Konstantin Komarov @ 2021-09-27 18:52 ` Kari Argillander 0 siblings, 0 replies; 13+ messages in thread From: Kari Argillander @ 2021-09-27 18:52 UTC (permalink / raw) To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, linux-fsdevel On Mon, Sep 27, 2021 at 06:48:38PM +0300, Konstantin Komarov wrote: > Remove ntfs_sb_info members sector_size and sector_bits. > Print details why mount failed. > > Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> Like the changes. Some thing below. Reviewed-by: Kari Argillander <kari.argillander@gmail.com> > --- > fs/ntfs3/ntfs_fs.h | 2 -- > fs/ntfs3/super.c | 32 +++++++++++++++++++------------- > 2 files changed, 19 insertions(+), 15 deletions(-) > > diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h > index 6731b5d9e2d8..38b7c1a9dc52 100644 > --- a/fs/ntfs3/ntfs_fs.h > +++ b/fs/ntfs3/ntfs_fs.h > @@ -211,10 +211,8 @@ struct ntfs_sb_info { > u32 blocks_per_cluster; // cluster_size / sb->s_blocksize > > u32 record_size; > - u32 sector_size; > u32 index_size; > > - u8 sector_bits; > u8 cluster_bits; > u8 record_bits; > > diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c > index 193f9a98f6ab..5fe9484c6781 100644 > --- a/fs/ntfs3/super.c > +++ b/fs/ntfs3/super.c > @@ -682,7 +682,7 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, > struct ntfs_sb_info *sbi = sb->s_fs_info; > int err; > u32 mb, gb, boot_sector_size, sct_per_clst, record_size; > - u64 sectors, clusters, fs_size, mlcn, mlcn2; > + u64 sectors, clusters, mlcn, mlcn2; > struct NTFS_BOOT *boot; > struct buffer_head *bh; > struct MFT_REC *rec; > @@ -740,20 +740,20 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, > goto out; > } > > - sbi->sector_size = boot_sector_size; > - sbi->sector_bits = blksize_bits(boot_sector_size); > - fs_size = (sectors + 1) << sbi->sector_bits; > + sbi->volume.size = sectors * boot_sector_size; > > - gb = format_size_gb(fs_size, &mb); > + gb = format_size_gb(sbi->volume.size + boot_sector_size, &mb); > > /* > * - Volume formatted and mounted with the same sector size. > * - Volume formatted 4K and mounted as 512. > * - Volume formatted 512 and mounted as 4K. > */ > - if (sbi->sector_size != sector_size) { > - ntfs_warn(sb, > - "Different NTFS' sector size and media sector size"); > + if (boot_sector_size != sector_size) { > + ntfs_warn( > + sb, > + "Different NTFS' sector size (%u) and media sector size (%u)", > + boot_sector_size, sector_size); > dev_size += sector_size - 1; > } > > @@ -764,12 +764,19 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, > sbi->mft.lbo2 = mlcn2 << sbi->cluster_bits; > > /* Compare boot's cluster and sector. */ > - if (sbi->cluster_size < sbi->sector_size) > + if (sbi->cluster_size < boot_sector_size) > goto out; > > /* Compare boot's cluster and media sector. */ > - if (sbi->cluster_size < sector_size) > - goto out; /* No way to use ntfs_get_block in this case. */ > + if (sbi->cluster_size < sector_size) { > + /* No way to use ntfs_get_block in this case. */ > + ntfs_err( > + sb, > + "Failed to mount 'cause NTFS's cluster size (%u) is " > + "less than media sector size (%u)", This is first time I see splitted string like this in ntfs3. No need to split like this. Small nit for that this chunk should already could be in patch 2/3. > + sbi->cluster_size, sector_size); > + goto out; > + } > > sbi->cluster_mask = sbi->cluster_size - 1; > sbi->cluster_mask_inv = ~(u64)sbi->cluster_mask; > @@ -794,10 +801,9 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size, > : (u32)boot->index_size << sbi->cluster_bits; > > sbi->volume.ser_num = le64_to_cpu(boot->serial_num); > - sbi->volume.size = sectors << sbi->sector_bits; > > /* Warning if RAW volume. */ > - if (dev_size < fs_size) { > + if (dev_size < sbi->volume.size + boot_sector_size) { > u32 mb0, gb0; > > gb0 = format_size_gb(dev_size, &mb0); > -- > 2.33.0 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-11-05 23:49 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-27 15:46 [PATCH 0/3] fs/ntfs3: Refactoring of super.c Konstantin Komarov 2021-09-27 15:47 ` [PATCH 1/3] fs/ntfs3: Fix memory leak if fill_super failed Konstantin Komarov 2021-09-27 18:40 ` Kari Argillander 2021-10-01 3:56 ` kernel test robot 2021-10-01 3:56 ` kernel test robot 2021-11-05 23:48 ` kernel test robot 2021-11-05 23:48 ` kernel test robot 2021-09-27 15:48 ` [PATCH 2/3] fs/ntfs3: Reject mount if boot's cluster size < media sector size Konstantin Komarov 2021-09-27 18:56 ` Kari Argillander 2021-09-28 17:21 ` Konstantin Komarov 2021-09-28 17:58 ` Kari Argillander 2021-09-27 15:48 ` [PATCH 3/3] fs/ntfs3: Refactoring of ntfs_init_from_boot Konstantin Komarov 2021-09-27 18:52 ` Kari Argillander
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.