Mirrors
/
gparted
mirror of https://gitlab.gnome.org/GNOME/gparted.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Switch to POSIX open() in detect_filesystem_internal() (!46)(#16) 

For every partitioned device that GParted scans it triggers a set of
udev change events from it's internal file system signature detection
function.  This is the sequence of events:

  gparted    |set_devices_thread(pdevices)  pdevices->["/dev/sdb"]
  gparted    |  ped_device_get("/dev/sdb")
  libparted  |      open("/dev/sdb", O_RDONLY) = 8
  libparted  |      close(8)
  gparted    |  useable_device(lp_device)  lp_device->path="/dev/sdb"
  gparted    |    open("/dev/sdb", O_RDONLY|O_NONBLOCK) = 8
  gparted    |    read(8, ...)
  gparted    |    close(8)
  gparted    |  set_device_from_disk(device, device_path="/dev/sdb")
  gparted    |    get_device(device_path="/dev/sdb", ..., flush=true)
  gparted    |      ped_device_get()
  gparted    |      flush_device(lp_device)  lp_device->path="/dev/sdb"
  gparted    |        ped_device_open()
  libparted  |          open("/dev/sdb", O_RDWR) = 8
  gparted    |        ped_device_sync()
  gparted    |        ped_device_close()
  libparted  |          close(8)
  udev(async)|            KERNEL remove /devices/.../sdb/sdb1 (block)
  udev(async)|            KERNEL remove /devices/.../sdb/sdb2 (block)
  udev(async)|            KERNEL change /devices/.../sdb (block)
  udev(async)|            KERNEL add    /devices/.../sdb/sdb1 (block)
  udev(async)|            KERNEL add    /devices/.../sdb/sdb2 (block)
  udev(async)|            UDEV   remove /devices/.../sdb/sdb1 (block)
  udev(async)|            UDEV   remove /devices/.../sdb/sdb2 (block)
  udev(async)|            UDEV   change /devices/.../sdb (block)
  udev(async)|            UDEV   add    /devices/.../sdb/sdb1 (block)
  udev(async)|            UDEV   add    /devices/.../sdb/sdb2 (block)
  gparted    |        settle_device()
  gparted    |          Utils::execute_command("udevadm settle --timeout=1")
  gparted    |    detect_filesystem(lp_device, NULL, ...)  lp_device->path="/dev/sdb"
  gparted    |      detect_filesystem_internal(lp_device, NULL)  lp_device->path="/dev/sdb"
  gparted    |        ped_device_open()
  libparted  |          open("/dev/sdb", O_RDWR) = 8
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_read()
  gparted    |        ped_device_close()
  libparted  |          close(8)
  udev(async)|            KERNEL remove /devices/.../sdb/sdb1 (block)
  udev(async)|            KERNEL remove /devices/.../sdb/sdb2 (block)
  udev(async)|            KERNEL change /devices/.../sdb (block)
  udev(async)|            KERNEL add    /devices/.../sdb/sdb1 (block)
  udev(async)|            KERNEL add    /devices/.../sdb/sdb2 (block)
  udev(async)|            UDEV   remove /devices/.../sdb/sdb1 (block)
  udev(async)|            UDEV   remove /devices/.../sdb/sdb2 (block)
  udev(async)|            UDEV   change /devices/.../sdb (block)
  udev(async)|            UDEV   add    /devices/.../sdb/sdb1 (block)
  udev(async)|            UDEV   add    /devices/.../sdb/sdb2 (block)
  gparted    |    get_disk(lp_device, lp_disk, strict=false)
  gparted    |      ped_disk_new(lp_device)  lp_device->path="/dev/sdb"
  libparted  |        open("/dev/sdb", O_RDWR) = 8
  libparted  |        close(8)
  udev(async)|          KERNEL remove /devices/.../sdb/sdb1 (block)
  udev(async)|          KERNEL remove /devices/.../sdb/sdb2 (block)
  udev(async)|          KERNEL change /devices/.../sdb (block)
  udev(async)|          KERNEL add    /devices/.../sdb/sdb1 (block)
  udev(async)|          KERNEL add    /devices/.../sdb/sdb2 (block)
  udev(async)|          UDEV   remove /devices/.../sdb/sdb1 (block)
  udev(async)|          UDEV   remove /devices/.../sdb/sdb2 (block)
  udev(async)|          UDEV   change /devices/.../sdb (block)
  udev(async)|          UDEV   add    /devices/.../sdb/sdb1 (block)
  udev(async)|          UDEV   add    /devices/.../sdb/sdb2 (block)
  gparted    |      settle_device()
  gparted    |        Utils::execute_command("udevadm settle --timeout=1")
  gparted    |    set_device_partitions()
  gparted    |      detect_filesystem()
  gparted    |      set_partition_label_and_uuid(partition)  partition.get_path()="/dev/sdb1"
  gparted    |        read_label()
  gparted    |          ext2::read_label()
  gparted    |            Utils::execute_command("e2label /dev/sdb1")

