Accepting request 594835 from Kernel:kdump
OBS-URL: https://build.opensuse.org/request/show/594835 OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/makedumpfile?expand=0&rev=68
This commit is contained in:
90
makedumpfile-do-not-print-ETA-if-progress-is-0.patch
Normal file
90
makedumpfile-do-not-print-ETA-if-progress-is-0.patch
Normal file
@@ -0,0 +1,90 @@
|
||||
From: Petr Tesarik <ptesarik@suse.com>
|
||||
Date: Mon, 9 Apr 2018 09:59:46 +0200
|
||||
Subject: Do not print ETA value if current progress is 0
|
||||
References: bsc#1084936
|
||||
Upstream: submitted 2018-04-09
|
||||
|
||||
Essentially, the estimated remaining time is calculated as:
|
||||
|
||||
elapsed * (100 - progress) / progress
|
||||
|
||||
However, print_progress() is also called when progress is 0. The
|
||||
result of a floating point division by zero is either NaN (if
|
||||
elapsed is zero), or infinity (if the system clock happens to cross
|
||||
a second's boundary since reading the start timestamp).
|
||||
|
||||
The C standard defines only conversion of floating point values
|
||||
within the range of the destination integer variable. This means
|
||||
that conversion of NaN or infinity to an integer is undefined
|
||||
behaviour. Yes, it happens to produce INT_MIN with GCC on major
|
||||
platforms...
|
||||
|
||||
This bug has gone unnoticed, because the very first call to
|
||||
print_progress() does not specify a start timestamp (so it cannot
|
||||
trigger the bug), and all subsequent calls are rate-limited to one
|
||||
per second. As a result, the bug is triggered very rarely.
|
||||
|
||||
Before commit e5f96e79d69a1d295f19130da00ec6514d28a8ae, the bug also
|
||||
caused a buffer overflow. The buffer overflow is mitigated thanks to
|
||||
using snprintf() instead of sprintf(), but the program may still
|
||||
invoke undefined behaviour.
|
||||
|
||||
Note that all other changes in the above-mentioned commit were
|
||||
ineffective. They merely reduced the precision of the calculation:
|
||||
Why would you add delta.tv_usec as a fraction if the fractional part
|
||||
is immediately truncated by a converstion to int64_t?
|
||||
|
||||
Additionally, when the original bug is hit, the output is still
|
||||
incorrect, e.g. on my system I get:
|
||||
|
||||
Copying data : [ 0.0 %] / eta: -9223372036854775808s
|
||||
|
||||
For that reason, let me revert the changes from commit
|
||||
e5f96e79d69a1d295f19130da00ec6514d28a8ae and fix the bug properly,
|
||||
i.e. do not calculate ETA if progress is 0.
|
||||
|
||||
Last but not least, part of the issue was probably caused by the
|
||||
wrong assumption that integers < 100 can be interpreted with max 3
|
||||
ASCII characters, but that's not true for signed integers. To make
|
||||
eta_to_human_short() a bit safer, use an unsigned integer type.
|
||||
|
||||
Signed-off-by: Petr Tesarik <ptesarik@suse.com>
|
||||
---
|
||||
print_info.c | 12 ++++++------
|
||||
1 file changed, 6 insertions(+), 6 deletions(-)
|
||||
|
||||
--- a/print_info.c
|
||||
+++ b/print_info.c
|
||||
@@ -352,18 +352,18 @@ static void calc_delta(struct timeval *t
|
||||
}
|
||||
|
||||
/* produce less than 12 bytes on msg */
|
||||
-static int eta_to_human_short (int secs, char* msg)
|
||||
+static int eta_to_human_short (unsigned secs, char* msg)
|
||||
{
|
||||
strcpy(msg, "eta: ");
|
||||
msg += strlen("eta: ");
|
||||
if (secs < 100)
|
||||
- sprintf(msg, "%ds", secs);
|
||||
+ sprintf(msg, "%us", secs);
|
||||
else if (secs < 100 * 60)
|
||||
- sprintf(msg, "%dm%ds", secs / 60, secs % 60);
|
||||
+ sprintf(msg, "%um%us", secs / 60, secs % 60);
|
||||
else if (secs < 48 * 3600)
|
||||
- sprintf(msg, "%dh%dm", secs / 3600, (secs / 60) % 60);
|
||||
+ sprintf(msg, "%uh%um", secs / 3600, (secs / 60) % 60);
|
||||
else if (secs < 100 * 86400)
|
||||
- sprintf(msg, "%dd%dh", secs / 86400, (secs / 3600) % 24);
|
||||
+ sprintf(msg, "%ud%uh", secs / 86400, (secs / 3600) % 24);
|
||||
else
|
||||
sprintf(msg, ">2day");
|
||||
return 0;
|
||||
@@ -391,7 +391,7 @@ print_progress(const char *msg, unsigned
|
||||
} else
|
||||
progress = 100;
|
||||
|
||||
- if (start != NULL) {
|
||||
+ if (start != NULL && current != 0) {
|
||||
calc_delta(start, &delta);
|
||||
eta = delta.tv_sec + delta.tv_usec / 1e6;
|
||||
eta = (100 - progress) * eta / progress;
|
@@ -1,3 +1,9 @@
|
||||
-------------------------------------------------------------------
|
||||
Mon Apr 9 09:53:05 UTC 2018 - ptesarik@suse.com
|
||||
|
||||
- makedumpfile-do-not-print-ETA-if-progress-is-0.patch: Do not
|
||||
print ETA value if current progress is 0 (bsc#1084936).
|
||||
|
||||
-------------------------------------------------------------------
|
||||
Mon Mar 26 16:46:43 CEST 2018 - kukuk@suse.de
|
||||
|
||||
|
@@ -41,6 +41,7 @@ Patch0: %{name}-coptflags.diff
|
||||
Patch1: %{name}-override-libtinfo.patch
|
||||
Patch2: %{name}-always-use-bigger-SECTION_MAP_MASK.patch
|
||||
Patch3: %{name}-sadump-fix-PTI-enabled-kernels.patch
|
||||
Patch4: %{name}-do-not-print-ETA-if-progress-is-0.patch
|
||||
BuildRequires: libdw-devel
|
||||
BuildRequires: libebl-devel
|
||||
BuildRequires: libelf-devel
|
||||
@@ -72,6 +73,7 @@ via gdb or crash utility.
|
||||
%patch1 -p1
|
||||
%patch2 -p1
|
||||
%patch3 -p1
|
||||
%patch4 -p1
|
||||
|
||||
%build
|
||||
%if %{have_snappy}
|
||||
|
Reference in New Issue
Block a user