public inbox for live-patching@vger.kernel.org
 help / color / mirror / Atom feed
From: Joe Lawrence <joe.lawrence@redhat.com>
To: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: live-patching@vger.kernel.org, Song Liu <song@kernel.org>,
	Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
	Petr Mladek <pmladek@suse.com>
Subject: Re: [PATCH v3 01/13] objtool/klp: honor SHF_MERGE entry alignment in elf_add_data()
Date: Tue, 10 Mar 2026 10:02:58 -0400	[thread overview]
Message-ID: <abAkkrWoTao_tIi7@redhat.com> (raw)
In-Reply-To: <p67ixebt5ufjed44j6wrufwihmsh3ufhbdog7767ro6tygleaw@lvp55v6brjxw>

On Wed, Mar 04, 2026 at 06:33:47PM -0800, Josh Poimboeuf wrote:
> On Tue, Feb 17, 2026 at 11:14:17AM -0500, Joe Lawrence wrote:
> > On Tue, Feb 17, 2026 at 11:06:32AM -0500, Joe Lawrence wrote:
> > > When adding data to an SHF_MERGE section, set the Elf_Data d_align to
> > > the section's sh_addralign so libelf aligns entries within the section.
> > > This ensures that entry offsets are consistent with previously calculated
> > > relocation addends.
> > > 
> > > Fixes: 431dbabf2d9d ("objtool: Add elf_create_data()")
> > > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> > > ---
> > >  tools/objtool/elf.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> > > index 2c02c7b49265..bd6502e7bdc0 100644
> > > --- a/tools/objtool/elf.c
> > > +++ b/tools/objtool/elf.c
> > > @@ -1375,7 +1375,7 @@ void *elf_add_data(struct elf *elf, struct section *sec, const void *data, size_
> > >  		memcpy(sec->data->d_buf, data, size);
> > >  
> > >  	sec->data->d_size = size;
> > > -	sec->data->d_align = 1;
> > > +	sec->data->d_align = (sec->sh.sh_flags & SHF_MERGE) ? sec->sh.sh_addralign : 1;
> > >  
> > >  	offset = ALIGN(sec->sh.sh_size, sec->sh.sh_addralign);
> > >  	sec->sh.sh_size = offset + size;
> > > -- 
> > > 2.53.0
> > > 
> > > 
> > 
> > This one stretches my ELF internals knowledge a bit, is ^^ true or
> > should we rely on the section ".str1.8" suffix to indicate internal
> > alignment?
> 
> I hit the same issue in my testing for the klp-build arm64 port, I think
> it can be simplified to the below?
> 
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> @@ -1375,7 +1382,7 @@ void *elf_add_data(struct elf *elf, struct section *sec, const void *data, size_
>  		memcpy(sec->data->d_buf, data, size);
>  
>  	sec->data->d_size = size;
> -	sec->data->d_align = 1;
> +	sec->data->d_align = sec->sh.sh_addralign;
>  
>  	offset = ALIGN(sec->sh.sh_size, sec->sh.sh_addralign);
>  	sec->sh.sh_size = offset + size;
> 

Yeah that make sense, but I think we both missed the second half of the
fix in the symbol table.  This patch fails with your version:

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

diff --git a/fs/proc/stat.c b/fs/proc/stat.c
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -54,6 +54,13 @@ static u64 get_iowait_time(struct kernel_cpustat *kcs, int cpu)
 	return iowait;
 }
 
+static void klp_test_stat_accessed(void)
+{
+	static atomic_t call_count = ATOMIC_INIT(0);
+	atomic_inc(&call_count);
+	pr_info("klp-build-test: stat accessed %d times\n", atomic_read(&call_count));
+}
+
 static int show_stat(struct seq_file *p, void *v)
 {
 	int i, j;
@@ -92,6 +99,7 @@ static int show_stat(struct seq_file *p, void *v)
 	user = nice = system = idle = iowait =
 		irq = softirq = steal = 0;
 	guest = guest_nice = 0;
+	klp_test_stat_accessed();
 	getboottime64(&boottime);
 	/* shift boot timestamp according to the timens offset */
 	timens_sub_boottime(&boottime);

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

Confused on instruction decoding?

  livepatch-test.o: warning: objtool: show_stat+0x696: can't find jump dest instruction at .rodata.show_stat.str1.1+0x38
  make[3]: *** [/root/linux/scripts/Makefile.build:505: livepatch-test.o] Error 255
  make[3]: *** Deleting file 'livepatch-test.o'
  make[2]: *** [/root/linux/Makefile:2064: .] Error 2
  make[1]: *** [/root/linux/Makefile:248: __sub-make] Error 2
  make: *** [Makefile:248: __sub-make] Error 2
  error: klp-build: line 793: '"${cmd[@]}" > >(tee -a "$log") 2> >(tee -a "$log" 1>&2)'
  error: klp-build: line 795: '( cd "$SRC"; "${cmd[@]}" > >(tee -a "$log") 2> >(tee -a "$log" 1>&2) )'

Notice the 0x10 section offset value for show_stat in the generated
vmlinux.o diff object symbol table:

  $ readelf --wide --symbols klp-tmp/diff/vmlinux.o | grep -E 'Ndx|\.text\.show_stat$|show_stat$|pfx_show_stat$'
     Num:    Value          Size Type    Bind   Vis      Ndx Name
       1: 0000000000000000    16 FUNC    LOCAL  DEFAULT    4 __pfx_show_stat
       2: 0000000000000010  2126 FUNC    LOCAL  DEFAULT    4 show_stat
       5: 0000000000000000     0 SECTION LOCAL  DEFAULT    4 .text.show_stat

but then in the disassembly, show_stat() starts 0x20 bytes into the
section:

  $ objdump -d -j .text.show_stat --start-address=0x0 --stop-address=0x24 klp-tmp/diff/vmlinux.o
  
  klp-tmp/diff/vmlinux.o:     file format elf64-x86-64
  
  
  Disassembly of section .text.show_stat:
  
  0000000000000000 <__pfx_show_stat>:
     0:   90                      nop
     1:   90                      nop
     2:   90                      nop
     3:   90                      nop
     4:   90                      nop
     5:   90                      nop
     6:   90                      nop
     7:   90                      nop
     8:   90                      nop
     9:   90                      nop
     a:   90                      nop
     b:   90                      nop
     c:   90                      nop
     d:   90                      nop
     e:   90                      nop
     f:   90                      nop
  
  0000000000000010 <show_stat>:
          ...
    20:   f3 0f 1e fa             endbr64


When cloning the symbol, if we ensure the symbol's offset aligns
with the sh_addralign (as elf_add_data() does now):

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index a3198a63c2..c2c4e4968b 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -14,6 +14,7 @@
 #include <objtool/util.h>
 #include <arch/special.h>
 
+#include <linux/align.h>
 #include <linux/objtool_types.h>
 #include <linux/livepatch_external.h>
 #include <linux/stringify.h>
@@ -560,7 +561,7 @@ static struct symbol *__clone_symbol(struct elf *elf, struct symbol *patched_sym
 		}
 
 		if (!is_sec_sym(patched_sym))
-			offset = sec_size(out_sec);
+			offset = ALIGN(sec_size(out_sec), out_sec->sh.sh_addralign);
 
 		if (patched_sym->len || is_sec_sym(patched_sym)) {
 			void *data = NULL;

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

Results are as expected:

  $ readelf --wide --symbols klp-tmp/diff/vmlinux.o | grep -E 'Ndx|\.text\.show_stat$|show_stat$|pfx_show_stat$'
     Num:    Value          Size Type    Bind   Vis      Ndx Name
       1: 0000000000000000    16 FUNC    LOCAL  DEFAULT    4 __pfx_show_stat
       2: 0000000000000020  2126 FUNC    LOCAL  DEFAULT    4 show_stat
       5: 0000000000000000     0 SECTION LOCAL  DEFAULT    4 .text.show_stat

$ objdump -d -j .text.show_stat --start-address=0x0 --stop-address=0x24 klp-tmp/diff/vmlinux.o 

klp-tmp/diff/vmlinux.o:     file format elf64-x86-64


Disassembly of section .text.show_stat:

0000000000000000 <__pfx_show_stat>:
   0:   90                      nop
   1:   90                      nop
   2:   90                      nop
   3:   90                      nop
   4:   90                      nop
   5:   90                      nop
   6:   90                      nop
   7:   90                      nop
   8:   90                      nop
   9:   90                      nop
   a:   90                      nop
   b:   90                      nop
   c:   90                      nop
   d:   90                      nop
   e:   90                      nop
   f:   90                      nop
        ...

0000000000000020 <show_stat>:
  20:   f3 0f 1e fa             endbr64


LMK if you want me to update the patch in this set, or drop it here so
you can update in ("objtool/arm64: Port klp-build to arm64").

Thanks,

--
Joe


  reply	other threads:[~2026-03-10 14:03 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-17 16:06 [PATCH v3 00/13] livepatch-klp-build: small fixups and enhancements Joe Lawrence
2026-02-17 16:06 ` [PATCH v3 01/13] objtool/klp: honor SHF_MERGE entry alignment in elf_add_data() Joe Lawrence
2026-02-17 16:14   ` Joe Lawrence
2026-03-05  2:33     ` Josh Poimboeuf
2026-03-10 14:02       ` Joe Lawrence [this message]
2026-03-10 16:34         ` Josh Poimboeuf
2026-02-17 16:06 ` [PATCH v3 02/13] objtool/klp: fix mkstemp() failure with long paths Joe Lawrence
2026-02-17 17:45   ` Song Liu
2026-02-17 16:06 ` [PATCH v3 03/13] livepatch/klp-build: support patches that add/remove files Joe Lawrence
2026-02-17 17:52   ` Song Liu
2026-02-17 16:06 ` [PATCH v3 04/13] livepatch/klp-build: switch to GNU patch and recountdiff Joe Lawrence
2026-02-17 16:06 ` [PATCH v3 05/13] livepatch/klp-build: add grep-override function Joe Lawrence
2026-02-17 18:22   ` Song Liu
2026-02-17 16:06 ` [PATCH v3 06/13] livepatch/klp-build: add Makefile with check target Joe Lawrence
2026-02-17 18:26   ` Song Liu
2026-02-17 16:06 ` [PATCH v3 07/13] livepatch/klp-build: fix shellcheck complaints Joe Lawrence
2026-02-17 18:30   ` Song Liu
2026-02-17 16:06 ` [PATCH v3 08/13] livepatch/klp-build: improve short-circuit validation Joe Lawrence
2026-02-17 18:29   ` Song Liu
2026-02-17 16:06 ` [PATCH v3 09/13] livepatch/klp-build: fix version mismatch when short-circuiting Joe Lawrence
2026-02-17 16:17   ` Joe Lawrence
2026-02-17 19:25     ` Song Liu
2026-02-18 15:04       ` Joe Lawrence
2026-02-19  2:52         ` Song Liu
2026-02-23 21:13     ` Josh Poimboeuf
2026-03-03  2:20       ` Joe Lawrence
2026-03-05 22:35         ` Josh Poimboeuf
2026-03-05 22:52           ` [PATCH] klp-build: Fix inconsistent kernel version Josh Poimboeuf
2026-03-10 13:45             ` Joe Lawrence
2026-03-10 16:30               ` Josh Poimboeuf
2026-02-17 16:06 ` [PATCH v3 10/13] livepatch/klp-build: provide friendlier error messages Joe Lawrence
2026-02-17 18:32   ` Song Liu
2026-02-23 21:15   ` Josh Poimboeuf
2026-03-03  2:20     ` Joe Lawrence
2026-02-17 16:06 ` [PATCH v3 11/13] livepatch/klp-build: add terminal color output Joe Lawrence
2026-02-17 16:20   ` Joe Lawrence
2026-02-17 18:45   ` Song Liu
2026-02-23 21:28   ` Josh Poimboeuf
2026-02-23 21:32     ` Josh Poimboeuf
2026-03-03  2:24       ` Joe Lawrence
2026-03-05 20:08         ` Josh Poimboeuf
2026-02-17 16:06 ` [PATCH v3 12/13] livepatch/klp-build: report patch validation drift Joe Lawrence
2026-02-17 19:42   ` Song Liu
2026-02-18 15:09     ` Joe Lawrence
2026-02-23 21:40   ` Josh Poimboeuf
2026-03-03  2:27     ` Joe Lawrence
2026-02-17 16:06 ` [PATCH v3 13/13] livepatch/klp-build: don't look for changed objects in tools/ Joe Lawrence
2026-02-17 19:29   ` Song Liu
2026-02-18 15:18     ` Joe Lawrence
2026-02-19  2:55       ` Song Liu
2026-02-23 21:41   ` Josh Poimboeuf
2026-03-03  2:30     ` Joe Lawrence
2026-03-05 20:10       ` Josh Poimboeuf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=abAkkrWoTao_tIi7@redhat.com \
    --to=joe.lawrence@redhat.com \
    --cc=jikos@kernel.org \
    --cc=jpoimboe@kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=pmladek@suse.com \
    --cc=song@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox