After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759916 - fat(32) resizing generates boot sector(s) with invalid jump instruction and pseudo-random boot code
fat(32) resizing generates boot sector(s) with invalid jump instruction and p...
Status: RESOLVED OBSOLETE
Product: gparted
Classification: Other
Component: application
0.24.0
Other Linux
: Normal normal
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2015-12-28 03:20 UTC by Tom Yan
Modified: 2020-11-13 10:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
hexdump of FAT32 from mkfs.fat (dosfstools) (3.26 KB, text/plain)
2015-12-28 03:27 UTC, Tom Yan
  Details
parted lib-fs-resize: Patch to fix recognition of FAT file system after resizing (v1) (3.59 KB, patch)
2016-04-10 20:05 UTC, Curtis Gedak
none Details | Review

Description Tom Yan 2015-12-28 03:20:45 UTC
https://gist.github.com/tomty89/49e16c0b38059a92260d/revisions?diff=split
https://gist.github.com/tomty89/8c26775247b277cda77d/revisions?diff=split

As you can see in the diff, the jump instruction is changed from "eb 58 90" (which is widely used/recognized) to "78 02 00". I don't know where comes the latter one, but libparted has the former in its code: http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/fs/r/fat/bootsector.h?id=1677dbbbbca3cd2aa94004cd09d64cbb8a451164#n37

Truth is Windows will not consider there is a filesystem if the first byte is not "eb". Maybe some other value is also accepted, but at least, not "78".

Although it doesn't seem to be required in Linux, mkfs.fat also uses "eb 58 90", and wipefs will wipe "eb" (but again, not "78").

Then the second problem is, the original boot code would be replaced by pseudo-random data comes from no where. You can compare the two diff and recogize this. I am fine with that the boot code will not be preserved, but it's very ugly that it insert irrational data which varies at every resize.

I am not sure if this is an upstream libparted issue. Thing is parted the program no longer uses any of the filesystem manipulation features, so I cannot test myself.
Comment 1 Tom Yan 2015-12-28 03:27:20 UTC
Created attachment 317945 [details]
hexdump of FAT32 from mkfs.fat (dosfstools)

Attached is a hexdump of FAT32 made by mkfs.fat in dosfstools for reference. You can see the jump instruction "eb 58 90" being used, and valid boot code that shows a message telling the disk is not bootable being insert, instead of pseudo-random data which simply hangs the boot process.
Comment 2 Tom Yan 2015-12-28 03:48:38 UTC
Actually it seems that the invalid jump instruction is (pseudo-random) as well:
https://gist.github.com/tomty89/2455439c556a8b33733b/revisions?diff=split
So I am quite sure that these are indeed bugs.
Comment 3 Tom Yan 2015-12-29 09:18:52 UTC
I think I've confirmed that it's a libparted issue. The last working commit I can test with is this:
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=d0a4cc1b57750a92afb48b229e4791154afa322b
It is the commit right before this:
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=80678bdd957cf49a9ccfc8b88ba3fb8b4c63fc12
which breaks FAT(32) resizing, and then upon this commit the introduced issue is "fixed":
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=5adae27101565a5d6fed4aadf28ddb39872e41f5
And the jump instruction and boot code is no longer preserved and weird things happen ever since.

So I'll report to upstream parted bug mailing list. I am not sure if the filesystem-related features are still maintained though, since they only exist in libparted but not parted any more. Would be nice if you guys can look into it and help I guess.
Comment 4 Tom Yan 2015-12-29 09:32:36 UTC
This show exactly how it is broken: https://gist.github.com/tomty89/2a1ca8698bad5ae969ad/revisions?diff=split
Comment 5 Mike Fleetwood 2015-12-29 10:35:45 UTC
Hi Tom,

Just to confirm what you have already worked out.  GParted uses
libparted from the parted project to resize FAT* file systems.

Thanks,
Mike
Comment 6 Curtis Gedak 2015-12-31 18:02:17 UTC
Thanks Tom for reporting this issue and submitting an upstream bug report to the parted team.

[libparted] jump instruction and boot code is corrupted with random bytes after fat is resized
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22266
Comment 7 Curtis Gedak 2016-04-07 19:58:03 UTC
Since I have not seen any further work on this issue, I decided to
delve into the problem myself.  Following is a description of my
work-to-date.


