Re: [PATCH v2 52/62] objtool/klp: Introduce klp diff subcommand for diffing object files

From: Joe Lawrence
Date: Mon Jun 09 2025 - 14:32:40 EST


On Fri, May 09, 2025 at 01:17:16PM -0700, Josh Poimboeuf wrote:
> +static int validate_ffunction_fdata_sections(struct elf *elf)
> +{
> + struct symbol *sym;
> + bool found_text = false, found_data = false;
> +
> + for_each_sym(elf, sym) {
> + char sec_name[SEC_NAME_LEN];
> +
> + if (!found_text && is_func_sym(sym)) {
> + snprintf(sec_name, SEC_NAME_LEN, ".text.%s", sym->name);
> + if (!strcmp(sym->sec->name, sec_name))
> + found_text = true;
> + }
> +
> + if (!found_data && is_object_sym(sym)) {
> + snprintf(sec_name, SEC_NAME_LEN, ".data.%s", sym->name);
> + if (!strcmp(sym->sec->name, sec_name))
> + found_data = true;

Hi Josh,

Should we check for other data section prefixes here, like:

else {
snprintf(sec_name, SEC_NAME_LEN, ".rodata.%s", sym->name);
if (!strcmp(sym->sec->name, sec_name))
found_data = true;
}

because on my system, I tried patching net/netfilter/nft_tunnel.c, but
if I look at its OBJECTS and their corresponding sections, I see:

24: 0000000000000000 24 OBJECT LOCAL DEFAULT 71 __msg.92
[71] .rodata.__msg.92 PROGBITS

43: 000000000000003f 22 OBJECT LOCAL DEFAULT 72 __UNIQUE_ID_alias_1010
42: 000000000000002f 16 OBJECT LOCAL DEFAULT 72 __UNIQUE_ID_alias_1011
44: 0000000000000055 47 OBJECT LOCAL DEFAULT 72 __UNIQUE_ID_author_1009
41: 0000000000000000 47 OBJECT LOCAL DEFAULT 72 __UNIQUE_ID_description_1012
45: 0000000000000084 12 OBJECT LOCAL DEFAULT 72 __UNIQUE_ID_license_1008
[72] .modinfo

47: 0000000000000000 8 OBJECT LOCAL DEFAULT 73 __UNIQUE_ID_addressable_cleanup_module_1007
[73] .exit.data

49: 0000000000000000 8 OBJECT LOCAL DEFAULT 75 __UNIQUE_ID_addressable_init_module_1006
[72] .modinfo

51: 0000000000000000 56 OBJECT LOCAL DEFAULT 77 nft_tunnel_obj_ops
[77] .rodata.nft_tunnel_obj_ops

5: 0000000000000000 64 OBJECT LOCAL DEFAULT 79 nft_tunnel_obj_type
4: 0000000000000040 80 OBJECT LOCAL DEFAULT 79 nft_tunnel_type
[79] .data..read_mostly

53: 0000000000000000 160 OBJECT LOCAL DEFAULT 81 nft_tunnel_key_policy
[81] .rodata.nft_tunnel_key_policy

30: 0000000000000000 64 OBJECT LOCAL DEFAULT 82 nft_tunnel_opts_policy
[82] .rodata.nft_tunnel_opts_policy

23: 0000000000000000 64 OBJECT LOCAL DEFAULT 83 nft_tunnel_opts_geneve_policy
[83] .rodata.nft_tunnel_opts_geneve_policy

32: 0000000000000000 80 OBJECT LOCAL DEFAULT 84 nft_tunnel_opts_erspan_policy
[84] .rodata.nft_tunnel_opts_erspan_policy

31: 0000000000000000 32 OBJECT LOCAL DEFAULT 85 nft_tunnel_opts_vxlan_policy
[85] .rodata.nft_tunnel_opts_vxlan_policy

20: 0000000000000000 64 OBJECT LOCAL DEFAULT 86 nft_tunnel_ip6_policy
[86] .rodata.nft_tunnel_ip6_policy

29: 0000000000000000 48 OBJECT LOCAL DEFAULT 87 nft_tunnel_ip_policy
[87] .rodata.nft_tunnel_ip_policy

62: 0000000000000000 136 OBJECT LOCAL DEFAULT 88 nft_tunnel_get_ops
[88] .rodata.nft_tunnel_get_ops

64: 0000000000000000 64 OBJECT LOCAL DEFAULT 91 nft_tunnel_policy
[91] .rodata.nft_tunnel_policy

I believe there are others like this, drivers/firmware/iscsi_ibft.o for
one, so even though validate_ffunction_fdata_sections() only needs to
find one .text.<section> and one .data.<section>, not all objects may be
able to provide that.

At the same time, while we're here, what about other .text.* section
prefixes?

--
Joe