From 0c8b86ea09e31b5c3b7138854cf42dc845b6f5c8 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Sat, 30 Jun 2018 02:10:12 +0530 Subject: [PATCH] lib/repo: Minor fixes around min-free-space Summary: * Remove a useless if condition in prepare_transaction() * Fix glnx_throw error propagation * Integer overflow check while parsing min-free-space-size config * Documentation fixes Closes: #1663 Approved by: jlebon --- man/ostree.repo-config.xml | 6 ++-- src/libostree/ostree-repo-commit.c | 48 ++++++++++++++---------------- src/libostree/ostree-repo.c | 10 +++++-- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 09312d5c..602d412e 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -126,10 +126,10 @@ Boston, MA 02111-1307, USA. min-free-space-size - Value (in MB, GB or TB) that specifies a minimum space (in blocks) - in the underlying filesystem to keep free. Also, note that min-free-space-percent + Value (in MB, GB or TB) that specifies a minimum space in the + underlying filesystem to keep free. Also, note that min-free-space-percent and min-free-space-size are mutually exclusive. Examples of acceptable values: - 500MB, 1GB etc. + 500MB, 1GB etc. The default value is 0MB, which disables this check. diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 54bd8b66..05635afd 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1632,34 +1632,32 @@ ostree_repo_prepare_transaction (OstreeRepo *self, self->in_transaction = TRUE; self->cleanup_stagedir = FALSE; - if (self->min_free_space_percent >= 0 || self->min_free_space_mb >= 0) - { - struct statvfs stvfsbuf; - if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0) - return glnx_throw_errno_prefix (error, "fstatvfs"); - g_mutex_lock (&self->txn_lock); - self->txn.blocksize = stvfsbuf.f_bsize; - guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf); - /* Use the appropriate free block count if we're unprivileged */ - guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree); - if (bfree > reserved_blocks) - self->txn.max_blocks = bfree - reserved_blocks; - else - { - guint64 bytes_required = bfree * self->txn.blocksize; - self->cleanup_stagedir = TRUE; - g_mutex_unlock (&self->txn_lock); - g_autofree char *formatted_free = g_format_size (bytes_required); - if (self->min_free_space_percent > 0) - return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available", - self->min_free_space_percent, formatted_free); - else - return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s available", - self->min_free_space_mb, formatted_free); - } + struct statvfs stvfsbuf; + if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0) + return glnx_throw_errno_prefix (error, "fstatvfs"); + + g_mutex_lock (&self->txn_lock); + self->txn.blocksize = stvfsbuf.f_bsize; + guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf); + /* Use the appropriate free block count if we're unprivileged */ + guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree); + if (bfree > reserved_blocks) + self->txn.max_blocks = bfree - reserved_blocks; + else + { + guint64 bytes_required = bfree * self->txn.blocksize; + self->cleanup_stagedir = TRUE; g_mutex_unlock (&self->txn_lock); + g_autofree char *formatted_free = g_format_size (bytes_required); + if (self->min_free_space_percent > 0) + return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available", + self->min_free_space_percent, formatted_free); + else + return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s available", + self->min_free_space_mb, formatted_free); } + g_mutex_unlock (&self->txn_lock); gboolean ret_transaction_resume = FALSE; if (!_ostree_repo_allocate_tmpdir (self->tmp_dir_fd, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index d64e4125..53d5cfda 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2670,7 +2670,7 @@ min_free_space_size_validate_and_convert (OstreeRepo *self, g_autoptr(GMatchInfo) match = NULL; if (!g_regex_match (regex, min_free_space_size_str, 0, &match)) - return glnx_prefix_error (error, "Failed to parse min-free-space-size parameter: '%s'", min_free_space_size_str); + return glnx_throw (error, "Failed to match '^[0-9]+[GMT]B$'"); g_autofree char *size_str = g_match_info_fetch (match, 1); g_autofree char *unit = g_match_info_fetch (match, 2); @@ -2691,7 +2691,11 @@ min_free_space_size_validate_and_convert (OstreeRepo *self, g_assert_not_reached (); } - self->min_free_space_mb = g_ascii_strtoull (size_str, NULL, 10) << shifts; + guint64 min_free_space = g_ascii_strtoull (size_str, NULL, 10); + if (shifts > 0 && g_bit_nth_lsf (min_free_space, 63 - shifts) != -1) + return glnx_throw (error, "Integer overflow detected"); + + self->min_free_space_mb = min_free_space << shifts; return TRUE; } @@ -2829,7 +2833,7 @@ reload_core_config (OstreeRepo *self, /* Validate the string and convert the size to MBs */ if (!min_free_space_size_validate_and_convert (self, min_free_space_size_str, error)) - return glnx_throw (error, "Invalid min-free-space-size '%s'", min_free_space_size_str); + return glnx_prefix_error (error, "Invalid min-free-space-size '%s'", min_free_space_size_str); } else {