Submitting your first patch to the Linux kernel

Finding things we can contribute to

Can’t think of anything? Documentation is always an option!

Creating a patch

Learning the guidelines for Linux kernel contributions

Creating a patch from a commit

commit 5ef3dd20555e8e878ac390a71e658db5fd02845c (HEAD)
Author: David Vernet <void@manifault.com>
Date: Tue Dec 21 07:39:31 2021 -0800

livepatch: Fix kobject refcount bug on klp_init_patch_early failure path

When enabling a klp patch with klp_enable_patch(), klp_init_patch_early()
is invoked to initialize the kobjects for the patch itself, as well as the
'struct klp_object' and 'struct klp_func' objects that comprise it.
However, there are some error paths in klp_enable_patch() where some
kobjects may have been initialized with kobject_init(), but an error code
is still returned due to e.g. a 'struct klp_object' having a NULL funcs
pointer.

...
<snip>
...

Signed-off-by: David Vernet <void@manifault.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 335d988bd811..7d228cdb44c5 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -862,14 +862,11 @@ static void klp_init_object_early(struct klp_patch *patch,
list_add_tail(&obj->node, &patch->obj_list);
}

-static int klp_init_patch_early(struct klp_patch *patch)
+static void klp_init_patch_early(struct klp_patch *patch)

<truncated>
...
$ git format-patch HEAD~1 -o /tmp
/tmp/0001-livepatch-Fix-kobject-refcount-bug-on-klp_init_patch.patch
$ cat /tmp/0001-livepatch-Fix-kobject-refcount-bug-on-klp_init_patch.patch
From 5ef3dd20555e8e878ac390a71e658db5fd02845c Mon Sep 17 00:00:00 2001
From: David Vernet <void@manifault.com>
Date: Tue, 21 Dec 2021 07:39:31 -0800
Subject: [PATCH] livepatch: Fix kobject refcount bug on klp_init_patch_early
failure path

When enabling a klp patch with klp_enable_patch(), klp_init_patch_early()
is invoked to initialize the kobjects for the patch itself, as well as the
'struct klp_object' and 'struct klp_func' objects that comprise it.
However, there are some error paths in klp_enable_patch() where some
kobjects may have been initialized with kobject_init(), but an error code
is still returned due to e.g. a 'struct klp_object' having a NULL funcs
pointer.

In these paths, the initial reference of the kobject of the 'struct
klp_patch' may never be released, along with one or more of its objects and
their functions, as kobject_put() is not invoked on the cleanup path if
klp_init_patch_early() returns an error code.

For example, if an object entry such as the following were added to the
sample livepatch module's klp patch, it would cause the vmlinux klp_object,
and its klp_func which updates 'cmdline_proc_show', to never be released:

static struct klp_object objs[] = {
{
/* name being NULL means vmlinux */
.funcs = funcs,
},
{
/* NULL funcs -- would cause reference leak */
.name = "kvm",
}, { }
};

Without this change, if CONFIG_DEBUG_KOBJECT is enabled, and the sample klp
patch is loaded, the kobjects (the patch, the vmlinux 'struct klp_object',
and its func) are observed as initialized, but never released, in the dmesg
log output. With the change, these kobject references no longer fail to be
released as the error case is properly handled before they are initialized.

Signed-off-by: David Vernet <void@manifault.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/livepatch/core.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 335d988bd811..7d228cdb44c5 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -862,14 +862,11 @@ static void klp_init_object_early(struct klp_patch *patch,
list_add_tail(&obj->node, &patch->obj_list);
}

-static int klp_init_patch_early(struct klp_patch *patch)
+static void klp_init_patch_early(struct klp_patch *patch)
{
struct klp_object *obj;
struct klp_func *func;

- if (!patch->objs)
- return -EINVAL;
-
INIT_LIST_HEAD(&patch->list);
INIT_LIST_HEAD(&patch->obj_list);
kobject_init(&patch->kobj, &klp_ktype_patch);
@@ -879,20 +876,12 @@ static int klp_init_patch_early(struct klp_patch *patch)
init_completion(&patch->finish);

klp_for_each_object_static(patch, obj) {
- if (!obj->funcs)
- return -EINVAL;
-
klp_init_object_early(patch, obj);

klp_for_each_func_static(obj, func) {
klp_init_func_early(obj, func);
}
}
-
- if (!try_module_get(patch->mod))
- return -ENODEV;
-
- return 0;
}