Determine Last Known Good Official Parted Release
-------------------------------------------------

Using the official release tarballs, I confirmed that libparted-3.1
correctly placed the jump instruction "eb 58 90" in the boot record.

  # With GParted/libparted-3.1 create 500 MiB FAT32 partition
  #   Note: for parted used "./configure --without-readline"

$ sudo hexdump -n 7k -C /dev/sda1
00000000  eb 58 90 6d 6b 66 73                              |.X.mkfs|
00000007

  # With GParted resize/shrink to 450 MiB FAT32 partition

$ sudo hexdump -n 7k -C /dev/sda1
00000000  eb 58 90 4d 53 57 49                              |.X.MSWI|
00000007
$

  # Note that jump instruction "eb 58 90" remains the same.

NOTE:  I also noticed that "mkfs" changed to "MSWI" which implies that
       the libparted code has this value hard-coded to be written when
       resizing FAT.  I tested with libparted-2.3 and the result was
       the same.


Searching for the problem
-------------------------

I tried using git bisect to locate the last known good commit.
Unfortunately I encountered numerous difficulties just getting parted
to compile cleanly on my lubuntu 14.04 VM.  I eventually gave up on
this path.

Due to the compile issue I endeavoured to read the code directly to
see if I could figure out why the jump instruction is not written when
fat32 is resized.


Possible problem location
--------------------------

After many hours of examination I believe I have discovered the cause
of the missing jump instruction.

The code that creates a new fat file system object is in:

  libparted/fs/r/fat/fat.c

in the function:

  PedFileSystem* fat_create (PedGeometry* geom, FatType fat_type, ...)

and specifically the following lines:

	if (!fat_boot_sector_set_boot_code (fs_info->boot_sector))
		goto error_free_buffers;
	if (!fat_boot_sector_generate (&fs_info->boot_sector, fs))
		goto error_free_buffers;
	if (!fat_boot_sector_write (fs_info->boot_sector, fs))
		goto error_free_buffers;
	if (fs_info->fat_type == FAT_TYPE_FAT32) {
		if (!fat_info_sector_generate (&fs_info->info_sector, fs))
			goto error_free_buffers;
		if (!fat_info_sector_write (fs_info->info_sector, fs))
			goto error_free_buffers;
	}

See
http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/fs/r/fat/fat.c#n313



The root problem appears to be that new boot sector code is set in

  fat_boot_sector_set_boot_code

and then AFTER that new memory is allocated for the boot sector in

  fat_boot_sector_generate

See the following code snippet from:
  libparted/fs/r/fat/bootsector.c



#ifndef DISCOVER_ONLY
int
fat_boot_sector_set_boot_code (FatBootSector* bs)
{
	PED_ASSERT (bs != NULL);

	memset (bs, 0, 512);
	memcpy (bs->boot_jump, FAT_BOOT_JUMP, 3);
	memcpy (bs->u.fat32.boot_code, FAT_BOOT_CODE, FAT_BOOT_CODE_LENGTH);
	return 1;
}

