[U-Boot,1/3] fat: write: fix broken write to fragmented files
diff mbox series

Message ID 20190916122417.27520-2-m.szyprowski@samsung.com
State Superseded
Delegated to: Tom Rini
Headers show
Series
  • Raspberry Pi4: add support for DFU over USB
Related show

Commit Message

Marek Szyprowski Sept. 16, 2019, 12:24 p.m. UTC
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>
---
 fs/fat/fat_write.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Oleksandr Suvorov Sept. 16, 2019, 1:18 p.m. UTC | #1
On Mon, Sep 16, 2019 at 3:25 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> 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>

Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>

> ---
>  fs/fat/fat_write.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 729cf39630d..6cfa5b45652 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 - 1) != endclust)
> +                               break;
>                         if (IS_LAST_CLUST(newclust, mydata->fatsize))
>                                 break;
>                         if (CHECK_CLUST(newclust, mydata->fatsize)) {
> @@ -811,7 +813,7 @@ set_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer,
>                         offset = 0;
>                 else
>                         offset = pos - cur_pos;
> -               wsize = min(cur_pos + actsize, filesize) - pos;
> +               wsize = min_t(unsigned long long, actsize, filesize - cur_pos);
>                 if (get_set_cluster(mydata, curclust, offset,
>                                     buffer, wsize, &actsize)) {
>                         printf("Error get-and-setting cluster\n");
> @@ -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;
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Lukasz Majewski Sept. 16, 2019, 9:38 p.m. UTC | #2
Hi Marek,

> 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>
> ---
>  fs/fat/fat_write.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 729cf39630d..6cfa5b45652 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 - 1) != endclust)
> +				break;
>  			if (IS_LAST_CLUST(newclust, mydata->fatsize))
>  				break;
>  			if (CHECK_CLUST(newclust, mydata->fatsize)) {
> @@ -811,7 +813,7 @@ set_contents(fsdata *mydata, dir_entry *dentptr,
> loff_t pos, __u8 *buffer, offset = 0;
>  		else
>  			offset = pos - cur_pos;
> -		wsize = min(cur_pos + actsize, filesize) - pos;
> +		wsize = min_t(unsigned long long, actsize, filesize
> - cur_pos); if (get_set_cluster(mydata, curclust, offset,
>  				    buffer, wsize, &actsize)) {
>  			printf("Error get-and-setting cluster\n");
> @@ -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;

Reviewed-by: Lukasz Majewski <lukma@denx.de>

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

Patch
diff mbox series

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 729cf39630d..6cfa5b45652 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 - 1) != endclust)
+				break;
 			if (IS_LAST_CLUST(newclust, mydata->fatsize))
 				break;
 			if (CHECK_CLUST(newclust, mydata->fatsize)) {
@@ -811,7 +813,7 @@  set_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer,
 			offset = 0;
 		else
 			offset = pos - cur_pos;
-		wsize = min(cur_pos + actsize, filesize) - pos;
+		wsize = min_t(unsigned long long, actsize, filesize - cur_pos);
 		if (get_set_cluster(mydata, curclust, offset,
 				    buffer, wsize, &actsize)) {
 			printf("Error get-and-setting cluster\n");
@@ -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;