Skip to content

Commit

Permalink
Fix ulp patches -p not returning an error
Browse files Browse the repository at this point in the history
Previously, the `ulp patches -p <pid>` tool did not return an error if
used in a process without livepatching capabilities.  This commit fixes
this.

Signed-off-by: Giuliano Belinassi <[email protected]>
  • Loading branch information
giulianobelinassi committed Oct 23, 2023
1 parent 40b7aa7 commit 7f1620d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 19 deletions.
4 changes: 3 additions & 1 deletion tests/patches.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
13 changes: 3 additions & 10 deletions tools/introspection.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}
Expand Down
25 changes: 17 additions & 8 deletions tools/patches.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 <pid>`, 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)
Expand All @@ -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. */
Expand Down Expand Up @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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;
Expand All @@ -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`.
Expand Down Expand Up @@ -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;
}

0 comments on commit 7f1620d

Please sign in to comment.