int
fat_boot_sector_generate (FatBootSector** bsp, const PedFileSystem* fs)
{
	FatSpecific*	fs_info = FAT_SPECIFIC (fs);

	PED_ASSERT (bsp != NULL);
	*bsp = ped_malloc (fs->geom->dev->sector_size);      # <- HERE!!
	FatBootSector *bs = *bsp;

	memcpy (bs->system_id, "MSWIN4.1", 8);
	bs->sector_size = PED_CPU_TO_LE16 (fs_info->logical_sector_size * 512);
	bs->cluster_size = fs_info->cluster_sectors
				/ fs_info->logical_sector_size;
	bs->reserved = PED_CPU_TO_LE16 (fs_info->fat_offset
					/ fs_info->logical_sector_size);
	bs->fats = fs_info->fat_table_count;

        <SNIP>....


Temporary Workaround
--------------------

If I comment out the ped_malloc call, then the issue of the missing
jump instruction goes away.  See line 300:

http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/fs/r/fat/bootsector.c#n300


Further Work Required
---------------------

More investigation is required because the
fat_boot_sector_set_boot_code appears to be hard coded for 512 byte
sectors.  I'm unsure of the implication of this at the current time.

Curtis
Comment 8 Curtis Gedak 2016-04-10 20:05:48 UTC
Created attachment 325683 [details] [review]
parted lib-fs-resize:  Patch to fix recognition of FAT file system after resizing (v1)

A patch to fix recognition of FAT file system after resizing with parted-3.2 libraries has been submitted to the upstream parted project (and also included in this report).

See the first reference link for where this problem is being tracked upstream in the parted project.

REFERENCES
----------

GNU bug report logs - #22266
[libparted] jump instruction and boot code is corrupted with random
bytes after fat is resized
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22266

GNU bug report logs - #22710
libparted 3.2 fat32 bootsector incompatible w. windows
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22710

GParted Bug Report:
Bug 759916 - fat(32) resizing generates boot sector(s) with invalid
             jump instruction and pseudo-random boot code
https://bugzilla.gnome.org/show_bug.cgi?id=759916

GParted Forum:
Vista/XP don't accept a shrinked Fat32 partition (USB-stick)
http://gparted-forum.surf4.info/viewtopic.php?id=17318
Comment 9 Curtis Gedak 2016-04-15 17:30:27 UTC
A patch for this issue in the libparted resize library has been committed in upstream parted.

The relevant git commits can be viewed at the following links:

lib-fs-resize: Fix recognition of FAT file system after resizing
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=c0d394abac4d6d2ce35c98585b6ecb33aea48583

Add NEWS entry for fat resize fix
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=3447264ba9fedf236da92b2199a2b4823b773cf5


Please note that the patch is currently in the upstream git master branch, and has not yet been bundled into an official parted release.
Comment 10 kdeboroda 2016-04-24 11:58:32 UTC
Just had the same problem, used about 1.5 days long to find the problem and fix it.
GRUB and Linux worked fine with the resized partition. Any utility said the partition is ALL RIGHT. But DOS and Windows did NOT work with it at all. Only testdisk said the bootsector is BAD, so I looked the FS parameters of BootSector in DiskEditor (program from Active@) - and entered them in the "Rebuild BS" dialogue in testdisk. I don't need the bootcode, but I need that OSes from m$ recognize the partition. Will it be useful if I post the differences somewhere (I have 2 dumps of disk start structures (boot sectors), 16400 bytes each?
Comment 11 Curtis Gedak 2016-04-24 15:58:41 UTC
@kdeboroda, the first post in this report already has a link to a diff file comparison of the initial sectors.

I think for Windows to recognize FAT32, it needs the initial bytes (at least 3 bytes "eb 58 90") set as follows:

$ sudo hexdump -n 7k -C /dev/sda1
00000000  eb 58 90 4d 53 57 49                              |.X.MSWI|
00000007

A patch for this issue has been committed to the (lib)parted project.  However because it might be a while before the next official (lib)parted release, we will manually patch (lib)parted in our next GParted Live release (scheduled for Apr. 26, 2016).
Comment 12 kdeboroda 2016-04-24 16:54:57 UTC
Great thanks for your quick reply!
Sorry for my English, it may not be perfect - I am from Russia.
Will the patch save the boot_sector boot code or will it simply fix the header? It's not good to have the need to reinstall the bootloader code after resizing the partition.
Comment 13 Curtis Gedak 2016-04-24 16:56:25 UTC
(In reply to kdeboroda from comment #12)
> Will the patch save the boot_sector boot code?

Yes, the patch preserves the boot_sector boot code.
Comment 14 kdeboroda 2016-04-24 16:59:21 UTC
Excellent!
Please, take your time to inform us all in this topic about the patched edition.
Thanks again.
Comment 15 kdeboroda 2016-04-24 19:06:37 UTC
(In reply to kdeboroda from comment #14)
> Please, take your time to inform us all in this topic ...

Sorry for my English. I meant, please, inform us here about the new version :).

> Thanks again.
Comment 16 Curtis Gedak 2016-05-02 17:08:17 UTC
A custom version of libparted which includes these patches was included in GParted Live 0.26.0-1.
Comment 17 Yan Pashkovsky 2017-02-14 21:23:38 UTC
Will you merge this patch with master branch of libparted?
Comment 18 Curtis Gedak 2017-02-15 15:56:54 UTC
@Yan, the patches have already been committed to the upstream parted/libparted git repository.  See comment #9.

The issue that remains is that various distros may or may not have picked up the patch as well.  Also there has been no new parted releases since version 3.2 on July 29, 2014 [1].

[1] https://savannah.gnu.org/forum/forum.php?forum_id=8042

Curtis
Comment 19 Yan Pashkovsky 2017-02-16 12:09:28 UTC
So it may be marked as fixed?
Comment 20 Curtis Gedak 2017-02-16 16:15:36 UTC
I have been holding off marking it as fixed for the reason that the patch has not been included in an official parted release.

If I close the report then it is more likely that we will receive more reports of the same issue.  For GParted users on distros that do not include the patch, this bug report is a continuing reality.

Curtis
Comment 21 Jérémie Roquet 2018-12-16 02:51:14 UTC
Thank you very much for the workaround! I've just encountered the issue when resizing a FAT32 partition with a fresh install of GParted on ArchLinux.

In case it can help someone, here's the oneliner I've used to fix the broken FS on a USB key, based on the comments above:

$ echo -ne '\xeb\x58\x90' | sudo dd conv=notrunc bs=1 count=3 of=/dev/sdb1

Be careful to target the right partition (/dev/sdb1 in my case, probably something else in yours) and to first test the command line on a text file to make sure the hexadecimal is properly interpreted by your shell (the above works in ZSH, but with other shell, you might have to double the backslashes).
Comment 22 Matthijs Kooijman 2019-05-31 13:29:17 UTC
I tried the above workaround on a FAT partition with syslinux installed, but found that it was not sufficient to keep syslinux working. Further investigation shows that when installing, syslinux installs a jump instruction (the first three bytes mentioned above) and its own name at the start of the partition and then puts the actual bootloader code at an offset of 90 bytes into the partition. With the following commands, I was able to work around this problem by backing up and restoring the relevant bits of the partition boot sector:

# Before: Backup the entire boot sector
dd if=/dev/sda1 of=/tmp/bootsect bs=512 count=1

.. Run parted fatresize ..

# After: Restore relevant bits of the bootsector
dd if=/tmp/bootsect of=/dev/sda1 bs=1 count=11
dd if=/tmp/bootsect of=/dev/sda1 bs=1 skip=90 seek=90 count=420


To figure out which bits were relevant for syslinux (which should be restored) and which bits were relevant for the filesystem (which should remain unchanged), I had a look at the syslinux installer code, in particular at and beyond the lines <https://repo.or.cz/syslinux.git/blob/05ac953c23f90b2328d393f7eecde96e41aed067:/libinstaller/fs.c#l41> and <https://repo.or.cz/syslinux.git/blob/05ac953c23f90b2328d393f7eecde96e41aed067:/libinstaller/syslxint.h#l230>. Since this defines the relevant areas using some non-trivial compiler macros and I did not feel like handcounting offsets, I wrote a small program to print the relevant values (Can be compiled with `gcc test.c` and run using `./a.out`, needs the `syslxint.h` linked above to be in the same directory while compiling):


#include <string.h>
#include <stdio.h>
#include <stddef.h>
#include "syslxint.h"

void main() {
        struct fat_boot_sector *bootsect = NULL;
        // Head area (jump instruction + OEM name)
        printf("Head at %zu size %zu\n", offsetof(struct fat_boot_sector, FAT_bsHead), FAT_bsHeadLen);
        // Code area
        printf("Code at %zu size %zu\n", offsetof(struct fat_boot_sector, FAT_bsCode), FAT_bsCodeLen);
}

Which prints:

Head at 0 size 11
Code at 90 size 420
Comment 23 André Klapper 2020-11-13 10:41:23 UTC
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports and feature requests in GNOME Bugzilla which have not seen updates for a long time.

If you still use gparted and if you still see this bug / want this feature in a recent and currently supported version, then please feel free to report it at
https://gitlab.gnome.org/GNOME/gparted/-/issues/
by following the guidelines at
https://wiki.gnome.org/Community/GettingInTouch/BugReportingGuidelines

Thank you for creating this report and we are sorry it could not be implemented so far (volunteer workforce and time is limited).