From 5f12b31389bf3d628f0846214383acf18f769c5a Mon Sep 17 00:00:00 2001 From: Brennan Lamoreaux Date: Wed, 11 Dec 2024 19:13:23 +0000 Subject: [PATCH] patch-hook: fix possible cast errors 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 --- kmod/patch/kpatch-patch-hook.c | 1 + kmod/patch/kpatch-patch.h | 37 +++++++++++++++++++++++++++---- kmod/patch/livepatch-patch-hook.c | 22 +++++------------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/kmod/patch/kpatch-patch-hook.c b/kmod/patch/kpatch-patch-hook.c index 92f101def..e77004d19 100644 --- a/kmod/patch/kpatch-patch-hook.c +++ b/kmod/patch/kpatch-patch-hook.c @@ -24,6 +24,7 @@ #include #include #include "kpatch.h" +#include #include "kpatch-patch.h" static bool replace; diff --git a/kmod/patch/kpatch-patch.h b/kmod/patch/kpatch-patch.h index da4f6a065..57846756b 100644 --- a/kmod/patch/kpatch-patch.h +++ b/kmod/patch/kpatch-patch.h @@ -22,6 +22,34 @@ #ifndef _KPATCH_PATCH_H_ #define _KPATCH_PATCH_H_ +#include + +/* 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; @@ -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; }; diff --git a/kmod/patch/livepatch-patch-hook.c b/kmod/patch/livepatch-patch-hook.c index 3d13ab97e..87c08f4d1 100644 --- a/kmod/patch/livepatch-patch-hook.c +++ b/kmod/patch/livepatch-patch-hook.c @@ -1,6 +1,6 @@ /* * Copyright (C) 2013-2014 Josh Poimboeuf - * Copyright (C) 2014 Seth Jennings + * Copyright (C) 2014 Seth Jennings * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -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)) || \ @@ -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; @@ -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; @@ -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; @@ -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;