From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lukasz Majewski <lukma@denx.de>
Date: Thu, 5 Dec 2019 17:52:10 +0100
Subject: [PATCH v4 1/6] fat: write: fix broken write to fragmented files
In-Reply-To: <20191202111118.12868-2-m.szyprowski@samsung.com>
References: <20191202111118.12868-1-m.szyprowski@samsung.com>
 <CGME20191202111129eucas1p271b4ed0d90cb10addce5fefa0af576aa@eucas1p2.samsung.com>
 <20191202111118.12868-2-m.szyprowski@samsung.com>
Message-ID: <20191205175210.2fb30dbd@jawa>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@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.

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: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20191205/b2f0d7c5/attachment.sig>