static int klp_init_patch(struct klp_patch *patch)
@@ -1024,10 +1013,17 @@ static int __klp_enable_patch(struct klp_patch *patch)
int klp_enable_patch(struct klp_patch *patch)
{
int ret;
+ struct klp_object *obj;

- if (!patch || !patch->mod)
+ if (!patch || !patch->mod || !patch->objs)
return -EINVAL;

+ klp_for_each_object_static(patch, obj) {
+ if (!obj->funcs)
+ return -EINVAL;
+ }
+
+
if (!is_livepatch_module(patch->mod)) {
pr_err("module %s is not marked as a livepatch module\n",
patch->mod->name);
@@ -1051,11 +1047,10 @@ int klp_enable_patch(struct klp_patch *patch)
return -EINVAL;
}

- ret = klp_init_patch_early(patch);
- if (ret) {
- mutex_unlock(&klp_mutex);
- return ret;
- }
+ if (!try_module_get(patch->mod))
+ return -ENODEV;
+
+ klp_init_patch_early(patch);

ret = klp_init_patch(patch);
if (ret)
--
2.25.1

Sanity checking the patch

$ ./scripts/checkpatch.pl /tmp/0001-livepatch-Fix-kobject-refcount-bug-on-klp_init_patch.patch

total: 0 errors, 0 warnings, 68 lines checked

/tmp/0001-livepatch-Fix-kobject-refcount-bug-on-klp_init_patch.patch has no obvious style problems and is ready for submission.

Sending the patch, and iterating with reviewers

Sending the patch

$ ./scripts/get_maintainer.pl /tmp/0001-livepatch-Fix-kobject-refcount-bug-on-klp_init_patch.patch

Josh Poimboeuf <jpoimboe@redhat.com> (maintainer:LIVE PATCHING)
Jiri Kosina <jikos@kernel.org> (maintainer:LIVE PATCHING)
Miroslav Benes <mbenes@suse.cz> (maintainer:LIVE PATCHING)
Petr Mladek <pmladek@suse.com> (maintainer:LIVE PATCHING)
Joe Lawrence <joe.lawrence@redhat.com> (reviewer:LIVE PATCHING)
live-patching@vger.kernel.org (open list:LIVE PATCHING)
linux-kernel@vger.kernel.org (open list)
$ sudo apt install git-email -y

$ git config --global sendemail.transferEncoding 7bit
$ git send-email --to jpoimboe@redhat.com --to jikos@kernel.org --to mbenes@suse.cz --to pmladek@suse.com --to joe.lawrence@redhat.com --to live-patching@vger.kernel.org --to linux-kernel@vger.kernel.org /tmp/0001-livepatch-Fix-kobject-refcount-bug-on-klp_init_patch.patch

From: David Vernet <void@manifault.com>
To: jpoimboe@redhat.com,
jikos@kernel.org,
mbenes@suse.cz,
pmladek@suse.com,
joe.lawrence@redhat.com,
live-patching@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: David Vernet <void@manifault.com>
Subject: [PATCH] livepatch: Fix kobject refcount bug on klp_init_patch_early failure path
Date: Sun, 6 Mar 2022 12:51:27 -0500
Message-Id: <20220306175127.5570-1-void@manifault.com>
X-Mailer: git-send-email 2.25.1
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit

Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): q

Interacting with reviewers

$ git format-patch HEAD~1 -v 2 -o /tmp
/tmp/v2-0001-livepatch-Fix-kobject-refcount-bug-on-klp_init_pa.patch
$ cat /tmp/v2-0001-livepatch-Fix-kobject-refcount-bug-on-klp_init_pa.patch | grep PATCH
Subject: [PATCH v2] livepatch: Fix kobject refcount bug on klp_init_patch_early
failure path

Quick aside — using kmail

Merging the patch

What to do if nobody responds to your email

Don’t be nervous, just do your best, and assume good intent

Wrapping up

--

--

--

Byte Lab is a technical blog that discusses systems engineering, and the tech industry.

Love podcasts or audiobooks? Learn on the go with our new app.

Recommended from Medium

Solving Single Source Shortest Path Using Dijkstra’s Algorithm

Google Maps Image

Building Power-x-Gym website

Engineering Mental Model — Analogy of a Restaurant Kitchen

My Simple COVID-19 SMS Updates with Twilio

AI with AWS SageMaker

Power-ups with Powercat

The Most Valuable Skill Every Software Developer Should Have

London Are you Python Expert? You should definitely know this uses of Asterisks then

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Byte Lab

Byte Lab

Byte Lab is a technical blog that discusses systems engineering, and the tech industry.

More from Medium

How to configure Two Node High Availability Cluster On RHEL/CentOS

Application vs Process vs Thread

Getting Started with Container Volumes and Persistent Storage

Running Docker Bench for Security to hardened your docker host and improve security