Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error return codes to libcare-ctl. #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/kpatch_log.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ void kpfatal(const char *fmt, ...)
__valog(LOG_ERR, "FATAL! ", fmt, va);
va_end(va);

exit(1);
#ifdef STRESS_TEST
if (parent_pid >= 0)
stress_test_notify_parent();
#endif
exit(ERROR_FATAL);
}

extern int elf_errno(void) __attribute__((weak));
Expand Down Expand Up @@ -98,5 +102,5 @@ void _kpfatalerror(const char *file, int line, const char *fmt, ...)
__valogerror(file, line, fmt, va);
va_end(va);

exit(EXIT_FAILURE);
exit(ERROR_FATAL);
}
22 changes: 22 additions & 0 deletions src/kpatch_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,28 @@

#include <stdio.h>

#define ERROR_CODE(return_code) (return_code - 256)

#define ERROR_SUCCESS 0
#define ERROR_GENERAL ERROR_CODE(1)
// ERROR_FATAL: libcare-ctl encountered fatal error and terminated execution
#define ERROR_FATAL ERROR_CODE(2)
// ERROR_ARGUMENTS: libcare-ctl was invoked with wrong arguments
#define ERROR_ARGUMENTS ERROR_CODE(3)
// ERROR_PROCESS_NOT_FOUND: process with specified pid does not exist or have already exited
#define ERROR_PROCESS_NOT_FOUND ERROR_CODE(4)
// ERROR_PATCH_NOT_FOUND: patch file not found, or patch of same or higher patch level is already applied to the process
#define ERROR_PATCH_NOT_FOUND ERROR_CODE(5)
// ERROR_PATCH_FAILURE: libcare-ctl failed to apply patch
#define ERROR_PATCH_FAILURE ERROR_CODE(6)
// ERROR_UNPATCH_FAILURE: libcare-ctl failed to cancel patch
// ERROR_UNPATCH_FAILURE can also be returned in case of error during unpatch after unsuccessful patching
#define ERROR_UNPATCH_FAILURE ERROR_CODE(8)
// ERROR_RESOURCE_ACCESS: libcare-ctl was not able to attach to process(PTRACE_ATTACH, write access to /proc/PID/mem or stack unwind)
#define ERROR_RESOURCE_ACCESS ERROR_CODE(9)
// ERROR_RESOURCE_BUSY: process is currently executing code that must be modified and can't be patched at this moment
#define ERROR_RESOURCE_BUSY ERROR_CODE(10)

extern int log_level, log_indent;

void kplog(int level, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
Expand Down
84 changes: 51 additions & 33 deletions src/kpatch_patch.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,20 @@ patch_ensure_safety(struct object_file *o,
ret = kpatch_ptrace_execute_until(o->proc, 3000, 0);

/* OK, at this point we may have new threads, discover them */
if (ret == 0)
if (ret == 0) {
ret = kpatch_process_attach(o->proc);
if (ret == ERROR_RESOURCE_ACCESS) {
free(retips);
return ret;
}
}
if (ret == 0)
ret = patch_verify_safety(o, NULL, action);
}

free(retips);

return ret ? -1 : 0;
return ret ? ERROR_RESOURCE_BUSY : 0;
}

/*****************************************************************************
Expand Down Expand Up @@ -347,12 +352,12 @@ object_apply_patch(struct object_file *o)
ret = duplicate_kp_file(o);
if (ret < 0) {
kplogerror("can't duplicate kp_file\n");
return -1;
return ERROR_PATCH_FAILURE;
}

ret = kpatch_elf_load_kpatch_info(o);
if (ret < 0)
return ret;
return ERROR_PATCH_FAILURE;

kp = o->kpfile.patch;

Expand All @@ -379,26 +384,26 @@ object_apply_patch(struct object_file *o)
*/
ret = kpatch_object_allocate_patch(o, sz);
if (ret < 0)
return ret;
return ERROR_PATCH_FAILURE;
ret = kpatch_resolve(o);
if (ret < 0)
return ret;
return ERROR_PATCH_FAILURE;
ret = kpatch_relocate(o);
if (ret < 0)
return ret;
return ERROR_PATCH_FAILURE;
ret = kpatch_process_mem_write(o->proc,
kp,
o->kpta,
kp->total_size);
if (ret < 0)
return -1;
return ERROR_PATCH_FAILURE;
if (o->jmp_table) {
ret = kpatch_process_mem_write(o->proc,
o->jmp_table,
o->kpta + kp->jmp_offset,
o->jmp_table->size);
if (ret < 0)
return ret;
return ERROR_PATCH_FAILURE;
}

ret = patch_ensure_safety(o, ACTION_APPLY_PATCH);
Expand All @@ -408,7 +413,7 @@ object_apply_patch(struct object_file *o)
for (i = 0; i < o->ninfo; i++) {
ret = patch_apply_hunk(o, i);
if (ret < 0)
return ret;
return ERROR_PATCH_FAILURE;
}

