From 7f1620d3d0962a571fba23a0d15bfe2ffd07211d Mon Sep 17 00:00:00 2001 From: Giuliano Belinassi Date: Mon, 23 Oct 2023 12:43:02 -0300 Subject: [PATCH] Fix `ulp patches -p` not returning an error Previously, the `ulp patches -p ` tool did not return an error if used in a process without livepatching capabilities. This commit fixes this. Signed-off-by: Giuliano Belinassi --- tests/patches.py | 4 +++- tools/introspection.c | 13 +++---------- tools/patches.c | 25 +++++++++++++++++-------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/tests/patches.py b/tests/patches.py index c57d8897..7fbd0409 100644 --- a/tests/patches.py +++ b/tests/patches.py @@ -33,7 +33,9 @@ except: exit(1) -if tool.returncode != 0: +# It should actually return an error because we explicitly trieed to retrieve +# information of a non-running process. +if tool.returncode == 0: exit(1) exit(0) diff --git a/tools/introspection.c b/tools/introspection.c index 6c9fc033..73baa006 100644 --- a/tools/introspection.c +++ b/tools/introspection.c @@ -1250,29 +1250,22 @@ initialize_data_structures(struct ulp_process *process) ulp_error_t ret = 0; if (!process) - return 1; + return EINVAL; DEBUG("getting in-memory information about process %d.", process->pid); if (attach(process->pid)) { - DEBUG("Unable to attach to %d to read data.\n", process->pid); - ret = 1; + ret = ETARGETHOOK; goto detach_process; } ret = parse_main_dynobj(process); if (ret) { - WARN("unable to get in-memory information about the main executable: %s\n", - libpulp_strerror(ret)); - ret = 1; goto detach_process; } ret = parse_libs_dynobj(process); if (ret) { - WARN("unable to get in-memory information about shared libraries: %s\n", - libpulp_strerror(ret)); - ret = 1; goto detach_process; } @@ -1281,7 +1274,7 @@ initialize_data_structures(struct ulp_process *process) if (read_memory((char *)&ulp_state, sizeof(ulp_state), process->pid, process->dynobj_libpulp->state) || ulp_state.load_state == 0) { - WARN("libpulp not ready (constructors not yet run). Try again later."); + DEBUG("libpulp not ready (constructors not yet run). Try again later."); ret = EAGAIN; goto detach_process; } diff --git a/tools/patches.c b/tools/patches.c index 550dc954..cea15241 100644 --- a/tools/patches.c +++ b/tools/patches.c @@ -45,7 +45,7 @@ bool enable_threading = false; static bool original_enable_threading; -void insert_target_process(int pid, struct ulp_process **list); +static int insert_target_process(int pid, struct ulp_process **list); static void * generate_ulp_list_thread(void *args) @@ -131,6 +131,10 @@ process_list_next(struct ulp_process_iterator *it) symbols. */ static pthread_t process_list_thread; +/** In case of `ulp patches -p `, we want to return an error for the + caller if something went wrong. */ +static error_t any_error = ENONE; + struct ulp_process * process_list_begin(struct ulp_process_iterator *it, const char *procname_wildcard, const char *user_wildcard) @@ -147,7 +151,7 @@ process_list_begin(struct ulp_process_iterator *it, if (isnumber(procname_wildcard)) { /* If wildcard is actually a number, then treat it as a PID. */ pid = atoi(procname_wildcard); - insert_target_process(pid, &it->last); + any_error = insert_target_process(pid, &it->last); it->now = it->last; /* Disable threading in this case. */ @@ -552,7 +556,8 @@ print_remote_err_status(struct ulp_process *p) } static void -print_process(struct ulp_process *process, int print_buildid, bool only_livepatched) +print_process(struct ulp_process *process, int print_buildid, + bool only_livepatched) { struct ulp_dynobj *object_item; pid_t pid = process->pid; @@ -575,7 +580,8 @@ print_process(struct ulp_process *process, int print_buildid, bool only_livepatc if (print_buildid) printf(" (%s)", buildid_to_string(object_item->build_id)); if (object_item->libpulp_version) { - const char *version = ulp_version_as_string(object_item->libpulp_version); + const char *version = + ulp_version_as_string(object_item->libpulp_version); printf(" (version %s)", version); } printf(":\n"); @@ -618,7 +624,7 @@ has_libpulp_loaded(pid_t pid) /* Inserts a new process structure into LIST if the process identified * by PID is live-patchable. */ -void +static int insert_target_process(int pid, struct ulp_process **list) { struct ulp_process *new = NULL; @@ -629,14 +635,16 @@ insert_target_process(int pid, struct ulp_process **list) new->pid = pid; ret = initialize_data_structures(new); if (ret) { - WARN("error gathering target process information."); + WARN("error gathering target process information: %s", + libpulp_strerror(ret)); release_ulp_process(new); - return; + return ret; } else { new->next = *list; *list = new; } + return 0; } /** @brief Prints all the info collected about the processes in `process_list`. @@ -668,11 +676,12 @@ run_patches(struct arguments *arguments) const char *user_wildcard = arguments->user_wildcard; struct ulp_process *p; + any_error = ENONE; FOR_EACH_ULP_PROCESS_FROM_USER_WILDCARD(p, process_wildcard, user_wildcard) { print_process(p, print_build_id, only_livepatched); } - return 0; + return any_error == 0 ? 0 : 1; }