| 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
| ||
|
Message-ID: <20250109150631.2feaa55e@gandalf.local.home>
Date: Thu, 9 Jan 2025 15:06:31 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Linux Trace Kernel
<linux-trace-kernel@...r.kernel.org>, bpf <bpf@...r.kernel.org>, Masami
Hiramatsu <mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Andrew Morton
<akpm@...ux-foundation.org>, Peter Zijlstra <peterz@...radead.org>,
Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor
<nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, Zheng Yejian
<zhengyejian1@...wei.com>, Martin Kelly <martin.kelly@...wdstrike.com>,
Christophe Leroy <christophe.leroy@...roup.eu>, Josh Poimboeuf
<jpoimboe@...hat.com>, Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [PATCH v2] scripts/sorttable: Move code from sorttable.h into
sorttable.c
On Thu, 9 Jan 2025 10:24:05 -0800
Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> On Tue, 7 Jan 2025 at 19:30, Steven Rostedt <rostedt@...dmis.org> wrote:
> >
> > +
> > +static int (*compare_extable)(const void *a, const void *b);
> > +static uint64_t (*ehdr_shoff)(Elf_Ehdr *ehdr);
> > +static uint16_t (*ehdr_shstrndx)(Elf_Ehdr *ehdr);
> ...
>
> Side note - and independently of the pure code movement - wouldn't it
> be nice to just make this a structure of function pointers, and then
> instead of this:
I do that in some of my personal code, but then all the calls to the
functions tend to be:
e.ehdr_shoff(..); or e->ehdr_shoff(..);
instead of simply having:
ehdr_shoff(..);
Hmm, if the structure is global, I guess then we could just have the helper
functions use the global variable. But then, instead of having:
static int (*compare_extable)(const void *a, const void *b);
static uint64_t (*ehdr_shoff)(Elf_Ehdr *ehdr);
static uint16_t (*ehdr_shstrndx)(Elf_Ehdr *ehdr);
static uint16_t (*ehdr_shentsize)(Elf_Ehdr *ehdr);
static uint16_t (*ehdr_shnum)(Elf_Ehdr *ehdr);
static uint64_t (*shdr_addr)(Elf_Shdr *shdr);
static uint64_t (*shdr_offset)(Elf_Shdr *shdr);
static uint64_t (*shdr_size)(Elf_Shdr *shdr);
static uint64_t (*shdr_entsize)(Elf_Shdr *shdr);
static uint32_t (*shdr_link)(Elf_Shdr *shdr);
static uint32_t (*shdr_name)(Elf_Shdr *shdr);
static uint32_t (*shdr_type)(Elf_Shdr *shdr);
static uint8_t (*sym_type)(Elf_Sym *sym);
static uint32_t (*sym_name)(Elf_Sym *sym);
static uint64_t (*sym_value)(Elf_Sym *sym);
static uint16_t (*sym_shndx)(Elf_Sym *sym);
static uint64_t (*rela_offset)(Elf_Rela *rela);
static uint64_t (*rela_info)(Elf_Rela *rela);
static uint64_t (*rela_addend)(Elf_Rela *rela);
static void (*rela_write_addend)(Elf_Rela *rela, uint64_t val);
case ELFCLASS32:
compare_extable = compare_extable_32;
ehdr_shoff = ehdr32_shoff;
ehdr_shentsize = ehdr32_shentsize;
ehdr_shstrndx = ehdr32_shstrndx;
ehdr_shnum = ehdr32_shnum;
shdr_addr = shdr32_addr;
shdr_offset = shdr32_offset;
shdr_link = shdr32_link;
shdr_size = shdr32_size;
shdr_name = shdr32_name;
shdr_type = shdr32_type;
shdr_entsize = shdr32_entsize;
sym_type = sym32_type;
sym_name = sym32_name;
sym_value = sym32_value;
sym_shndx = sym32_shndx;
rela_offset = rela32_offset;
rela_info = rela32_info;
rela_addend = rela32_addend;
rela_write_addend = rela32_write_addend;
long_size = 4;
extable_ent_size = 8;
break;
case ELFCLASS64:
compare_extable = compare_extable_64;
ehdr_shoff = ehdr64_shoff;
ehdr_shentsize = ehdr64_shentsize;
ehdr_shstrndx = ehdr64_shstrndx;
ehdr_shnum = ehdr64_shnum;
shdr_addr = shdr64_addr;
shdr_offset = shdr64_offset;
shdr_link = shdr64_link;
shdr_size = shdr64_size;
shdr_name = shdr64_name;
shdr_type = shdr64_type;
shdr_entsize = shdr64_entsize;
sym_type = sym64_type;
sym_name = sym64_name;
sym_value = sym64_value;
sym_shndx = sym64_shndx;
rela_offset = rela64_offset;
rela_info = rela64_info;
rela_addend = rela64_addend;
rela_write_addend = rela64_write_addend;
long_size = 8;
extable_ent_size = 16;
break;
We would have:
static struct elf_funcs {
int (*compare_extable)(const void *a, const void *b);
uint64_t (*ehdr_shoff)(Elf_Ehdr *ehdr);
uint16_t (*ehdr_shstrndx)(Elf_Ehdr *ehdr);
uint16_t (*ehdr_shentsize)(Elf_Ehdr *ehdr);
uint16_t (*ehdr_shnum)(Elf_Ehdr *ehdr);
uint64_t (*shdr_addr)(Elf_Shdr *shdr);
uint64_t (*shdr_offset)(Elf_Shdr *shdr);
uint64_t (*shdr_size)(Elf_Shdr *shdr);
uint64_t (*shdr_entsize)(Elf_Shdr *shdr);
uint32_t (*shdr_link)(Elf_Shdr *shdr);
uint32_t (*shdr_name)(Elf_Shdr *shdr);
uint32_t (*shdr_type)(Elf_Shdr *shdr);
uint8_t (*sym_type)(Elf_Sym *sym);
uint32_t (*sym_name)(Elf_Sym *sym);
uint64_t (*sym_value)(Elf_Sym *sym);
uint16_t (*sym_shndx)(Elf_Sym *sym);
} e;
// Hmm, I could add the helper function into the macros:
#define EHDR_HALF(fn_name) \
static uint16_t ehdr64_##fn_name(Elf_Ehdr *ehdr) \
{ \
return r2(&ehdr->e64.e_##fn_name); \
} \
\
static uint16_t ehdr32_##fn_name(Elf_Ehdr *ehdr) \
{ \
return r2(&ehdr->e32.e_##fn_name); \
} \
\
static uint16 ehdr_##fn_name(Elf_Ehdr *ehdr) \
{ \
return e.ehdr_##fn_name(ehdr); \
}
[..]
case ELFCLASS32: {
struct elf_funcs efuncs = {
.compare_extable = compare_extable_32,
.ehdr_shoff = ehdr32_shoff,
.ehdr_shentsize = ehdr32_shentsize,
[..]
};
e = efuncs;
long_size = 4;
extable_ent_size = 8;
}
break;
case ELFCLASS64: {
struct elf_funcs efuncs = {
.compare_extable = compare_extable_64,
.ehdr_shoff = ehdr64_shoff,
.ehdr_shentsize = ehdr64_shentsize,
[..]
};
e = efuncs;
long_size = 8;
extable_ent_size = 16;
}
break;
Which would give me this patch on top of this:
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index 656c1e9b5ad9..9f41575afd7a 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -85,6 +85,25 @@ static uint64_t (*r8)(const uint64_t *);
static void (*w)(uint32_t, uint32_t *);
typedef void (*table_sort_t)(char *, int);
+static struct elf_funcs {
+ int (*compare_extable)(const void *a, const void *b);
+ uint64_t (*ehdr_shoff)(Elf_Ehdr *ehdr);
+ uint16_t (*ehdr_shstrndx)(Elf_Ehdr *ehdr);
+ uint16_t (*ehdr_shentsize)(Elf_Ehdr *ehdr);
+ uint16_t (*ehdr_shnum)(Elf_Ehdr *ehdr);
+ uint64_t (*shdr_addr)(Elf_Shdr *shdr);
+ uint64_t (*shdr_offset)(Elf_Shdr *shdr);
+ uint64_t (*shdr_size)(Elf_Shdr *shdr);
+ uint64_t (*shdr_entsize)(Elf_Shdr *shdr);
+ uint32_t (*shdr_link)(Elf_Shdr *shdr);
+ uint32_t (*shdr_name)(Elf_Shdr *shdr);
+ uint32_t (*shdr_type)(Elf_Shdr *shdr);
+ uint8_t (*sym_type)(Elf_Sym *sym);
+ uint32_t (*sym_name)(Elf_Sym *sym);
+ uint64_t (*sym_value)(Elf_Sym *sym);
+ uint16_t (*sym_shndx)(Elf_Sym *sym);
+} e;
+
static uint64_t ehdr64_shoff(Elf_Ehdr *ehdr)
{
return r8(&ehdr->e64.e_shoff);
@@ -95,6 +114,11 @@ static uint64_t ehdr32_shoff(Elf_Ehdr *ehdr)
return r(&ehdr->e32.e_shoff);
}
+static uint64_t ehdr_shoff(Elf_Ehdr *ehdr)
+{
+ return e.ehdr_shoff(ehdr);
+}
+
#define EHDR_HALF(fn_name) \
static uint16_t ehdr64_##fn_name(Elf_Ehdr *ehdr) \
{ \
@@ -104,6 +128,11 @@ static uint16_t ehdr64_##fn_name(Elf_Ehdr *ehdr) \
static uint16_t ehdr32_##fn_name(Elf_Ehdr *ehdr) \
{ \
return r2(&ehdr->e32.e_##fn_name); \
+} \
+ \
+static uint16_t ehdr_##fn_name(Elf_Ehdr *ehdr) \
+{ \
+ return e.ehdr_##fn_name(ehdr); \
}
EHDR_HALF(shentsize)
@@ -119,6 +148,11 @@ static uint32_t shdr64_##fn_name(Elf_Shdr *shdr) \
static uint32_t shdr32_##fn_name(Elf_Shdr *shdr) \
{ \
return r(&shdr->e32.sh_##fn_name); \
+} \
+ \
+static uint32_t shdr_##fn_name(Elf_Shdr *shdr) \
+{ \
+ return e.shdr_##fn_name(shdr); \
}
#define SHDR_ADDR(fn_name) \
@@ -130,6 +164,11 @@ static uint64_t shdr64_##fn_name(Elf_Shdr *shdr) \
static uint64_t shdr32_##fn_name(Elf_Shdr *shdr) \
{ \
return r(&shdr->e32.sh_##fn_name); \
+} \
+ \
+static uint64_t shdr_##fn_name(Elf_Shdr *shdr) \
+{ \
+ return e.shdr_##fn_name(shdr); \
}
#define SHDR_WORD(fn_name) \
@@ -141,6 +180,10 @@ static uint32_t shdr64_##fn_name(Elf_Shdr *shdr) \
static uint32_t shdr32_##fn_name(Elf_Shdr *shdr) \
{ \
return r(&shdr->e32.sh_##fn_name); \
+} \
+static uint32_t shdr_##fn_name(Elf_Shdr *shdr) \
+{ \
+ return e.shdr_##fn_name(shdr); \
}
SHDR_ADDR(addr)
@@ -161,6 +204,11 @@ static uint64_t sym64_##fn_name(Elf_Sym *sym) \
static uint64_t sym32_##fn_name(Elf_Sym *sym) \
{ \
return r(&sym->e32.st_##fn_name); \
+} \
+ \
+static uint64_t sym_##fn_name(Elf_Sym *sym) \
+{ \
+ return e.sym_##fn_name(sym); \
}
#define SYM_WORD(fn_name) \
@@ -172,6 +220,11 @@ static uint32_t sym64_##fn_name(Elf_Sym *sym) \
static uint32_t sym32_##fn_name(Elf_Sym *sym) \
{ \
return r(&sym->e32.st_##fn_name); \
+} \
+ \
+static uint32_t sym_##fn_name(Elf_Sym *sym) \
+{ \
+ return e.sym_##fn_name(sym); \
}
#define SYM_HALF(fn_name) \
@@ -183,6 +236,11 @@ static uint16_t sym64_##fn_name(Elf_Sym *sym) \
static uint16_t sym32_##fn_name(Elf_Sym *sym) \
{ \
return r2(&sym->e32.st_##fn_name); \
+} \
+ \
+static uint16_t sym_##fn_name(Elf_Sym *sym) \
+{ \
+ return e.sym_##fn_name(sym); \
}
static uint8_t sym64_type(Elf_Sym *sym)
@@ -195,6 +253,11 @@ static uint8_t sym32_type(Elf_Sym *sym)
return ELF32_ST_TYPE(sym->e32.st_info);
}
+static uint8_t sym_type(Elf_Sym *sym)
+{
+ return e.sym_type(sym);
+}
+
SYM_ADDR(value)
SYM_WORD(name)
SYM_HALF(shndx)
@@ -322,29 +385,16 @@ static int compare_extable_64(const void *a, const void *b)
return av > bv;
}
+static int compare_extable(const void *a, const void *b)
+{
+ return e.compare_extable(a, b);
+}
+
static inline void *get_index(void *start, int entsize, int index)
{
return start + (entsize * index);
}
-
-static int (*compare_extable)(const void *a, const void *b);
-static uint64_t (*ehdr_shoff)(Elf_Ehdr *ehdr);
-static uint16_t (*ehdr_shstrndx)(Elf_Ehdr *ehdr);
-static uint16_t (*ehdr_shentsize)(Elf_Ehdr *ehdr);
-static uint16_t (*ehdr_shnum)(Elf_Ehdr *ehdr);
-static uint64_t (*shdr_addr)(Elf_Shdr *shdr);
-static uint64_t (*shdr_offset)(Elf_Shdr *shdr);
-static uint64_t (*shdr_size)(Elf_Shdr *shdr);
-static uint64_t (*shdr_entsize)(Elf_Shdr *shdr);
-static uint32_t (*shdr_link)(Elf_Shdr *shdr);
-static uint32_t (*shdr_name)(Elf_Shdr *shdr);
-static uint32_t (*shdr_type)(Elf_Shdr *shdr);
-static uint8_t (*sym_type)(Elf_Sym *sym);
-static uint32_t (*sym_name)(Elf_Sym *sym);
-static uint64_t (*sym_value)(Elf_Sym *sym);
-static uint16_t (*sym_shndx)(Elf_Sym *sym);
-
static int extable_ent_size;
static int long_size;
@@ -864,7 +914,30 @@ static int do_file(char const *const fname, void *addr)
}
switch (ehdr->e32.e_ident[EI_CLASS]) {
- case ELFCLASS32:
+ case ELFCLASS32: {
+ struct elf_funcs efuncs = {
+ .compare_extable = compare_extable_32,
+ .ehdr_shoff = ehdr32_shoff,
+ .ehdr_shentsize = ehdr32_shentsize,
+ .ehdr_shstrndx = ehdr32_shstrndx,
+ .ehdr_shnum = ehdr32_shnum,
+ .shdr_addr = shdr32_addr,
+ .shdr_offset = shdr32_offset,
+ .shdr_link = shdr32_link,
+ .shdr_size = shdr32_size,
+ .shdr_name = shdr32_name,
+ .shdr_type = shdr32_type,
+ .shdr_entsize = shdr32_entsize,
+ .sym_type = sym32_type,
+ .sym_name = sym32_name,
+ .sym_value = sym32_value,
+ .sym_shndx = sym32_shndx,
+ };
+
+ e = efuncs;
+ long_size = 4;
+ extable_ent_size = 8;
+
if (r2(&ehdr->e32.e_ehsize) != sizeof(Elf32_Ehdr) ||
r2(&ehdr->e32.e_shentsize) != sizeof(Elf32_Shdr)) {
fprintf(stderr,
@@ -872,26 +945,32 @@ static int do_file(char const *const fname, void *addr)
return -1;
}
- compare_extable = compare_extable_32;
- ehdr_shoff = ehdr32_shoff;
- ehdr_shentsize = ehdr32_shentsize;
- ehdr_shstrndx = ehdr32_shstrndx;
- ehdr_shnum = ehdr32_shnum;
- shdr_addr = shdr32_addr;
- shdr_offset = shdr32_offset;
- shdr_link = shdr32_link;
- shdr_size = shdr32_size;
- shdr_name = shdr32_name;
- shdr_type = shdr32_type;
- shdr_entsize = shdr32_entsize;
- sym_type = sym32_type;
- sym_name = sym32_name;
- sym_value = sym32_value;
- sym_shndx = sym32_shndx;
- long_size = 4;
- extable_ent_size = 8;
+ }
break;
- case ELFCLASS64:
+ case ELFCLASS64: {
+ struct elf_funcs efuncs = {
+ .compare_extable = compare_extable_64,
+ .ehdr_shoff = ehdr64_shoff,
+ .ehdr_shentsize = ehdr64_shentsize,
+ .ehdr_shstrndx = ehdr64_shstrndx,
+ .ehdr_shnum = ehdr64_shnum,
+ .shdr_addr = shdr64_addr,
+ .shdr_offset = shdr64_offset,
+ .shdr_link = shdr64_link,
+ .shdr_size = shdr64_size,
+ .shdr_name = shdr64_name,
+ .shdr_type = shdr64_type,
+ .shdr_entsize = shdr64_entsize,
+ .sym_type = sym64_type,
+ .sym_name = sym64_name,
+ .sym_value = sym64_value,
+ .sym_shndx = sym64_shndx,
+ };
+
+ e = efuncs;
+ long_size = 8;
+ extable_ent_size = 16;
+
if (r2(&ehdr->e64.e_ehsize) != sizeof(Elf64_Ehdr) ||
r2(&ehdr->e64.e_shentsize) != sizeof(Elf64_Shdr)) {
fprintf(stderr,
@@ -900,25 +979,7 @@ static int do_file(char const *const fname, void *addr)
return -1;
}
- compare_extable = compare_extable_64;
- ehdr_shoff = ehdr64_shoff;
- ehdr_shentsize = ehdr64_shentsize;
- ehdr_shstrndx = ehdr64_shstrndx;
- ehdr_shnum = ehdr64_shnum;
- shdr_addr = shdr64_addr;
- shdr_offset = shdr64_offset;
- shdr_link = shdr64_link;
- shdr_size = shdr64_size;
- shdr_name = shdr64_name;
- shdr_type = shdr64_type;
- shdr_entsize = shdr64_entsize;
- sym_type = sym64_type;
- sym_name = sym64_name;
- sym_value = sym64_value;
- sym_shndx = sym64_shndx;
- long_size = 8;
- extable_ent_size = 16;
-
+ }
break;
default:
fprintf(stderr, "unrecognized ELF class %d %s\n",
Is that what you are thinking?
-- Steve
Powered by blists - more mailing lists