linux-kernel - Re: [PATCH v4 51/63] objtool/klp: Introduce klp diff subcommand for diffing object files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <bnipx2pvsyxcd27uhxw5rr5yugm7iuint6rg3lzt3hdm7vkeue@g3wzb7kyr5da>
Date: Wed, 8 Oct 2025 08:27:45 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Petr Mladek <pmladek@...e.com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, 
	Miroslav Benes <mbenes@...e.cz>, Joe Lawrence <joe.lawrence@...hat.com>, 
	live-patching@...r.kernel.org, Song Liu <song@...nel.org>, laokz <laokz@...mail.com>, 
	Jiri Kosina <jikos@...nel.org>, Marcos Paulo de Souza <mpdesouza@...e.com>, 
	Weinan Liu <wnliu@...gle.com>, Fazla Mehrab <a.mehrab@...edance.com>, 
	Chen Zhongjin <chenzhongjin@...wei.com>, Puranjay Mohan <puranjay@...nel.org>, 
	Dylan Hatch <dylanbhatch@...gle.com>, Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v4 51/63] objtool/klp: Introduce klp diff subcommand for
 diffing object files

On Wed, Oct 08, 2025 at 04:01:50PM +0200, Petr Mladek wrote:
> On Wed 2025-09-17 09:03:59, Josh Poimboeuf wrote:
> > +static int read_exports(void)
> > +{
> > +	const char *symvers = "Module.symvers";
> > +	char line[1024], *path = NULL;
> > +	unsigned int line_num = 1;
> > +	FILE *file;
> > +
> > +	file = fopen(symvers, "r");
> > +	if (!file) {
> > +		path = top_level_dir(symvers);
> > +		if (!path) {
> > +			ERROR("can't open '%s', \"objtool diff\" should be run from the kernel tree", symvers);
> > +			return -1;
> > +		}
> > +
> > +		file = fopen(path, "r");
> > +		if (!file) {
> > +			ERROR_GLIBC("fopen");
> > +			return -1;
> > +		}
> > +	}
> > +
> > +	while (fgets(line, 1024, file)) {
> 
> Nit: It might be more safe to replace 1024 with sizeof(line).
>      It might be fixed later in a separate patch.

Indeed.

> > +/*
> > + * Klp relocations aren't allowed for __jump_table and .static_call_sites if
> > + * the referenced symbol lives in a kernel module, because such klp relocs may
> > + * be applied after static branch/call init, resulting in code corruption.
> > + *
> > + * Validate a special section entry to avoid that.  Note that an inert
> > + * tracepoint is harmless enough, in that case just skip the entry and print a
> > + * warning.  Otherwise, return an error.
> > + *
> > + * This is only a temporary limitation which will be fixed when livepatch adds
> > + * support for submodules: fully self-contained modules which are embedded in
> > + * the top-level livepatch module's data and which can be loaded on demand when
> > + * their corresponding to-be-patched module gets loaded.  Then klp relocs can
> > + * be retired.
> 
> I wonder how temporary this is ;-) The comment looks optimistic. I am
> just curious. Do you have any plans to implement the support for
> the submodules... ?

I actually already have a working POC for that, but didn't want to make
the patch set even longer ;-)

It was surprisingly easy and straightforward to implement.

> PS: To make some expectations. I am not doing a deep review.
>     I am just looking at the patchset to see how far and mature
>     it is. And I just comment what catches my eye.
> 
>     My first impression is that it is already in a pretty good state.
>     And I do not see any big problem there. Well, some documentation
>     would be fine ;-)
> 
>     What are your plans, please?

>From my perspective, it's testing well and in a good enough state for
merging soon (after the merge window?), if there aren't any objections
to that.

There will be more patches to come, like the submodules and other arch
support.  And of course there will be bugs discovered by broader
testing.  But I think this is a good foundation to begin with.

And the sooner we can get people using this, the sooner we can start
deprecating kpatch-build, which would be really nice.

-- 
Josh