return 1;
Expand Down Expand Up @@ -442,9 +447,10 @@ object_unapply_old_patch(struct object_file *o)
kpatch_applied->user_level,
kpatch_storage->user_level);
ret = object_unapply_patch(o, /* check_flag */ 0);
if (ret < 0)
if (ret < 0) {
kperr("can't unapply patch for %s\n", o->name);
else {
ret = ERROR_UNPATCH_FAILURE;
} else {
/* TODO(pboldin): handle joining the holes here */
o->applied_patch = NULL;
o->info = NULL;
Expand All @@ -464,7 +470,7 @@ kpatch_apply_patches(kpatch_process_t *proc)

ret = object_unapply_old_patch(o);
if (ret < 0)
break;
return ret;

ret = object_apply_patch(o);
if (ret < 0)
Expand All @@ -480,11 +486,11 @@ kpatch_apply_patches(kpatch_process_t *proc)
* TODO(pboldin): close the holes so the state is the same
* after unpatch
*/
ret = object_unapply_patch(o, /* check_flag */ 1);
if (ret < 0) {
if (object_unapply_patch(o, /* check_flag */ 1) < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, don't hesitate to use more lines. There is no need to short it like this, let's keep the code a single statement a line and let compiler do its job.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way value of 'ret' is not changed if object_unapply_patch was successful, and error code from object_apply_patch is returned. If object_unapply_patch returns error, ERROR_PATCH_UNPATCH will be returned to notify user.

ret = ERROR_UNPATCH_FAILURE;
kperr("Can't unapply patch for %s\n", o->name);
}
return -1;
return ret;
}

int process_patch(int pid, void *_data)
Expand Down Expand Up @@ -565,16 +571,18 @@ int process_patch(int pid, void *_data)

out:
if (ret < 0) {
printf("Failed to apply patch '%s'\n", storage->path);
if (ret == -1)
return ERROR_PATCH_FAILURE;
kpinfo("Failed to apply patch '%s'\n", storage->path);
kperr("Failed to apply patch '%s'\n", storage->path);
} else if (ret == 0)
printf("No patch(es) applicable to PID '%d' have been found\n", pid);
else {
printf("%d patch hunk(s) have been successfully applied to PID '%d'\n", ret, pid);
ret = 0;
return ret;
} else if (ret == 0) {
kpinfo("No patch(es) applicable to PID '%d' have been found\n", pid);
return ERROR_PATCH_NOT_FOUND;
} else {
kpinfo("%d patch hunk(s) have been successfully applied to PID '%d'\n", ret, pid);
return ERROR_SUCCESS;
}

return ret;
}


Expand All @@ -592,6 +600,8 @@ object_find_applied_patch_info(struct object_file *o)

if (o->info != NULL)
return 0;
if (!o->kpta || !o->kpfile.patch)
return -1;

iter = kpatch_process_mem_iter_init(o->proc);
if (iter == NULL)
Expand All @@ -617,6 +627,8 @@ object_find_applied_patch_info(struct object_file *o)
o->ninfo++;
} while (1);

if (!o->applied_patch)
return 0;
o->applied_patch->info = o->info;
o->applied_patch->ninfo = o->ninfo;

Expand All @@ -641,6 +653,8 @@ object_unapply_patch(struct object_file *o, int check_flag)
if (ret < 0)
return ret;

if (!o->kpta || !o->kpfile.patch)
return -1;
orig_code_addr = o->kpta + o->kpfile.patch->user_undo;

for (i = 0; i < o->ninfo; i++) {
Expand Down Expand Up @@ -728,7 +742,7 @@ int process_unpatch(int pid, void *_data)

ret = kpatch_process_init(proc, pid, /* start */ 0, /* send_fd */ -1);
if (ret < 0)
return -1;
return ret;

kpatch_process_print_short(proc);

Expand All @@ -749,13 +763,17 @@ int process_unpatch(int pid, void *_data)
out:
kpatch_process_free(proc);

if (ret < 0)
printf("Failed to cancel patches for %d\n", pid);
else if (ret == 0)
printf("No patch(es) cancellable from PID '%d' were found\n", pid);
else
printf("%d patch hunk(s) were successfully cancelled from PID '%d'\n", ret, pid);

return ret;
if (ret < 0) {
kpinfo("Failed to cancel patches for %d\n", pid);
if (ret == -1)
return ERROR_UNPATCH_FAILURE;
return ret;
} else if (ret == 0) {
kpinfo("No patch(es) cancellable from PID '%d' were found\n", pid);
return ERROR_PATCH_NOT_FOUND;
} else {
kpinfo("%d patch hunk(s) were successfully cancelled from PID '%d'\n", ret, pid);
return 0;
}
}

10 changes: 5 additions & 5 deletions src/kpatch_process.c
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ kpatch_process_mem_open(kpatch_process_t *proc, int mode)
proc->memfd = open(path, mode == MEM_WRITE ? O_RDWR : O_RDONLY);
if (proc->memfd < 0) {
kplogerror("can't open /proc/%d/mem", proc->pid);
return -1;
return ERROR_PROCESS_NOT_FOUND;
}

