[U-Boot] [PATCH v1] fat: fatwrite: fix the command for FAT12

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; }

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

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

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

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
participants (3)
-
Benoît Thébaudeau
-
Philipp Skadorov
-
Stefan Bruens