From d2d485ab244713be4ab185bae573e22e60d34a03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arkadiusz=20Mi=C5=9Bkiewicz?= Date: Thu, 13 Aug 2026 09:42:02 +0200 Subject: [PATCH 1/4] Escalate the reported status instead of overwriting it escalate_status() guarded against downgrades with ($exit_status|$exit_status_local), which is a bitwise string OR, not a logical one: 'CRITICAL'|'OK' is 'O[ITICAL', so the guard only ever matched while both variables were already equal. Under -g the per-device variable is reset for every disk, so a WARNING on a later disk silently overwrote a CRITICAL from an earlier one and the plugin exited 1 instead of 2. Rank the statuses and raise each variable only when the new one is worse. This also keeps the per-device status correct when the global status is already higher. --- check_smart.pl | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/check_smart.pl b/check_smart.pl index 4087dee..74347a1 100755 --- a/check_smart.pl +++ b/check_smart.pl @@ -81,6 +81,9 @@ # Standard Nagios return codes my %ERRORS=('OK'=>0,'WARNING'=>1,'CRITICAL'=>2,'UNKNOWN'=>3,'DEPENDENT'=>4); +# Severity ranking, used to escalate the status without ever lowering it +my %STATUS_RANK=('OK'=>0,'UNKNOWN'=>1,'WARNING'=>2,'CRITICAL'=>3); + my @sys_path = qw(/usr/bin /bin /usr/sbin /sbin /usr/local/bin /usr/local/sbin); $ENV{'BASH_ENV'}=''; @@ -953,14 +956,10 @@ sub print_help { # escalate an exit status IFF it's more severe than the previous exit status sub escalate_status { my $requested_status = shift; - # no test for 'CRITICAL'; automatically escalates upwards - if ($requested_status eq 'WARNING') { - return if ($exit_status|$exit_status_local) eq 'CRITICAL'; - } - if ($requested_status eq 'UNKNOWN') { - return if ($exit_status|$exit_status_local) eq 'WARNING'; - return if ($exit_status|$exit_status_local) eq 'CRITICAL'; - } - $exit_status = $requested_status; - $exit_status_local = $requested_status; + # $exit_status covers all devices, $exit_status_local only the current + # one; raise either only when the requested status is actually worse + $exit_status = $requested_status + if $STATUS_RANK{$requested_status} > $STATUS_RANK{$exit_status}; + $exit_status_local = $requested_status + if $STATUS_RANK{$requested_status} > $STATUS_RANK{$exit_status_local}; } From 9e5c0bcb1bc9ba3a7f3c9c8d36aa9d4bb243f249 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arkadiusz=20Mi=C5=9Bkiewicz?= Date: Thu, 13 Aug 2026 09:42:02 +0200 Subject: [PATCH 2/4] Decode the wait status of system() before testing smartctl's exit code bits system() sets $? to the wait status, so the exit code is $? >> 8. Testing $? & 0x01 .. 0x80 inspects the low byte, which is 0 for every normal exit, so none of the exit status checks could ever fire. Shifting alone would make a smartctl killed by a signal look like a clean run, so handle abnormal termination explicitly and report it as UNKNOWN. --- check_smart.pl | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/check_smart.pl b/check_smart.pl index 74347a1..3a62949 100755 --- a/check_smart.pl +++ b/check_smart.pl @@ -448,7 +448,7 @@ warn "(debug) executing:\n$full_command\n\n" if $opt_debug; system($full_command); - my $return_code = $?; + my $return_code = decoded_exit_code($?, \@error_messages); warn "(debug) exit code:\n$return_code\n\n" if $opt_debug; if ($return_code & 0x01) { @@ -498,7 +498,7 @@ warn "(debug) selftest log check activated\n\n" if $opt_debug; $full_command = "$smart_command -d $interface -q silent -l selftest $device"; system($full_command); - my $return_code = $?; + my $return_code = decoded_exit_code($?, \@error_messages); warn "(debug) exit code:\n$return_code\n\n" if $opt_debug; if ($return_code > 0) { @@ -953,6 +953,23 @@ sub print_help { print " -v/--version: Version number\n"; } +# Decode the wait status of system() into smartctl's exit code. Abnormal +# termination returns 0 so that no exit status bit is read out of a signal. +sub decoded_exit_code { + my ($wait_status, $messages) = @_; + if ($wait_status == -1) { + push(@$messages, "Failed to execute $smart_command"); + escalate_status('UNKNOWN'); + return 0; + } + if ($wait_status & 127) { + push(@$messages, sprintf('smartctl died with signal %d', $wait_status & 127)); + escalate_status('UNKNOWN'); + return 0; + } + return $wait_status >> 8; +} + # escalate an exit status IFF it's more severe than the previous exit status sub escalate_status { my $requested_status = shift; From 00e5dc403f5e0815379957908e6caa4d9085640f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arkadiusz=20Mi=C5=9Bkiewicz?= Date: Thu, 13 Aug 2026 09:42:02 +0200 Subject: [PATCH 3/4] Honour --skip-error-log for the error log exit status bit The flag suppressed the ATA Error Count check on the attribute output but not bit 6 of smartctl's exit status, which reports the same condition. --- check_smart.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check_smart.pl b/check_smart.pl index 3a62949..f17eeef 100755 --- a/check_smart.pl +++ b/check_smart.pl @@ -475,7 +475,7 @@ push(@warning_messages, 'Disk may be close to failure'); escalate_status('WARNING'); } - if ($return_code & 0x40) { + if (($return_code & 0x40) && !$opt_skip_error_log) { push(@warning_messages, 'Error log contains errors'); escalate_status('WARNING'); } From f64b0c4aff5a289c859be1a3c15dd1ff5e1858b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arkadiusz=20Mi=C5=9Bkiewicz?= Date: Thu, 13 Aug 2026 09:42:02 +0200 Subject: [PATCH 4/4] Describe exit status bit 2 the way smartctl defines it Bit 2 is "Some SMART or other ATA command to the disk failed, or there was a checksum error in a SMART data structure", and in practice it is usually the former: an optional command the drive or controller does not support. Calling it a checksum failure sends the operator after the wrong problem. --- check_smart.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check_smart.pl b/check_smart.pl index f17eeef..70a4a85 100755 --- a/check_smart.pl +++ b/check_smart.pl @@ -460,7 +460,7 @@ escalate_status('UNKNOWN'); } if ($return_code & 0x04) { - push(@warning_messages, 'Checksum failure'); + push(@warning_messages, 'SMART command failed or checksum error in SMART data'); escalate_status('WARNING'); } if ($return_code & 0x08) {