Note that the udev block device change events triggered in
detect_filesystem_internal() occur before the waited for ones in the
second commit "Wait for udev to recreate /dev/PTN entries when querying
partition FSs (!46)" in get_disk() so these were already waited for.

The call chain is:
    set_devices_thread()
        set_device_from_disk()
            detect_filesystem()
                detect_filesystem_internal()
                    ped_device_open()
                    ped_device_read()
                    ped_device_close()

This occurs because file system detection is performed on whole disk
devices, but the device contains a partition table, so blkid won't
report any content GParted accepts as a file system, so it tries it's
own internal detection.

Fix this by changing detect_filesystem_internal() to use POSIX open(),
read() and close() so the file can be opened read-only and udev change
events aren't triggered.

Note that when using detect_filesystem_internal() on a partition this
also changes the device it reads from.  Before it used libparted which
always reads from the whole disk device, now it reads from the partition
device.  This doesn't matter because GParted ensures the data is
consistent between them [2] and blkid reads from each partition device
which GParted already uses.

As the new code avoids using the libparted API, and just skips to the
next signature on lseek() and read() errors, it therefore won't generate
libparted exceptions such as this when scanning very small drives:
    Invalid argument during seek for read on /dev/PTN

[1] f8faee6377
    Avoid whole disk FAT being detected as MSDOS partition table
    (#743181)

[2] 3bea067596
    Flush devices when scanning to prevent reading stale signatures
    (#723842)

Closes !46 - Whole device FAT32 file system reports device busy warning
             from mlabel
Closes #16 - "invalid argument for seek()" error on very small (<=40KiB)
             drives
This commit is contained in:
Mike Fleetwood 2019-07-11 19:16:09 +01:00 committed by Curtis Gedak
parent 7289d4c52a
commit 228374fe50
2 changed files with 10 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/GParted_Core.h
View File

@ -89,7 +89,7 @@ private:
std::vector<Glib::ustring> & messages );
void set_luks_partition( PartitionLUKS & partition );
void set_partition_label_and_uuid( Partition & partition );
static FSType detect_filesystem_internal( PedDevice * lp_device, PedPartition * lp_partition );
static FSType detect_filesystem_internal(const Glib::ustring& path, const PedDevice* lp_device);
static FSType detect_filesystem( PedDevice * lp_device, PedPartition * lp_partition,
std::vector<Glib::ustring> & messages );
void read_label( Partition & partition ) ;

18
src/GParted_Core.cc
View File

@ -1116,9 +1116,10 @@ void GParted_Core::set_partition_label_and_uuid( Partition & partition )
}
}
// GParted simple internal file system signature detection. Use sparingly. Only when
// (old versions of) blkid and libparted don't recognise a signature.
FSType GParted_Core::detect_filesystem_internal( PedDevice * lp_device, PedPartition * lp_partition )
FSType GParted_Core::detect_filesystem_internal(const Glib::ustring& path, const PedDevice* lp_device)
{
char magic1[16]; // Big enough for largest signatures[].sig1 or sig2
char magic2[16];
@ -1128,7 +1129,8 @@ FSType GParted_Core::detect_filesystem_internal( PedDevice * lp_device, PedParti
if ( ! buf )
return FS_UNKNOWN;
if ( ! ped_device_open( lp_device ) )
int fd = open(path.c_str(), O_RDONLY|O_NONBLOCK);
if (fd == -1)
{
free( buf );
return FS_UNKNOWN;
@ -1189,13 +1191,11 @@ FSType GParted_Core::detect_filesystem_internal( PedDevice * lp_device, PedParti
if ( len1 == 0UL || ( signatures[i].sig2 != NULL && len2 == 0UL ) )
continue; // Don't allow 0 length signatures to match
Sector start = 0LL;
if ( lp_partition )
start = lp_partition->geom.start;
start += signatures[i].offset1 / lp_device->sector_size;
Byte_Value read_offset = signatures[i].offset1 / lp_device->sector_size * lp_device->sector_size;
memset( buf, 0, lp_device->sector_size );
if ( ped_device_read( lp_device, buf, start, 1 ) != 0 )
if (lseek(fd, read_offset, SEEK_SET) == read_offset &&
read(fd, buf, lp_device->sector_size) == lp_device->sector_size )
{
memcpy( magic1, buf + signatures[i].offset1 % lp_device->sector_size, len1 );
@ -1215,7 +1215,7 @@ FSType GParted_Core::detect_filesystem_internal( PedDevice * lp_device, PedParti
}
}
ped_device_close( lp_device );
close(fd);
free( buf );
return fstype;
@ -1321,7 +1321,7 @@ FSType GParted_Core::detect_filesystem( PedDevice * lp_device, PedPartition * lp
}
// (Q4) Fallback to GParted simple internal file system detection
FSType fstype = detect_filesystem_internal( lp_device, lp_partition );
FSType fstype = detect_filesystem_internal(path, lp_device);
if ( fstype != FS_UNKNOWN )
return fstype;