[PATCH v4 1/6] fat: write: fix broken write to fragmented files

[Matthias Brugger]

On 05/12/2019 17:52, Lukasz Majewski wrote:
> Hi Tom, Matthias,
> 
>> The code for handing file overwrite incorrectly assumed that the file
>> on disk is always contiguous. This resulted in corrupting disk
>> structure every time when write to existing fragmented file happened.
>> Fix this by adding proper check for cluster discontinuity and adjust
>> chunk size on each partial write.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
>> ---
>>
>> This patch partially fixes the issue revealed by the following test
>> script:
>>
> 
> Tom could you pic this patch and the following one (2/6):
> https://patchwork.ozlabs.org/patch/1203101/
> 
> to -master as a fix?
> 
> This seems like a _real_ fix for FAT.

Right, I think the first patches should go in for v2020.01. I can send them
together with some fixes for RPi I'm working on.

Tom what do you think?

Regards,
Matthias

> 
> The dfu part of this series IMHO shall be grabbed by Matthias or me into
> the -next branch of u-boot-dfu/rpi4 tree.
> 
> I do guess that Matthias shall fetch this series as he is assigned to
> it in the patchwork?
> I'm fine for this as I've already acked / reviewed DFU part of this
> series.
> 
>> --->8-fat_test1.sh---  
>> #!/bin/bash
>> make sandbox_defconfig
>> make
>> dd if=/dev/zero of=/tmp/10M.img bs=1024 count=10k
>> mkfs.vfat -v /tmp/10M.img
>> cat >/tmp/cmds <<EOF
>> x
>> host bind 0 /tmp/10M.img
>> fatls host 0
>> mw 0x1000000 0x0a434241 0x1000 # "ABC\n"
>> mw 0x1100000 0x0a464544 0x8000 # "DEF\n"
>> fatwrite host 0 0x1000000 file0001.raw 0x1000
>> fatwrite host 0 0x1000000 file0002.raw 0x1000
>> fatwrite host 0 0x1000000 file0003.raw 0x1000
>> fatwrite host 0 0x1000000 file0004.raw 0x1000
>> fatwrite host 0 0x1000000 file0005.raw 0x1000
>> fatrm host 0 file0002.raw
>> fatrm host 0 file0004.raw
>> fatls host 0
>> fatwrite host 0 0x1100000 file0007.raw 0x4000
>> fatwrite host 0 0x1100000 file0007.raw 0x4000
>> reset
>> EOF
>> ./u-boot </tmp/cmds
>> #verify
>> rm -r /tmp/result /tmp/model
>> mkdir /tmp/result
>> mkdir /tmp/model
>> yes ABC | head -c 4096 >/tmp/model/file0001.raw
>> yes ABC | head -c 4096 >/tmp/model/file0003.raw
>> yes ABC | head -c 4096 >/tmp/model/file0005.raw
>> yes DEF | head -c 16384 >/tmp/model/file0007.raw
>> mcopy -n -i /tmp/10M.img ::file0001.raw /tmp/result
>> mcopy -n -i /tmp/10M.img ::file0003.raw /tmp/result
>> mcopy -n -i /tmp/10M.img ::file0005.raw /tmp/result
>> mcopy -n -i /tmp/10M.img ::file0007.raw /tmp/result
>> hd /tmp/10M.img
>> if diff -urq /tmp/model /tmp/result
>> then
>> 	echo Test okay
>> else
>> 	echo Test fail
>> fi
>> --->8---  
>>
>> Overwritting a discontiguous test file (file0007.raw) no longer causes
>> corruption to file0003.raw, which's data lies between the chunks of
>> the test file. The amount of data written to disk is still incorrect,
>> what causes damage to the file (file0005.raw), which's data lies next
>> to the test file. This will be fixed by the next patch.
>>
>> Feel free to prepare a proper sandbox/py_test based tests based on the
>> provided test scripts.
>> ---
>>  fs/fat/fat_write.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
>> index 729cf39630..f946030f7d 100644
>> --- a/fs/fat/fat_write.c
>> +++ b/fs/fat/fat_write.c
>> @@ -794,6 +794,8 @@ set_contents(fsdata *mydata, dir_entry *dentptr,
>> loff_t pos, __u8 *buffer, 
>>  			newclust = get_fatent(mydata, endclust);
>>  
>> +			if (newclust != endclust + 1)
>> +				break;
>>  			if (IS_LAST_CLUST(newclust, mydata->fatsize))
>>  				break;
>>  			if (CHECK_CLUST(newclust, mydata->fatsize)) {
>> @@ -824,8 +826,6 @@ set_contents(fsdata *mydata, dir_entry *dentptr,
>> loff_t pos, __u8 *buffer, if (filesize <= cur_pos)
>>  			break;
>>  
>> -		/* CHECK: newclust = get_fatent(mydata, endclust); */
>> -
>>  		if (IS_LAST_CLUST(newclust, mydata->fatsize))
>>  			/* no more clusters */
>>  			break;
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20191205/3761086d/attachment.sig>