Add global checks for partition layout

This commit adds several checks while reading the configuration of the
`partition` module, in case the partition layout configuration is
misformed. If an error is encountered, an message is printed to the
console and the module reverts to the default partition layout.

Checks are also added when implementing the partition layout, in case a
problem occurs that couldn't be anticipated (for example, when a
partition size is in %, checking its absolute value require knowing the
total device size, which is not the case when the configuration is
being read).

Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>

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

@@ -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::PartSize( size );
-    if ( !min.isEmpty() )
-        partMinSize = PartUtils::PartSize( min );
-    if ( !max.isEmpty() )
-        partMaxSize = PartUtils::PartSize( max );
+    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 = part.partSize.toSectors( totalSize, dev->logicalSize() );
-        minSize = part.partMinSize.toSectors( totalSize, dev->logicalSize() );
-        maxSize = part.partMaxSize.toSectors( 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

@@ -43,14 +43,22 @@ public:
         QString partLabel;
         QString partMountPoint;
         FileSystem::Type partFileSystem = FileSystem::Unknown;
-        PartUtils::PartSize partSize = PartUtils::PartSize(0, PartUtils::SizeUnit::Percent);
-        PartUtils::PartSize partMinSize = PartUtils::PartSize(0, PartUtils::SizeUnit::Percent);
-        PartUtils::PartSize partMaxSize = PartUtils::PartSize(100, 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();
@@ -58,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.