Merge pull request #1127 from a-wai/partition-layout-size-checks

Add various checks for partition layout configuration

src/libcalamares/utils/NamedEnum.h

@@ -1,6 +1,7 @@
 /* === This file is part of Calamares - <https://github.com/calamares> ===
  *
  *   Copyright 2019, Adriaan de Groot <groot@kde.org>
+ *   Copyright 2019, Collabora Ltd <arnaud.ferraris@collabora.com>
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -30,6 +31,7 @@
 
 #include <QString>
 
+#include <vector>
 #include <type_traits>
 #include <initializer_list>
 

src/modules/partition/core/PartUtils.cpp

@@ -43,6 +43,216 @@
 namespace PartUtils
 {
 
+static const NamedEnumTable<SizeUnit>&
+unitSuffixes()
+{
+    static const NamedEnumTable<SizeUnit> names{
+        { QStringLiteral( "%" ), SizeUnit::Percent },
+        { QStringLiteral( "B" ), SizeUnit::Byte },
+        { QStringLiteral( "K" ), SizeUnit::KiB },
+        { QStringLiteral( "M" ), SizeUnit::MiB },
+        { QStringLiteral( "G" ), SizeUnit::GiB }
+    };
+
+    return names;
+}
+
+PartSize::PartSize( const QString& s )
+    : NamedSuffix( unitSuffixes(), s )
+{
+    if ( ( unit() == SizeUnit::Percent ) && ( value() > 100 || value() < 0 ) )
+    {
+        cDebug() << "Percent value" << value() << "is not valid.";
+        m_value = 0;
+    }
+
+    if ( m_unit == SizeUnit::None )
+    {
+        m_value = s.toInt();
+        if ( m_value > 0 )
+            m_unit = SizeUnit::Byte;
+    }
+
+    if ( m_value <= 0 )
+    {
+        m_value = 0;
+        m_unit = SizeUnit::None;
+    }
+}
+
+qint64
+PartSize::toSectors( qint64 totalSectors, qint64 sectorSize ) const
+{
+    if ( !isValid() )
+        return -1;
+    if ( totalSectors < 1 || sectorSize < 1 )
+        return -1;
+
+    switch ( m_unit )
+    {
+    case unit_t::None:
+        return -1;
+    case unit_t::Percent:
+        if ( value() == 100 )
+            return totalSectors;  // Common-case, avoid futzing around
+        else
+            return totalSectors * value() / 100;
+    case unit_t::Byte:
+    case unit_t::KiB:
+    case unit_t::MiB:
+    case unit_t::GiB:
+        return bytesToSectors ( toBytes(), sectorSize );
+    }
+
+    return -1;
+}
+
+qint64
+PartSize::toBytes( qint64 totalSectors, qint64 sectorSize ) const
+{
+    if ( !isValid() )
+        return -1;
+
+    switch ( m_unit )
+    {
+    case unit_t::None:
+        return -1;
+    case unit_t::Percent:
+        if ( totalSectors < 1 || sectorSize < 1 )
+            return -1;
+        if ( value() == 100 )
+            return totalSectors * sectorSize;  // Common-case, avoid futzing around
+        else
+            return totalSectors * value() / 100;
+    case unit_t::Byte:
+    case unit_t::KiB:
+    case unit_t::MiB:
+    case unit_t::GiB:
+        return toBytes();
+    }
+
+    // notreached
+    return -1;
+}
+
+qint64
+PartSize::toBytes( qint64 totalBytes ) const
+{
+    if ( !isValid() )
+        return -1;
+
+    switch ( m_unit )
+    {
+    case unit_t::None:
+        return -1;
+    case unit_t::Percent:
+        if ( totalBytes < 1 )
+            return -1;
+        if ( value() == 100 )
+            return totalBytes;  // Common-case, avoid futzing around
+        else
+            return totalBytes * value() / 100;
+    case unit_t::Byte:
+    case unit_t::KiB:
+    case unit_t::MiB:
+    case unit_t::GiB:
+        return toBytes();
+    }
+
+    // notreached
+    return -1;
+}
+
+qint64
+PartSize::toBytes() const
+{
+    if ( !isValid() )
+        return -1;
+
+    switch ( m_unit )
+    {
+    case unit_t::Byte:
+        return value();
+    case unit_t::KiB:
+        return CalamaresUtils::KiBtoBytes( static_cast<unsigned long long>( value() ) );
+    case unit_t::MiB:
+        return CalamaresUtils::MiBtoBytes( static_cast<unsigned long long>( value() ) );
+    case unit_t::GiB:
+        return CalamaresUtils::GiBtoBytes( static_cast<unsigned long long>( value() ) );
+    default:
+        break;
+    }
+
+    // Reached only when unit is Percent or None
+    return -1;
+}
+
+bool
+PartSize::operator< ( const PartSize& other ) const
+{
+    if ( ( m_unit == SizeUnit::None || other.m_unit == SizeUnit::None ) ||
+         ( m_unit == SizeUnit::Percent && other.m_unit != SizeUnit::Percent ) ||
+         ( m_unit != SizeUnit::Percent && other.m_unit == SizeUnit::Percent ) )
+        return false;
+
+    switch ( m_unit )
+    {
+    case SizeUnit::Percent:
+        return ( m_value < other.m_value );
+    case SizeUnit::Byte:
+    case SizeUnit::KiB:
+    case SizeUnit::MiB:
+    case SizeUnit::GiB:
+        return ( toBytes() < other.toBytes () );
+    }
+
+    return false;
+}
+
+bool
+PartSize::operator> ( const PartSize& other ) const
+{
+    if ( ( m_unit == SizeUnit::None || other.m_unit == SizeUnit::None ) ||
+         ( m_unit == SizeUnit::Percent && other.m_unit != SizeUnit::Percent ) ||
+         ( m_unit != SizeUnit::Percent && other.m_unit == SizeUnit::Percent ) )
+        return false;
+
+    switch ( m_unit )
+    {
+    case SizeUnit::Percent:
+        return ( m_value > other.m_value );
+    case SizeUnit::Byte:
+    case SizeUnit::KiB:
+    case SizeUnit::MiB:
+    case SizeUnit::GiB:
+        return ( toBytes() > other.toBytes () );
+    }
+
+    return false;
+}
+
+bool
+PartSize::operator== ( const PartSize& other ) const
+{
+    if ( ( m_unit == SizeUnit::None || other.m_unit == SizeUnit::None ) ||
+         ( m_unit == SizeUnit::Percent && other.m_unit != SizeUnit::Percent ) ||
+         ( m_unit != SizeUnit::Percent && other.m_unit == SizeUnit::Percent ) )
+        return false;
+
+    switch ( m_unit )
+    {
+    case SizeUnit::Percent:
+        return ( m_value == other.m_value );
+    case SizeUnit::Byte:
+    case SizeUnit::KiB:
+    case SizeUnit::MiB:
+    case SizeUnit::GiB:
+        return ( toBytes() == other.toBytes () );
+    }
+
+    return false;
+}
+
 QString
 convenienceName( const Partition* const candidate )
 {
@@ -484,99 +694,6 @@ findFS( QString fsName, FileSystem::Type* fsType )
     return fsName;
 }
 
-static qint64
-sizeToBytes( double size, SizeUnit unit, qint64 totalSize )
-{
-    qint64 bytes;
-
-    switch ( unit )
-    {
-    case SizeUnit::Percent:
-        bytes = qint64( static_cast<double>( totalSize ) * size / 100.0L );
-        break;
-    case SizeUnit::KiB:
-        bytes = CalamaresUtils::KiBtoBytes(size);
-        break;
-    case SizeUnit::MiB:
-        bytes = CalamaresUtils::MiBtoBytes(size);
-        break;
-    case SizeUnit::GiB:
-        bytes = CalamaresUtils::GiBtoBytes(size);
-        break;
-    default:
-        bytes = size;
-        break;
-    }
-
-    return bytes;
-}
-
-double
-parseSizeString( const QString& sizeString, SizeUnit* unit )
-{
-    double value;
-    bool ok;
-    QString valueString;
-    QString unitString;
-
-    QRegExp rx( "[KkMmGg%]" );
-    int pos = rx.indexIn( sizeString );
-    if (pos > 0)
-    {
-        valueString = sizeString.mid( 0, pos );
-        unitString = sizeString.mid( pos );
-    }
-    else
-        valueString = sizeString;
-
-    value = valueString.toDouble( &ok );
-    if ( !ok )
-    {
-        /*
-         * In case the conversion fails, a size of 100% allows a few cases to pass
-         * anyway (e.g. when it is the last partition of the layout)
-         */
-        *unit = SizeUnit::Percent;
-        return 100.0L;
-    }
-
-    if ( unitString.length() > 0 )
-    {
-        if ( unitString.at(0) == '%' )
-            *unit = SizeUnit::Percent;
-        else if ( unitString.at(0).toUpper() == 'K' )
-            *unit = SizeUnit::KiB;
-        else if ( unitString.at(0).toUpper() == 'M' )
-            *unit = SizeUnit::MiB;
-        else if ( unitString.at(0).toUpper() == 'G' )
-            *unit = SizeUnit::GiB;
-        else
-            *unit = SizeUnit::Byte;
-    }
-    else
-    {
-        *unit = SizeUnit::Byte;
-    }
-
-    return value;
-}
-
-qint64
-parseSizeString( const QString& sizeString, qint64 totalSize )
-{
-    SizeUnit unit;
-    double value = parseSizeString( sizeString, &unit );
-
-    return sizeToBytes( value, unit, totalSize );
-}
-
-qint64
-sizeToSectors( double size, SizeUnit unit, qint64 totalSectors, qint64 logicalSize )
-{
-    qint64 bytes = sizeToBytes( size, unit, totalSectors * logicalSize );
-    return bytesToSectors( static_cast<unsigned long long>( bytes ), logicalSize );
-}
-
 }  // nmamespace PartUtils
 
 /* Implementation of methods for FstabEntry, from OsproberEntry.h */