return 0;
Expand All @@ -643,7 +643,7 @@ kpatch_process_attach(kpatch_process_t *proc)
size_t i, npids = 0, alloc = 0, prevnpids = 0, nattempts;

if (kpatch_process_mem_open(proc, MEM_WRITE) < 0)
return -1;
return ERROR_RESOURCE_ACCESS;

for (nattempts = 0; nattempts < max_attach_attempts; nattempts++) {
ret = process_list_threads(proc, &pids, &npids, &alloc);
Expand Down Expand Up @@ -713,7 +713,7 @@ kpatch_process_attach(kpatch_process_t *proc)
process_detach(proc);
dealloc:
free(pids);
return -1;
return ERROR_RESOURCE_ACCESS;
}

static void
Expand Down Expand Up @@ -890,7 +890,7 @@ kpatch_process_load_libraries(kpatch_process_t *proc)
ret = kpatch_process_attach(proc);
if (ret < 0) {
kperr("unable to attach to just started process\n");
return -1;
return ret;
}

if (proc->send_fd != -1) {
Expand Down Expand Up @@ -1131,7 +1131,7 @@ kpatch_process_init(kpatch_process_t *proc,
out_unlock:
unlock_process(pid, fdmaps);
out_err:
return -1;
return ERROR_PROCESS_NOT_FOUND;
}

void
Expand Down
27 changes: 15 additions & 12 deletions src/kpatch_user.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ static char storage_dir[PATH_MAX] = "/var/lib/libcare";
* Utilities.
****************************************************************************/

/* Return -1 to indicate error, -2 to stop immediately */
/* Return -1 to indicate error */
typedef int (callback_t)(int pid, void *data);

static int
Expand Down Expand Up @@ -68,7 +68,7 @@ static int usage_patch(const char *err)
fprintf(stderr, "\nOptions:\n");
fprintf(stderr, " -h - this message\n");
fprintf(stderr, " -p <PID> - target process\n");
return err ? 0 : -1;
return err ? 0 : ERROR_ARGUMENTS;
}

static int
Expand All @@ -80,7 +80,7 @@ patch_user(const char *storage_path, int pid,

ret = storage_init(&storage, storage_path);
if (ret < 0)
return ret;
return ERROR_PATCH_NOT_FOUND;

ret = processes_patch(&storage, pid, is_just_started, send_fd);

Expand Down Expand Up @@ -337,7 +337,7 @@ process_info(int pid, void *_data)

ret = kpatch_process_init(proc, pid, /* start */ 0, /* send_fd */ -1);
if (ret < 0)
return -1;
return ret;

ret = kpatch_process_mem_open(proc, MEM_READ);
if (ret < 0)
Expand Down Expand Up @@ -631,6 +631,7 @@ static int cmd_stress_test(int fd, int argc, char *argv[])
{
int child = fork();
if (child == 0) {
signal(SIGCHLD, SIG_DFL);
int rv = server_stress_test(fd, argc, argv);
exit(rv);
}
Expand All @@ -640,8 +641,12 @@ static int cmd_stress_test(int fd, int argc, char *argv[])

static int usage_stresstest()
{
fprintf(stderr, "usage: libcare-stresstest PERIOD(ms, 0 - only patch) <UNIX socket> [STORAGE ROOT]\n");
return -1;
fprintf(stderr, "usage: libcare-stresstest [args] PERIOD(ms, 0 - only patch) <UNIX socket> [STORAGE ROOT]\n");
fprintf(stderr, "\nOptions:\n");
fprintf(stderr, " -v - verbose mode\n");
fprintf(stderr, " -l <BASE> - log file name <BASE>-PID\n");
fprintf(stderr, " -h - this message\n");
return ERROR_ARGUMENTS;
}

#endif
Expand Down Expand Up @@ -745,7 +750,7 @@ static int usage_server(const char *err)
if (err)
fprintf(stderr, "err: %s\n", err);
fprintf(stderr, "usage: libcare-ctl server <UNIX socket> [STORAGE ROOT]\n");
return -1;
return ERROR_ARGUMENTS;
}

#define LISTEN_BACKLOG 1
Expand Down Expand Up @@ -904,9 +909,7 @@ processes_do(int pid, callback_t callback, void *data)

rv = callback(pid, data);
if (rv < 0)
ret = -1;
if (rv == -2)
break;
ret = rv;
}

closedir(dir);
Expand All @@ -930,7 +933,7 @@ static int usage(const char *err)
fprintf(stderr, " unpatch- unapply patch from a user-space process\n");
fprintf(stderr, " info - show info on applied patches\n");
fprintf(stderr, " server - listen on a unix socket for commands\n");
return -1;
return ERROR_ARGUMENTS;
}

static int
Expand Down Expand Up @@ -960,7 +963,7 @@ int main(int argc, char *argv[])
log_level += 1;
break;
case 'h':
return usage(NULL);
return usage(0);
default:
return usage("unknown option");
}
Expand Down