public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12
@ 2016-12-09 18:55 Philipp Skadorov
  2016-12-10 23:21 ` Benoît Thébaudeau
  2016-12-10 23:29 ` Stefan Bruens
  0 siblings, 2 replies; 5+ messages in thread
From: Philipp Skadorov @ 2016-12-09 18:55 UTC (permalink / raw)
  To: u-boot

The u-boot command fatwrite empties FAT clusters from the beginning
till the end of the file.
Specifically for FAT12 it fails to detect the end of the file and goes
beyond the file bounds thus corrupting the file system.

The users normally workaround this by re-formatting the partition as
FAT16/FAT32, like here:
https://github.com/FEDEVEL/openrex-uboot-v2015.10/issues/1

The patch is to check file bounds by already-existing macro that
accounts for FAT12.
The command then works correctly for all types of FAT.

Signed-off-by: Philipp Skadorov <philipp.skadorov@savoirfairelinux.com>
Cc:Donggeun Kim <dg77.kim@samsung.com>
---
 fs/fat/fat_write.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 40a3860..e4f600e 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -670,16 +670,13 @@ static int clear_fatent(fsdata *mydata, __u32 entry)
 {
 	__u32 fat_val;
 
-	while (1) {
+	while (!CHECK_CLUST(entry, mydata->fatsize)) {
 		fat_val = get_fatent_value(mydata, entry);
 		if (fat_val != 0)
 			set_fatent_value(mydata, entry, 0);
 		else
 			break;
 
-		if (fat_val == 0xfffffff || fat_val == 0xffff)
-			break;
-
 		entry = fat_val;
 	}
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12
  2016-12-09 18:55 [U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12 Philipp Skadorov
@ 2016-12-10 23:21 ` Benoît Thébaudeau
  2016-12-10 23:29 ` Stefan Bruens
  1 sibling, 0 replies; 5+ messages in thread
From: Benoît Thébaudeau @ 2016-12-10 23:21 UTC (permalink / raw)
  To: u-boot

Dear Philipp Skadorov,

On Fri, Dec 9, 2016 at 7:55 PM, Philipp Skadorov
<philipp.skadorov@savoirfairelinux.com> wrote:
> The u-boot command fatwrite empties FAT clusters from the beginning
> till the end of the file.
> Specifically for FAT12 it fails to detect the end of the file and goes
> beyond the file bounds thus corrupting the file system.
>
> The users normally workaround this by re-formatting the partition as
> FAT16/FAT32, like here:
> https://github.com/FEDEVEL/openrex-uboot-v2015.10/issues/1
>
> The patch is to check file bounds by already-existing macro that
> accounts for FAT12.
> The command then works correctly for all types of FAT.
>
> Signed-off-by: Philipp Skadorov <philipp.skadorov@savoirfairelinux.com>
> Cc:Donggeun Kim <dg77.kim@samsung.com>

[...]

Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12
  2016-12-09 18:55 [U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12 Philipp Skadorov
  2016-12-10 23:21 ` Benoît Thébaudeau
@ 2016-12-10 23:29 ` Stefan Bruens
  2016-12-11 15:05   ` Benoît Thébaudeau
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Bruens @ 2016-12-10 23:29 UTC (permalink / raw)
  To: u-boot

On Freitag, 9. Dezember 2016 13:55:37 CET Philipp Skadorov wrote:
> The u-boot command fatwrite empties FAT clusters from the beginning
> till the end of the file.
> Specifically for FAT12 it fails to detect the end of the file and goes
> beyond the file bounds thus corrupting the file system.
> 
> The users normally workaround this by re-formatting the partition as
> FAT16/FAT32, like here:
> https://github.com/FEDEVEL/openrex-uboot-v2015.10/issues/1
> 
> The patch is to check file bounds by already-existing macro that
> accounts for FAT12.
> The command then works correctly for all types of FAT.
> 
> Signed-off-by: Philipp Skadorov <philipp.skadorov@savoirfairelinux.com>
> Cc:Donggeun Kim <dg77.kim@samsung.com>
> ---
>  fs/fat/fat_write.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 40a3860..e4f600e 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -670,16 +670,13 @@ static int clear_fatent(fsdata *mydata, __u32 entry)
>  {
>  	__u32 fat_val;
> 
> -	while (1) {
> +	while (!CHECK_CLUST(entry, mydata->fatsize)) {
>  		fat_val = get_fatent_value(mydata, entry);
>  		if (fat_val != 0)
>  			set_fatent_value(mydata, entry, 0);
>  		else
>  			break;
> 
> -		if (fat_val == 0xfffffff || fat_val == 0xffff)
> -			break;
> -
>  		entry = fat_val;
>  	}

NAK.

This corrupts the file system, as set_fatent_value(...) has:

	switch (mydata->fatsize) {
	case 32:
		bufnum = entry / FAT32BUFSIZE;
		offset = entry - bufnum * FAT32BUFSIZE;
		break;
	case 16:
		bufnum = entry / FAT16BUFSIZE;
		offset = entry - bufnum * FAT16BUFSIZE;
		break;
	default:
		/* Unsupported FAT size */
		return -1;
	}

Kind regards,

Stefan

-- 
Stefan Br?ns  /  Bergstra?e 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
work: +49 2405 49936-424

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12
  2016-12-10 23:29 ` Stefan Bruens
@ 2016-12-11 15:05   ` Benoît Thébaudeau
  2016-12-11 15:39     ` Philipp Skadorov
  0 siblings, 1 reply; 5+ messages in thread
From: Benoît Thébaudeau @ 2016-12-11 15:05 UTC (permalink / raw)
  To: u-boot

Dear Stefan Br?ns,

On Sun, Dec 11, 2016 at 12:29 AM, Stefan Bruens
<stefan.bruens@rwth-aachen.de> wrote:
> On Freitag, 9. Dezember 2016 13:55:37 CET Philipp Skadorov wrote:
>> The u-boot command fatwrite empties FAT clusters from the beginning
>> till the end of the file.
>> Specifically for FAT12 it fails to detect the end of the file and goes
>> beyond the file bounds thus corrupting the file system.
>>
>> The users normally workaround this by re-formatting the partition as
>> FAT16/FAT32, like here:
>> https://github.com/FEDEVEL/openrex-uboot-v2015.10/issues/1
>>
>> The patch is to check file bounds by already-existing macro that
>> accounts for FAT12.
>> The command then works correctly for all types of FAT.
>>
>> Signed-off-by: Philipp Skadorov <philipp.skadorov@savoirfairelinux.com>
>> Cc:Donggeun Kim <dg77.kim@samsung.com>
>> ---
>>  fs/fat/fat_write.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
>> index 40a3860..e4f600e 100644
>> --- a/fs/fat/fat_write.c
>> +++ b/fs/fat/fat_write.c
>> @@ -670,16 +670,13 @@ static int clear_fatent(fsdata *mydata, __u32 entry)
>>  {
>>       __u32 fat_val;
>>
>> -     while (1) {
>> +     while (!CHECK_CLUST(entry, mydata->fatsize)) {
>>               fat_val = get_fatent_value(mydata, entry);
>>               if (fat_val != 0)
>>                       set_fatent_value(mydata, entry, 0);
>>               else
>>                       break;
>>
>> -             if (fat_val == 0xfffffff || fat_val == 0xffff)
>> -                     break;
>> -
>>               entry = fat_val;
>>       }
>
> NAK.
>
> This corrupts the file system, as set_fatent_value(...) has:
>
>         switch (mydata->fatsize) {
>         case 32:
>                 bufnum = entry / FAT32BUFSIZE;
>                 offset = entry - bufnum * FAT32BUFSIZE;
>                 break;
>         case 16:
>                 bufnum = entry / FAT16BUFSIZE;
>                 offset = entry - bufnum * FAT16BUFSIZE;
>                 break;
>         default:
>                 /* Unsupported FAT size */
>                 return -1;
>         }

So this patch can be kept, but it needs to be combined with a new one
in a series to fully fix fatwrite for FAT12.

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12
  2016-12-11 15:05   ` Benoît Thébaudeau
@ 2016-12-11 15:39     ` Philipp Skadorov
  0 siblings, 0 replies; 5+ messages in thread
From: Philipp Skadorov @ 2016-12-11 15:39 UTC (permalink / raw)
  To: u-boot

Good morning,
I will update and test set_fatent_value as well and will send you patch v.2

Regards,
Philipp

> On Dec 11, 2016, at 10:05 AM, Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com> wrote:
> 
> Dear Stefan Br?ns,
> 
> On Sun, Dec 11, 2016 at 12:29 AM, Stefan Bruens
> <stefan.bruens@rwth-aachen.de> wrote:
>> On Freitag, 9. Dezember 2016 13:55:37 CET Philipp Skadorov wrote:
>>> The u-boot command fatwrite empties FAT clusters from the beginning
>>> till the end of the file.
>>> Specifically for FAT12 it fails to detect the end of the file and goes
>>> beyond the file bounds thus corrupting the file system.
>>> 
>>> The users normally workaround this by re-formatting the partition as
>>> FAT16/FAT32, like here:
>>> https://github.com/FEDEVEL/openrex-uboot-v2015.10/issues/1
>>> 
>>> The patch is to check file bounds by already-existing macro that
>>> accounts for FAT12.
>>> The command then works correctly for all types of FAT.
>>> 
>>> Signed-off-by: Philipp Skadorov <philipp.skadorov@savoirfairelinux.com>
>>> Cc:Donggeun Kim <dg77.kim@samsung.com>
>>> ---
>>> fs/fat/fat_write.c | 5 +----
>>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>> 
>>> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
>>> index 40a3860..e4f600e 100644
>>> --- a/fs/fat/fat_write.c
>>> +++ b/fs/fat/fat_write.c
>>> @@ -670,16 +670,13 @@ static int clear_fatent(fsdata *mydata, __u32 entry)
>>> {
>>>      __u32 fat_val;
>>> 
>>> -     while (1) {
>>> +     while (!CHECK_CLUST(entry, mydata->fatsize)) {
>>>              fat_val = get_fatent_value(mydata, entry);
>>>              if (fat_val != 0)
>>>                      set_fatent_value(mydata, entry, 0);
>>>              else
>>>                      break;
>>> 
>>> -             if (fat_val == 0xfffffff || fat_val == 0xffff)
>>> -                     break;
>>> -
>>>              entry = fat_val;
>>>      }
>> 
>> NAK.
>> 
>> This corrupts the file system, as set_fatent_value(...) has:
>> 
>>        switch (mydata->fatsize) {
>>        case 32:
>>                bufnum = entry / FAT32BUFSIZE;
>>                offset = entry - bufnum * FAT32BUFSIZE;
>>                break;
>>        case 16:
>>                bufnum = entry / FAT16BUFSIZE;
>>                offset = entry - bufnum * FAT16BUFSIZE;
>>                break;
>>        default:
>>                /* Unsupported FAT size */
>>                return -1;
>>        }
> 
> So this patch can be kept, but it needs to be combined with a new one
> in a series to fully fix fatwrite for FAT12.
> 
> Best regards,
> Beno?t

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-12-11 15:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-09 18:55 [U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12 Philipp Skadorov
2016-12-10 23:21 ` Benoît Thébaudeau
2016-12-10 23:29 ` Stefan Bruens
2016-12-11 15:05   ` Benoît Thébaudeau
2016-12-11 15:39     ` Philipp Skadorov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox