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

[Joe Lawrence <joe.lawrence@redhat.com>]

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