Skip to content

Commit

Permalink
patch-hook: fix possible cast errors
Browse files Browse the repository at this point in the history
Livepatch callbacks are packed into specific sections
by gcc by the macros defined in kpatch-macros.h. Callbacks
take the form of:

    struct example {
        type *(fn)(struct klp_object *obj);
        char* objname;
    }

However, create-diff-objects.c can't include the kernel livepatch
header because it is compiled separately from the kernel module.
So the solution before was to define the structs in the header file as:

    struct example {
                type *(fn)(void *obj);
                char* objname;
        }

and then cast fn to (type *(fn)(struct klp_object *obj)) if necessary.
This way, the same header file and definitions can be used in
both places.

But! If compiled with the right GCC flags, such as -Wbad-function-cast,
this casting may throw an error during livepatch building, which is what
I am seeing (not sure why, since it was always fine before...).

Solution: Define the callback function parameter as the correct type, which
allows us to get rid of the troublesome casting.

Since we need to know if callbacks are present in kpatch-patch.h, we can move
the definition of HAVE_CALLBACKS to from livepatch-patch-hook.c to kpatch-patch.h.

IMPORTANT: For correctness, this change requires proper ordering of include
statements! kpatch-patch.h must always be included AFTER linux/livepatch.h
(if using linux/livepatch.h).

Signed-off-by: Brennan Lamoreaux <[email protected]>
  • Loading branch information
bhllamoreaux committed Dec 11, 2024
1 parent 846db25 commit 5f12b31
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 21 deletions.
1 change: 1 addition & 0 deletions kmod/patch/kpatch-patch-hook.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <linux/slab.h>
#include <linux/kallsyms.h>
#include "kpatch.h"
#include <linux/livepatch.h>
#include "kpatch-patch.h"

static bool replace;
Expand Down
37 changes: 33 additions & 4 deletions kmod/patch/kpatch-patch.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,34 @@
#ifndef _KPATCH_PATCH_H_
#define _KPATCH_PATCH_H_

#include <linux/version.h>

/* Support for livepatch callbacks? */
#ifdef RHEL_RELEASE_CODE
# if RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 5)
# define HAVE_CALLBACKS
# endif
#elif LINUX_VERSION_CODE >= KERNEL_VERSION(4, 15, 0)
# define HAVE_CALLBACKS
#endif

#ifdef HAVE_CALLBACKS
/* IMPORTANT: include ordering is critical! */
# ifdef _LINUX_LIVEPATCH_H_
typedef struct klp_object klp_obj;
# else
/*
* Basically just a placeholder for when we can't include linux/livepatch.h.
* The correct type, which is what gets packed in the section, is
* struct klp_object.
*/
typedef void klp_obj;
# endif /* _LINUX_LIVEPATCH_H_ */
#else
#include "kpatch.h"
typedef struct kpatch_object klp_obj;
#endif /* HAVE_CALLBACKS */

struct kpatch_patch_func {
unsigned long new_addr;
unsigned long new_size;
Expand All @@ -43,20 +71,21 @@ struct kpatch_patch_dynrela {
long addend;
};


struct kpatch_pre_patch_callback {
int (*callback)(void *obj);
int (*callback)(klp_obj *obj);
char *objname;
};
struct kpatch_post_patch_callback {
void (*callback)(void *obj);
void (*callback)(klp_obj *obj);
char *objname;
};
struct kpatch_pre_unpatch_callback {
void (*callback)(void *obj);
void (*callback)(klp_obj *obj);
char *objname;
};
struct kpatch_post_unpatch_callback {
void (*callback)(void *obj);
void (*callback)(klp_obj *obj);
char *objname;
};

Expand Down
22 changes: 5 additions & 17 deletions kmod/patch/livepatch-patch-hook.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2013-2014 Josh Poimboeuf <[email protected]>
* Copyright (C) 2014 Seth Jennings <[email protected]>
* Copyright (C) 2014 Seth Jennings <[email protected]>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
Expand Down Expand Up @@ -56,14 +56,6 @@
# define HAVE_IMMEDIATE
#endif

#ifdef RHEL_RELEASE_CODE
# if RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 5)
# define HAVE_CALLBACKS
# endif
#elif LINUX_VERSION_CODE >= KERNEL_VERSION(4, 15, 0)
# define HAVE_CALLBACKS
#endif

#ifdef RHEL_RELEASE_CODE
# if (RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 8) && \
RHEL_RELEASE_CODE < RHEL_RELEASE_VERSION(8, 0)) || \
Expand Down Expand Up @@ -356,8 +348,7 @@ static int add_callbacks_to_patch_objects(void)
object->name ? object->name : "vmlinux");
return -EINVAL;
}
object->callbacks.pre_patch = (int (*)(struct klp_object *))
p_pre_patch_callback->callback;
object->callbacks.pre_patch = p_pre_patch_callback->callback;
}

for (p_post_patch_callback = __kpatch_callbacks_post_patch;
Expand All @@ -371,8 +362,7 @@ static int add_callbacks_to_patch_objects(void)
object->name ? object->name : "vmlinux");
return -EINVAL;
}
object->callbacks.post_patch = (void (*)(struct klp_object *))
p_post_patch_callback->callback;
object->callbacks.post_patch = p_post_patch_callback->callback;
}

for (p_pre_unpatch_callback = __kpatch_callbacks_pre_unpatch;
Expand All @@ -386,8 +376,7 @@ static int add_callbacks_to_patch_objects(void)
object->name ? object->name : "vmlinux");
return -EINVAL;
}
object->callbacks.pre_unpatch = (void (*)(struct klp_object *))
p_pre_unpatch_callback->callback;
object->callbacks.pre_unpatch = p_pre_unpatch_callback->callback;
}

for (p_post_unpatch_callback = __kpatch_callbacks_post_unpatch;
Expand All @@ -401,8 +390,7 @@ static int add_callbacks_to_patch_objects(void)
object->name ? object->name : "vmlinux");
return -EINVAL;
}
object->callbacks.post_unpatch = (void (*)(struct klp_object *))
p_post_unpatch_callback->callback;
object->callbacks.post_unpatch = p_post_unpatch_callback->callback;
}

return 0;
Expand Down

0 comments on commit 5f12b31

Please sign in to comment.