Fix bugs and improve error handling in emerg-shutdown#359
Open
assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
Open
Fix bugs and improve error handling in emerg-shutdown#359assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
Conversation
- Fix print() writing NUL terminator to fd (off-by-one in strlen+1) - Fix load_list() hardcoding "," separator instead of using sep param, breaking pipe-separated key alias parsing - Add missing exit() after kill_system() in paranoid mode and --instant-shutdown to prevent fall-through on syscall failure - Fix memory leak of arg_copy on early return in load_list() - Fix overly strict device key support check: require any alias per group to be supported, not all aliases - Add NULL check for empty --timeout= value to prevent NULL deref - Remove duplicate stdbool.h include - Initialize console_fd to -1 instead of 0 (stdin) https://claude.ai/code/session_016o12KW3j7F1dUBsXBXjfQg
ArrayBolt3
reviewed
Apr 7, 2026
Contributor
ArrayBolt3
left a comment
ArrayBolt3
left a comment
There was a problem hiding this comment.
Mostly accepted in ArrayBolt3@17a9da5. Comments below.
| #include <fcntl.h> | ||
| #include <stdlib.h> | ||
| #include <stdint.h> | ||
| #include <stdbool.h> |
Comment on lines
-113
to
+112
| int console_fd = 0; | ||
| int console_fd = -1; |
Contributor
There was a problem hiding this comment.
Accepted. (Doesn't actually do anything but this is probably a more reasonable default.)
Comment on lines
-280
to
+279
| size_t len = strlen(str) + 1; | ||
| size_t len = strlen(str); |
Contributor
There was a problem hiding this comment.
Already applied from an earlier PR.
|
|
||
| arg_part = strtok(arg_val, sep); | ||
| if (arg_part == NULL) { | ||
| free(arg_copy); |
| result_list[result_list_len - 1] = safe_calloc(1, strlen(arg_part) + 1); | ||
| strcpy(result_list[result_list_len - 1], arg_part); | ||
| } while ((arg_part = strtok(NULL, ",")) != NULL); | ||
| } while ((arg_part = strtok(NULL, sep)) != NULL); |
Contributor
There was a problem hiding this comment.
Already applied from an earlier PR.
| if (paranoid_mode) { | ||
| /* Something was removed, we don't care what, shut down now */ | ||
| kill_system(); | ||
| exit(0); |
Contributor
There was a problem hiding this comment.
exit() was moved into kill_system().
| } | ||
|
|
||
| kill_system(); | ||
| exit(0); |
Contributor
There was a problem hiding this comment.
exit() was moved into kill_system().
Comment on lines
+694
to
+702
| bool group_supported = false; | ||
| for (kg_idx = 0; panic_key_list[pkl_idx][kg_idx] != 0; kg_idx++) { | ||
| if (!bitset_get(key_flags, panic_key_list[pkl_idx][kg_idx])) { | ||
| supports_panic = false; | ||
| if (bitset_get(key_flags, panic_key_list[pkl_idx][kg_idx])) { | ||
| group_supported = true; | ||
| break; | ||
| } | ||
| } | ||
| if (!supports_panic) { | ||
| if (!group_supported) { | ||
| supports_panic = false; |
Comment on lines
+958
to
+962
| if (arg_part == NULL) { | ||
| print(fd_stderr, "Timeout value is empty!\n"); | ||
| print_usage(); | ||
| exit(1); | ||
| } |
Contributor
There was a problem hiding this comment.
Already accepted from an earlier PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses several bugs and improves error handling in the emergency shutdown utility.
Key changes:
#include <stdbool.h>which was not needed0to-1to properly represent an uninitialized file descriptor+ 1fromstrlen(str)in theprint()function to avoid writing the null terminatorfree(arg_copy)whenstrtok()returns NULL inload_list()to prevent memory leaks on early return","to use thesepvariable in the secondstrtok()call for consistencyhw_monitor()to correctly detect when a key group is supported (changed from checking if key is NOT supported to checking if it IS supported)exit(0)afterkill_system()calls to ensure proper program terminationarg_partinfifo_monitor()to validate that the timeout value is not empty before parsing itThese changes improve code correctness, prevent resource leaks, and enhance error handling robustness.
https://claude.ai/code/session_016o12KW3j7F1dUBsXBXjfQg