src/modules/partition/core/PartUtils.h

@@ -23,6 +23,7 @@
 
 #include "OsproberEntry.h"
 #include "utils/Units.h"
+#include "utils/NamedSuffix.h"
 
 // KPMcore
 #include <kpmcore/fs/filesystem.h>
@@ -37,15 +38,77 @@ namespace PartUtils
 {
 using CalamaresUtils::MiBtoBytes;
 
-enum SizeUnit
+enum class SizeUnit
 {
-    Percent = 0,
+    None,
+    Percent,
     Byte,
     KiB,
     MiB,
     GiB
 };
 
+/** @brief Partition size expressions
+ *
+ * Sizes can be specified in bytes, KiB, MiB, GiB or percent (of
+ * the available drive space are on). This class handles parsing
+ * of such strings from the config file.
+ */
+class PartSize : public NamedSuffix<SizeUnit, SizeUnit::None>
+{
+public:
+    PartSize() : NamedSuffix() { };
+    PartSize( int v, unit_t u ) : NamedSuffix( v, u ) { };
+    PartSize( const QString& );
+
+    bool isValid() const
+    {
+        return ( unit() != SizeUnit::None ) && ( value() > 0 );
+    }
+
+    bool operator< ( const PartSize& other ) const;
+    bool operator> ( const PartSize& other ) const;
+    bool operator== ( const PartSize& other ) const;
+
+    /** @brief Convert the size to the number of sectors @p totalSectors .
+     *
+     * Each sector has size @p sectorSize, for converting sizes in Bytes,
+     * KiB, MiB or GiB to sector counts.
+     *
+     * @return  the number of sectors needed, or -1 for invalid sizes.
+     */
+    qint64 toSectors( qint64 totalSectors, qint64 sectorSize ) const;
+
+    /** @brief Convert the size to bytes.
+     *
+     * The device's sectors count @p totalSectors and sector size
+     * @p sectoreSize are used to calculated the total size, which
+     * is then used to calculate the size when using Percent.
+     *
+     * @return  the size in bytes, or -1 for invalid sizes.
+     */
+    qint64 toBytes( qint64 totalSectors, qint64 sectorSize ) const;
+
+    /** @brief Convert the size to bytes.
+     *
+     * Total size @p totalBytes is needed for sizes in Percent. This
+     * parameter is unused in any other case.
+     *
+     * @return  the size in bytes, or -1 for invalid sizes.
+     */
+    qint64 toBytes( qint64 totalBytes ) const;
+
+    /** @brief Convert the size to bytes.
+     *
+     * This method is only valid for sizes in Bytes, KiB, MiB or GiB.
+     * It will return -1 in any other case.
+     *
+     * @return  the size in bytes, or -1 if it cannot be calculated.
+     */
+    qint64 toBytes() const;
+};
+
+
 /**
  * @brief Provides a nice human-readable name for @p candidate
  *
@@ -108,22 +171,6 @@ bool isEfiBootable( const Partition* candidate );
  */
 QString findFS( QString fsName, FileSystem::Type* fsType );
 
-/**
- * @brief Parse a partition size string and return its value and unit used.
- * @param sizeString the string to parse.
- * @param unit pointer to a SizeUnit variable for storing the parsed unit.
- * @return the size value, as parsed from the input string.
- */
-double parseSizeString( const QString& sizeString, SizeUnit* unit );
-
-/**
- * @brief Parse a partition size string and return its value in bytes.
- * @param sizeString the string to parse.
- * @param totalSize the size of the selected drive (used when the size is expressed in %)
- * @return the size value in bytes.
- */
-qint64 parseSizeString( const QString& sizeString, qint64 totalSize );
-
 /**
  * @brief Convert a partition size to a sectors count.
  * @param size the partition size.

src/modules/partition/core/PartitionActions.cpp

@@ -102,9 +102,14 @@ doAutopartition( PartitionCoreModule* core, Device* dev, Choices::AutoPartitionO
     if ( isEfi )
     {
         if ( gs->contains( "efiSystemPartitionSize" ) )
-            uefisys_part_sizeB = PartUtils::parseSizeString( gs->value( "efiSystemPartitionSize" ).toString(), dev->capacity() );
+        {
+            PartUtils::PartSize part_size = PartUtils::PartSize( gs->value( "efiSystemPartitionSize" ).toString() );
+            uefisys_part_sizeB = part_size.toBytes( dev->capacity() );
+        }
         else
+        {
             uefisys_part_sizeB = 300_MiB;
+        }
     }
 
     // Since sectors count from 0, if the space is 2048 sectors in size,

src/modules/partition/core/PartitionCoreModule.cpp

@@ -4,6 +4,7 @@
  *   Copyright 2014-2015, Teo Mrnjavac <teo@kde.org>
  *   Copyright 2017-2018, Adriaan de Groot <groot@kde.org>
  *   Copyright 2018, Caio Carvalho <caiojcarvalho@gmail.com>
+ *   Copyright 2019, Collabora Ltd <arnaud.ferraris@collabora.com>
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -797,6 +798,16 @@ PartitionCoreModule::initLayout( const QVariantList& config )
     {
         QVariantMap pentry = r.toMap();
 
+        if ( !pentry.contains( "name" ) || !pentry.contains( "mountPoint" ) ||
+             !pentry.contains( "filesystem" ) || !pentry.contains( "size" ) )
+        {
+            cError() << "Partition layout entry #" << config.indexOf(r)
+                << "lacks mandatory attributes, switching to default layout.";
+            delete( m_partLayout );
+            initLayout();
+            break;
+        }
+
         if ( pentry.contains("size") && CalamaresUtils::getString( pentry, "size" ).isEmpty() )
             sizeString.setNum( CalamaresUtils::getInteger( pentry, "size", 0 ) );
         else
@@ -808,17 +819,24 @@ PartitionCoreModule::initLayout( const QVariantList& config )
             minSizeString = CalamaresUtils::getString( pentry, "minSize" );
 
         if ( pentry.contains("maxSize") && CalamaresUtils::getString( pentry, "maxSize" ).isEmpty() )
-            maxSizeString.setNum( CalamaresUtils::getInteger( pentry, "maxSize", 100 ) );
+            maxSizeString.setNum( CalamaresUtils::getInteger( pentry, "maxSize", 0 ) );
         else
             maxSizeString = CalamaresUtils::getString( pentry, "maxSize" );
 
-        m_partLayout->addEntry( CalamaresUtils::getString( pentry, "name" ),
-                                CalamaresUtils::getString( pentry, "mountPoint" ),
-                                CalamaresUtils::getString( pentry, "filesystem" ),
-                                sizeString,
-                                minSizeString,
-                                maxSizeString
-                              );
+        if ( !m_partLayout->addEntry( CalamaresUtils::getString( pentry, "name" ),
+                                      CalamaresUtils::getString( pentry, "mountPoint" ),
+                                      CalamaresUtils::getString( pentry, "filesystem" ),
+                                      sizeString,
+                                      minSizeString,
+                                      maxSizeString
+                                    ) )
+        {
+            cError() << "Partition layout entry #" << config.indexOf(r)
+                << "is invalid, switching to default layout.";
+            delete( m_partLayout );
+            initLayout();
+            break;
+        }
     }
 }
 

src/modules/partition/core/PartitionLayout.cpp

@@ -2,7 +2,7 @@
  *
  *   Copyright 2014-2017, Teo Mrnjavac <teo@kde.org>
  *   Copyright 2017-2018, Adriaan de Groot <groot@kde.org>
- *   Copyright 2018, Collabora Ltd
+ *   Copyright 2018-2019, Collabora Ltd <arnaud.ferraris@collabora.com>
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -21,6 +21,8 @@
 #include "GlobalStorage.h"
 #include "JobQueue.h"
 
+#include "utils/Logger.h"
+
 #include "core/PartitionLayout.h"
 
 #include "core/KPMHelpers.h"
@@ -69,37 +71,67 @@ PartitionLayout::~PartitionLayout()
 {
 }
 
-void
+bool
 PartitionLayout::addEntry( PartitionLayout::PartitionEntry entry )
 {
+    if ( !entry.isValid() )
+    {
+        cError() << "Partition size is invalid or has min size > max size";
+        return false;
+    }
+
     m_partLayout.append( entry );
+
+    return true;
 }
 
 PartitionLayout::PartitionEntry::PartitionEntry( const QString& size, const QString& min, const QString& max )
 {
-    partSize = PartUtils::parseSizeString( size , &partSizeUnit );
-    if ( !min.isEmpty() )
-        partMinSize = PartUtils::parseSizeString( min , &partMinSizeUnit );
-    if ( !max.isEmpty() )
-        partMaxSize = PartUtils::parseSizeString( max , &partMaxSizeUnit );
+    partSize = PartUtils::PartSize( size );
+    partMinSize = PartUtils::PartSize( min );
+    partMaxSize = PartUtils::PartSize( max );
 }
 
-void
+bool
 PartitionLayout::addEntry( const QString& mountPoint, const QString& size, const QString& min, const QString& max )
 {
     PartitionLayout::PartitionEntry entry( size, min, max );
 
+    if ( !entry.isValid() )
+    {
+        cError() << "Partition size" << size << "is invalid or" << min << ">" << max;
+        return false;
+    }
+    if ( mountPoint.isEmpty() || !mountPoint.startsWith( QString( "/" ) ) )
+    {
+        cError() << "Partition mount point" << mountPoint << "is invalid";
+        return false;
+    }
+
     entry.partMountPoint = mountPoint;
     entry.partFileSystem = m_defaultFsType;
 
     m_partLayout.append( entry );
+
+    return true;
 }
 
-void
+bool
 PartitionLayout::addEntry( const QString& label, const QString& mountPoint, const QString& fs, const QString& size, const QString& min, const QString& max )
 {
     PartitionLayout::PartitionEntry entry( size, min, max );
 
+    if ( !entry.isValid() )
+    {
+        cError() << "Partition size" << size << "is invalid or" << min << ">" << max;
+        return false;
+    }
+    if ( mountPoint.isEmpty() || !mountPoint.startsWith( QString( "/" ) ) )
+    {
+        cError() << "Partition mount point" << mountPoint << "is invalid";
+        return false;
+    }
+
     entry.partLabel = label;
     entry.partMountPoint = mountPoint;
     PartUtils::findFS( fs, &entry.partFileSystem );
@@ -107,6 +139,8 @@ PartitionLayout::addEntry( const QString& label, const QString& mountPoint, cons
         entry.partFileSystem = m_defaultFsType;
 
     m_partLayout.append( entry );
+
+    return true;
 }
 
 QList< Partition* >
@@ -128,9 +162,36 @@ PartitionLayout::execute( Device *dev, qint64 firstSector,
         Partition *currentPartition = nullptr;
 
         // Calculate partition size
-        size = PartUtils::sizeToSectors( part.partSize, part.partSizeUnit, totalSize, dev->logicalSize() );
-        minSize = PartUtils::sizeToSectors( part.partMinSize, part.partMinSizeUnit, totalSize, dev->logicalSize() );
-        maxSize = PartUtils::sizeToSectors( part.partMaxSize, part.partMaxSizeUnit, totalSize, dev->logicalSize() );
+        if ( part.partSize.isValid() )
+        {
+            size = part.partSize.toSectors( totalSize, dev->logicalSize() );
+        }
+        else
+        {
+            cWarning() << "Partition" << part.partMountPoint << "size ("
+                << size <<  "sectors) is invalid, skipping...";
+            continue;
+        }
+
+        if ( part.partMinSize.isValid() )
+            minSize = part.partMinSize.toSectors( totalSize, dev->logicalSize() );
+        else
+            minSize = 0;
+
+        if ( part.partMaxSize.isValid() )
+            maxSize = part.partMaxSize.toSectors( totalSize, dev->logicalSize() );
+        else
+            maxSize = availableSize;
+
+        // Make sure we never go under minSize once converted to sectors
+        if ( maxSize < minSize )
+        {
+            cWarning() << "Partition" << part.partMountPoint << "max size (" << maxSize
+                <<  "sectors) is < min size (" << minSize << "sectors), using min size";
+            maxSize = minSize;
+        }
+
+        // Adjust partition size based on user-defined boundaries and available space
         if ( size < minSize )
             size = minSize;
         if ( size > maxSize )

src/modules/partition/core/PartitionLayout.h

@@ -1,6 +1,6 @@
 /* === This file is part of Calamares - <https://github.com/calamares> ===
  *
- *   Copyright 2018, Collabora Ltd
+ *   Copyright 2018-2019, Collabora Ltd <arnaud.ferraris@collabora.com>
  *   Copyright 2019, Adriaan de Groot <groot@kde.org>
  *
  *   Calamares is free software: you can redistribute it and/or modify
@@ -43,17 +43,22 @@ public:
         QString partLabel;
         QString partMountPoint;
         FileSystem::Type partFileSystem = FileSystem::Unknown;
-        double partSize = 0.0L;
-        PartUtils::SizeUnit partSizeUnit = PartUtils::SizeUnit::Percent;
-        double partMinSize = 0.0L;
-        PartUtils::SizeUnit partMinSizeUnit = PartUtils::SizeUnit::Percent;
-        double partMaxSize = 100.0L;
-        PartUtils::SizeUnit partMaxSizeUnit = PartUtils::SizeUnit::Percent;
+        PartUtils::PartSize partSize;
+        PartUtils::PartSize partMinSize;
+        PartUtils::PartSize partMaxSize;
 
         /// @brief All-zeroes PartitionEntry
         PartitionEntry() {}
         /// @brief Parse @p size, @p min and @p max to their respective member variables
         PartitionEntry( const QString& size, const QString& min, const QString& max );
+
+        bool isValid() const
+        {
+            if ( !partSize.isValid() ||
+                 ( partMinSize.isValid() && partMaxSize.isValid() && partMinSize > partMaxSize ) )
+                return false;
+            return true;
+        }
     };
 
     PartitionLayout();
@@ -61,9 +66,9 @@ public:
     PartitionLayout( const PartitionLayout& layout );
     ~PartitionLayout();
 
-    void addEntry( PartitionEntry entry );
-    void addEntry( const QString& mountPoint, const QString& size, const QString& min = QString(), const QString& max = QString() );
-    void addEntry( const QString& label, const QString& mountPoint, const QString& fs, const QString& size, const QString& min = QString(), const QString& max = QString() );
+    bool addEntry( PartitionEntry entry );
+    bool addEntry( const QString& mountPoint, const QString& size, const QString& min = QString(), const QString& max = QString() );
+    bool addEntry( const QString& label, const QString& mountPoint, const QString& fs, const QString& size, const QString& min = QString(), const QString& max = QString() );
 
     /**
      * @brief Apply the current partition layout to the selected drive space.