| Message ID | 20181031204636.15149-1-xypron.glpk@gmx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,1/1] fs: fat: validate sector and cluster size | expand |
On Wed, Oct 31, 2018 at 09:46:36PM +0100, Heinrich Schuchardt wrote: > In the rest of the FAT driver we want to be able to rely on the fact that > the cluster size is a power of two. So let's check that both the sector > size and the number of sectors per cluster are valid. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> This breaks a number of tests under test/py/tests/test_fs/test_basic.py as seen under 'make tests'.
On 11/20/18 3:56 PM, Tom Rini wrote: > On Wed, Oct 31, 2018 at 09:46:36PM +0100, Heinrich Schuchardt wrote: > >> In the rest of the FAT driver we want to be able to rely on the fact that >> the cluster size is a power of two. So let's check that both the sector >> size and the number of sectors per cluster are valid. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > This breaks a number of tests under test/py/tests/test_fs/test_basic.py > as seen under 'make tests'. > Hello Tom, I am not able to reproduce your observation based on commit d73d81fd85e4a030ade42c4b2d13466d45090aa3 (master HEAD). See test output below. Regards Heinrich user@work:~/workspace/u-boot-build/denx$ git checkout master Switched to branch 'master' Your branch is behind 'origin/master' by 207 commits, and can be fast-forwarded. (use "git pull" to update your local branch) user@LT02:~/workspace/u-boot-build/denx$ git rebase First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. user@work:~/workspace/u-boot-build/denx$ git branch -D build Deleted branch build (was 08a1a289970). user@work:~/workspace/u-boot-build/denx$ git checkout -b build Switched to a new branch 'build' user@work:~/workspace/u-boot-build/denx$ git am ../patch/*-fat-* Applying: fs: fat: validate sector and cluster size user@work:~/workspace/u-boot-build/denx$ make tests ./test/run sandbox: +make O=/home/user/workspace/u-boot-build/denx/build-sandbox -s sandbox_defconfig +make O=/home/user/workspace/u-boot-build/denx/build-sandbox -s -j8 =========================================================================== test session starts ============================================================================ platform linux2 -- Python 2.7.15+, pytest-3.6.4, py-1.7.0, pluggy-0.8.0 rootdir: /home/user/workspace/u-boot-build/denx/test/py, inifile: pytest.ini collected 440 items test/py/tests/test_000_version.py . [ 0%] test/py/tests/test_avb.py ss..s [ 0%] test/py/tests/test_bind.py .. [ 0%] test/py/tests/test_dfu.py s [ 0%] test/py/tests/test_efi_loader.py .sssss [ 0%] test/py/tests/test_efi_selftest.py sssss [ 0%] test/py/tests/test_env.py ............ [ 0%] test/py/tests/test_fit.py . [ 0%] test/py/tests/test_fpga.py sssssssssssssssssssssssssss [ 0%] test/py/tests/test_gpt.py sssssss [ 0%] test/py/tests/test_help.py . [ 0%] test/py/tests/test_hush_if_test.py ....................................................... [ 0%] test/py/tests/test_log.py .. [ 0%] test/py/tests/test_md.py .. [ 0%] test/py/tests/test_mmc_rd.py s [ 0%] test/py/tests/test_net.py .sssss [ 0%] test/py/tests/test_ofplatdata.py s [ 0%] test/py/tests/test_pinmux.py ....... [ 0%] test/py/tests/test_sandbox_exit.py .. [ 0%] test/py/tests/test_sf.py ssss [ 0%] test/py/tests/test_shell_basics.py .... [ 0%] test/py/tests/test_sleep.py . [ 0%] test/py/tests/test_tpm2.py ........... [ 0%] test/py/tests/test_ums.py s [ 0%] test/py/tests/test_unknown_cmd.py . [ 0%] test/py/tests/test_ut.py .............................................................................................................................................................................................. [ 0%] test/py/tests/test_vboot.py . [ 0%] test/py/tests/test_fs/test_basic.py sssssssssssssssssssssssssssssssssssssss [ 0%] test/py/tests/test_fs/test_ext.py ssssssssssssssssss [ 0%] test/py/tests/test_fs/test_mkdir.py ssssssssssss [ 0%] test/py/tests/test_fs/test_unlink.py ssssssssssssss ================================================================= 297 passed, 143 skipped in 37.78 seconds ================================================================= sandbox_spl: +make O=/home/user/workspace/u-boot-build/denx/build-sandbox_spl -s sandbox_spl_defconfig +make O=/home/user/workspace/u-boot-build/denx/build-sandbox_spl -s -j8 =========================================================================== test session starts ============================================================================ platform linux2 -- Python 2.7.15+, pytest-3.6.4, py-1.7.0, pluggy-0.8.0 rootdir: /home/user/workspace/u-boot-build/denx/test/py, inifile: pytest.ini collected 434 items / 433 deselected test/py/tests/test_ofplatdata.py . ================================================================= 1 passed, 433 deselected in 0.24 seconds ================================================================= sandbox_flattree: +make O=/home/user/workspace/u-boot-build/denx/build-sandbox_flattree -s sandbox_flattree_defconfig +make O=/home/user/workspace/u-boot-build/denx/build-sandbox_flattree -s -j8 =========================================================================== test session starts ============================================================================ platform linux2 -- Python 2.7.15+, pytest-3.6.4, py-1.7.0, pluggy-0.8.0 rootdir: /home/user/workspace/u-boot-build/denx/test/py, inifile: pytest.ini collected 435 items / 250 deselected test/py/tests/test_ut.py ......................................................................................................................................................................................... ================================================================ 185 passed, 250 deselected in 4.28 seconds ================================================================ binman: <unittest.result.TestResult run=139 errors=0 failures=0> patman: <unittest.result.TestResult run=12 errors=0 failures=0> buildman: <unittest.result.TestResult run=35 errors=0 failures=0> fdt: <unittest.result.TestResult run=34 errors=0 failures=0> dtoc: <unittest.result.TestResult run=22 errors=0 failures=0> binman code coverage: <unittest.result.TestResult run=139 errors=0 failures=0> 100% dtoc code coverage: <unittest.result.TestResult run=22 errors=0 failures=0> 100% fdt code coverage: <unittest.result.TestResult run=34 errors=0 failures=0> 100% Tests passed! user@work:~/workspace/u-boot-build/denx$
On Tue, Nov 20, 2018 at 09:29:34PM +0100, Heinrich Schuchardt wrote: > On 11/20/18 3:56 PM, Tom Rini wrote: > > On Wed, Oct 31, 2018 at 09:46:36PM +0100, Heinrich Schuchardt wrote: > > > >> In the rest of the FAT driver we want to be able to rely on the fact that > >> the cluster size is a power of two. So let's check that both the sector > >> size and the number of sectors per cluster are valid. > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > > > This breaks a number of tests under test/py/tests/test_fs/test_basic.py > > as seen under 'make tests'. > > > > Hello Tom, > > I am not able to reproduce your observation based on commit > d73d81fd85e4a030ade42c4b2d13466d45090aa3 (master HEAD). See test output > below. [snip] > test/py/tests/test_fs/test_basic.py > sssssssssssssssssssssssssssssssssssssss OK, my fault for being imprecise. These tests here are the ones that fail and you have them being skipped.
On 11/20/18 9:33 PM, Tom Rini wrote: > On Tue, Nov 20, 2018 at 09:29:34PM +0100, Heinrich Schuchardt wrote: > >> On 11/20/18 3:56 PM, Tom Rini wrote: >>> On Wed, Oct 31, 2018 at 09:46:36PM +0100, Heinrich Schuchardt wrote: >>> >>>> In the rest of the FAT driver we want to be able to rely on the fact that >>>> the cluster size is a power of two. So let's check that both the sector >>>> size and the number of sectors per cluster are valid. >>>> >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >>> >>> This breaks a number of tests under test/py/tests/test_fs/test_basic.py >>> as seen under 'make tests'. >>> >> >> Hello Tom, >> >> I am not able to reproduce your observation based on commit >> d73d81fd85e4a030ade42c4b2d13466d45090aa3 (master HEAD). See test output >> below. > [snip] >> test/py/tests/test_fs/test_basic.py >> sssssssssssssssssssssssssssssssssssssss > > OK, my fault for being imprecise. These tests here are the ones that > fail and you have them being skipped. > r make tests is building sandbox. Why are the tests skipped? I did not set them to skipped explicitly. The tests make reference to a file system. Where is it built? Regards Heinrich
On Tue, Nov 20, 2018 at 11:08:54PM +0100, Heinrich Schuchardt wrote: > On 11/20/18 9:33 PM, Tom Rini wrote: > > On Tue, Nov 20, 2018 at 09:29:34PM +0100, Heinrich Schuchardt wrote: > > > >> On 11/20/18 3:56 PM, Tom Rini wrote: > >>> On Wed, Oct 31, 2018 at 09:46:36PM +0100, Heinrich Schuchardt wrote: > >>> > >>>> In the rest of the FAT driver we want to be able to rely on the fact that > >>>> the cluster size is a power of two. So let's check that both the sector > >>>> size and the number of sectors per cluster are valid. > >>>> > >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >>> > >>> This breaks a number of tests under test/py/tests/test_fs/test_basic.py > >>> as seen under 'make tests'. > >>> > >> > >> Hello Tom, > >> > >> I am not able to reproduce your observation based on commit > >> d73d81fd85e4a030ade42c4b2d13466d45090aa3 (master HEAD). See test output > >> below. > > [snip] > >> test/py/tests/test_fs/test_basic.py > >> sssssssssssssssssssssssssssssssssssssss > > > > OK, my fault for being imprecise. These tests here are the ones that > > fail and you have them being skipped. > > r > > > make tests is building sandbox. Why are the tests skipped? I did not set > them to skipped explicitly. > > The tests make reference to a file system. Where is it built? I suspect that since it needs sudo to run, you don't have that allowed and thus it goes with "skip" not running and pass/failing.
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index b08949d3705..0f69fa4df78 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -547,6 +547,39 @@ static int get_fs_info(fsdata *mydata) return ret; } + /* Validate sector and cluster size */ + switch (bs.sector_size[1]) { + case 2: /* 512 bytes per sector */ + case 4: /* 1024 bytes per sector */ + case 8: /* 2048 bytes per sector */ + case 16: /* 4096 bytes per sector */ + break; + default: + ret = -1; + } + if (ret || bs.sector_size[0]) { + debug("FAT: invalid sector size\n"); + return -1; + } + switch (bs.cluster_size) { + case 1: + case 2: + case 4: + case 8: + case 16: + case 32: + case 64: + case 128: + break; + default: + ret = -1; + } + /* Check bytes per cluster <= 32768 */ + if (ret || (u32)bs.sector_size[1] * (u32)bs.cluster_size > 128) { + debug("FAT: invalid cluster size\n"); + return -1; + } + if (mydata->fatsize == 32) { mydata->fatlength = bs.fat32_length; mydata->total_sect = bs.total_sect; @@ -564,7 +597,7 @@ static int get_fs_info(fsdata *mydata) mydata->rootdir_sect = mydata->fat_sect + mydata->fatlength * bs.fats; - mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0]; + mydata->sect_size = bs.sector_size[1] << 8; mydata->clust_size = bs.cluster_size; if (mydata->sect_size != cur_part_info.blksz) { printf("Error: FAT sector size mismatch (fs=%hu, dev=%lu)\n",
In the rest of the FAT driver we want to be able to rely on the fact that the cluster size is a power of two. So let's check that both the sector size and the number of sectors per cluster are valid. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- fs/fat/fat.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)