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 693955 - mkdosfs detects "complete disk" vs. "partition" incorrectly
mkdosfs detects "complete disk" vs. "partition" incorrectly
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2013-02-16 13:22 UTC by Jan Claeys
Modified: 2013-03-19 16:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add -I option to mkdosfs for FAT16 & FAT32 filesystem creation (1.93 KB, patch)
2013-02-16 19:57 UTC, Jan Claeys
none Details | Review
add -I option to mkdosfs for FAT16 & FAT32 filesystem creation (reformatted commit message) (2.01 KB, patch)
2013-02-16 21:42 UTC, Jan Claeys
none Details | Review

Description Jan Claeys 2013-02-16 13:22:57 UTC
Copy/paste from IRC:

<Kano> hi
<Kano> i have got a small suggestion for the fat creation
<Kano> i can not create fat on mdadm raid partitions until i hack gparted like
<Kano> sed -ri 's|(mkdosfs )-F(..) -v|\1-vIF\2 |g' /usr/sbin/gpartedbin
<Kano> that means i need -I option
<Kano> maybe add it by default


From 'man mkdosfs':

       -I     It is typical for fixed disk devices to be  partitioned  so,  by
              default, you are not permitted to create a filesystem across the
              entire device.  mkdosfs will  complain  and  tell  you  that  it
              refuses  to  work.   This is different when using MO disks.  One
              doesn't always need partitions on MO disks.  The file system can
              go  directly  to the whole disk.  Under other OSes this is known
              as the 'superfloppy' format.

              This switch will force mkdosfs to work properly.


Adding -I by default doesn't sound like something dangerous to me, as it would just make mkdosfs work like the mkfs for (most) other filesystems?

BTW: this probably also impacts other device types that aren't simple partitions using a traditional partitioning scheme (I'm not sure what mkdosfs considers a "partition").
Comment 1 Jan Claeys 2013-02-16 17:26:22 UTC
Excerpt from 'mkdosfs.c':

    if (fstat(dev, &statbuf) < 0)
        die("unable to stat %s");
    if (!S_ISBLK(statbuf.st_mode)) {
        statbuf.st_rdev = 0;
        check = 0;
    } else
        /*
         * Ignore any 'full' fixed disk devices, if -I is not given.
         * On a MO-disk one doesn't need partitions.  The filesytem can go
         * directly to the whole disk.  Under other OSes this is known as
         * the 'superfloppy' format.  As I don't know how to find out if
         * this is a MO disk I introduce a -I (ignore) switch.  -Joey
         */
        if (!ignore_full_disk && ((statbuf.st_rdev & 0xff3f) == 0x0300 ||       /* hda, hdb */
                                  (statbuf.st_rdev & 0xff0f) == 0x0800 ||       /* sd */
                                  (statbuf.st_rdev & 0xff3f) == 0x0d00 ||       /* xd */
                                  (statbuf.st_rdev & 0xff3f) == 0x1600) /* hdc, hdd */
        )
        die("Device partition expected, not making filesystem on entire device '%s' (use -I to override)");


Not only does that look like non-portable code to me (linux-only?), but I'm pretty sure that even on linux it doesn't do reliably what it supposedly does...?
Comment 2 Curtis Gedak 2013-02-16 17:52:37 UTC
Hi Jan,

Thank you for reporting this problem, how you fixed it, and the code from mkdosfs.c regarding the -I option.

Your suggestion of adding the "-I" option to the mkdosfs command in GParted sounds reasonable.  Since GParted only permits formatting partitions, I do not foresee any problem with adding this option.

Since you discovered the problem and suggested the solution, would you like to create a git patch for this?
Comment 3 Jan Claeys 2013-02-16 17:57:14 UTC
I asked Kano to run stat on one of those partitions:

$ LANG= stat /dev/md126p1 
  File: `/dev/md126p1'
  Size: 0               Blocks: 0          IO Block: 4096   block special file
Device: 5h/5d   Inode: 8603        Links: 1     Device type: 103,0
Access: (1660/brw-rw---T)  Uid: (    0/    root)   Gid: (    6/    disk)
Access: 2013-02-16 17:53:40.650868999 +0100
Modify: 2013-02-16 17:53:40.630868999 +0100
Change: 2013-02-16 17:53:40.630868999 +0100


And a quick check by me shows:

$ python3.3
Python 3.3.0 (default, Sep 29 2012, 17:14:58) 
[GCC 4.7.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> hex(os.makedev(0x103,0) & 0xff3f)
'0x300'


So this check in mkdosfs is working incorrectly (catching a RAID *partition*
with a test for IDE hard disk devices?), just like I thought...  (I think it
should have used major() instead of playing with bitmasks on the raw device
number to be portable & future proof?)

Fortunately, the -I option works around it (it sets the 'ignore_full_disk'
variable to 1).
Comment 4 Jan Claeys 2013-02-16 19:57:56 UTC
Created attachment 236405 [details] [review]
add -I option to mkdosfs for FAT16 & FAT32 filesystem creation

I think this patch should fix this issue.
Comment 5 Curtis Gedak 2013-02-16 20:51:49 UTC
Adding the bug report of the upstream report of the problem with mkdosfs as Jan pointed it out to me:


Launchpad 935480 - mkdosfs refuses to run on /dev/loop0p1 
https://bugs.launchpad.net/ubuntu/+source/dosfstools/+bug/935480
Comment 6 Jan Claeys 2013-02-16 21:42:30 UTC
Created attachment 236413 [details] [review]
add -I option to mkdosfs for FAT16 & FAT32 filesystem creation (reformatted commit message)

Hopefully the commit message is policy-compliant now  :)
Comment 7 Curtis Gedak 2013-02-21 17:48:05 UTC
Thank you Jan for identifying this problem and providing a patch.

This enhancement has been committed to the git repository for inclusion in the next release of GParted.

The relevant git commit can be viewed at the following link:

Work around faulty "complete disks" detection in mkdosfs (#693955)
http://git.gnome.org/browse/gparted/commit/?id=cdb6cbfa80601859a96cbfc0fd8f0f5c55147345
Comment 8 Curtis Gedak 2013-02-25 18:33:25 UTC
It looks like dosfstools upstream has a patch to address the faulty "complete disks" detection problem.  The fix is included in dosfstools 3.0.15.

Dosfstools
http://www.daniel-baumann.ch/software/dosfstools/

Redhat Bug Report - mkfs.vfat doesn't correctly detect device partitions
https://bugzilla.redhat.com/show_bug.cgi?id=710480

Fix in git for device detection:
http://www.daniel-baumann.ch/gitweb/?p=software/dosfstools.git;a=commit;h=b8201b34eb83c2bf87bd1c2b1a7fa57a1dc28191

I believe it is advantageous to keep Jan's patch in GParted because this enables the proper operation of GParted for all dosfstools versions.
Comment 9 Curtis Gedak 2013-03-19 16:57:26 UTC
The code change to address this report has been included in GParted 0.15.0 released on March 19, 2013.