fs: fat: validate sector and cluster size

PATCH New Heinrich Schuchardt 2 participants 6 messages Oct 31, 2018 – Nov 21, 2018 VFAT/FAT/msdos filesystem

Share & tools

Heinrich Schuchardt

author

2018-10-31 20:46 UTC #0 lore raw

PATCH 1/1 fs: fat: validate sector and cluster size

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 <[email protected]>

---
 fs/fat/fat.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

fs/fat/fat.c +34 -1

index b08949d3705..0f69fa4df78 100644
@@ -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",

Signature · 1 line

2.19.1

Tom Rini

2018-11-20 14:56 UTC #1 lore raw

PATCH 1/1 fs: fat: validate sector and cluster size

Quoted 5 lines · Heinrich Schuchardt

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 <[email protected]>

This breaks a number of tests under test/py/tests/test_fs/test_basic.py as seen under 'make tests'.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181120/de5d919a/attachment.sig>

Heinrich Schuchardt

author

2018-11-20 20:29 UTC #2 lore raw

PATCH 1/1 fs: fat: validate sector and cluster size

Quoted 11 lines · Tom Rini

Quoted 5 lines · Heinrich Schuchardt
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 <[email protected]>
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 at 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 at 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 at work:~/workspace/u-boot-build/denx$ git branch -D build Deleted branch build (was 08a1a289970).
user at work:~/workspace/u-boot-build/denx$ git checkout -b build Switched to a new branch 'build'
user at work:~/workspace/u-boot-build/denx$ git am ../patch/*-fat-*
Applying: fs: fat: validate sector and cluster size
user at 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

================================================================ 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 at work:~/workspace/u-boot-build/denx$

Tom Rini

2018-11-20 20:33 UTC #3 lore raw

PATCH 1/1 fs: fat: validate sector and cluster size

Quoted 18 lines · Heinrich Schuchardt

Quoted 11 lines · Tom Rini
Quoted 5 lines · Heinrich Schuchardt
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 <[email protected]>
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]

Quoted 2 lines

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.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181120/236291ee/attachment.sig>

Heinrich Schuchardt

author

2018-11-20 22:08 UTC #4 lore raw

PATCH 1/1 fs: fat: validate sector and cluster size

Quoted 20 lines · Tom Rini

Quoted 13 lines · Heinrich Schuchardt
Quoted 11 lines · Tom Rini
Quoted 5 lines · Heinrich Schuchardt
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 <[email protected]>
This breaks a number of tests under test/py/tests/test_fs/test_basic.py as seen under 'make tests'.
Quoted 5 lines
Hello Tom,
I am not able to reproduce your observation based on commit d73d81fd85e4a030ade42c4b2d13466d45090aa3 (master HEAD). See test output below.

Quoted 7 lines

[snip]
Quoted 2 lines
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

Tom Rini

2018-11-21 02:17 UTC #5 lore raw

PATCH 1/1 fs: fat: validate sector and cluster size

Quoted 30 lines · Heinrich Schuchardt

Quoted 20 lines · Tom Rini
Quoted 13 lines · Heinrich Schuchardt
Quoted 11 lines · Tom Rini
Quoted 5 lines · Heinrich Schuchardt
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 <[email protected]>
This breaks a number of tests under test/py/tests/test_fs/test_basic.py as seen under 'make tests'.
Quoted 5 lines
Hello Tom,
I am not able to reproduce your observation based on commit d73d81fd85e4a030ade42c4b2d13466d45090aa3 (master HEAD). See test output below.
Quoted 7 lines
[snip]
Quoted 2 lines
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

Quoted 4 lines

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.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181120/c997aaa8/attachment.sig>