Skip to content

btf : Fix union/anonymous struct resolution#4973

Merged
kkourt merged 2 commits into
cilium:mainfrom
tdaudi:pr/tdaudi/fix-resolve-union
May 11, 2026
Merged

btf : Fix union/anonymous struct resolution#4973
kkourt merged 2 commits into
cilium:mainfrom
tdaudi:pr/tdaudi/fix-resolve-union

Conversation

@tdaudi
Copy link
Copy Markdown
Contributor

@tdaudi tdaudi commented May 8, 2026

Fixes #4974

Description

Since the resolve algorithm was implemented, Union and anonymous struct types were incorrectly resolved. The reason for this is that the final element found offset were written on top on the union/anonymous struct.

With this fix, the offset of the union/anonymous struct is incremented only when the final element matches.

No test were added because one already exist for union/anonymous struct. Therefore I just updated the logic of the test because this one was also broken.

Example

With the following policy

kprobes:
  - call: "security_key_alloc"
    syscall: false
    args:
    - index: 0
      type: "string"
      resolve: "type.name"

Before

The event look like this

"function_name": "security_key_alloc",
    "args": [
      {
        "error_arg": {
          "Message": "2"
        }
      }
    ],

After

"function_name": "security_key_alloc",
    "args": [
      {
        "string_arg": "rxrpc"
      }
    ],

tdaudi added 2 commits May 8, 2026 17:46
When unstacking a nested BTF union or anonymous struct, we fail to store the
position of the union/struct itself, only capturing the final element.
This leads to incorrect offset resolution.

To fix this, we introduce an accumulator mechanism that accumulates the
union/anonymous struct offset only when the member.name matches the expected
attribute.

Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
Attribute resolution for unions/anonymous struct has been broken since the Resolve
algorithm's introduction. This commit aligns with the new logic by
implementing the same accumulator mechanism, ensuring identical return
values.

Signed-off-by: Tristan d'Audibert <tdaudibe@isovalent.com>
@tdaudi tdaudi marked this pull request as ready for review May 8, 2026 16:50
@tdaudi tdaudi requested a review from a team as a code owner May 8, 2026 16:50
@tdaudi tdaudi requested a review from jrfastab May 8, 2026 16:50
@andrewstrohman andrewstrohman self-requested a review May 8, 2026 16:56
@andrewstrohman andrewstrohman added the release-note/bug This PR fixes an issue in a previous release of Tetragon. label May 8, 2026
@andrewstrohman
Copy link
Copy Markdown
Contributor

andrewstrohman commented May 8, 2026

I tested this change with:

#include <stdio.h>
#include <stdint.h>

struct yetanotherstruct {
        char *first;
        int second;
};

struct anotherstruct {
        int someint;
        struct {
                char achar;
        };
        struct {
                uint32_t someuint;
                struct {
                        char * charp;
                        struct {
                                int first;
                                struct yetanotherstruct second;
                        };
                };
        };
};


struct mystruct {
        int someint;
        struct {
                struct {
                        int anotherint;
                        struct anotherstruct another;
                };
        };
};



void passit(struct mystruct *s) {
        printf("%d\n", s->another.second.second);
}


int main(int argc, char **argv) {
        struct mystruct mys;
        mys.another.second.second = 7;
        passit(&mys);
}

And policy:

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: "uprobe"
spec:
  uprobes:
  - path: "/home/andy/src/nested-union-test"
    BTFPath: "/home/andy/src/nested-union-test.btf"
    symbols:
    - "passit"
    args:
    - index: 0
      type: "int"
      resolve: "another.second.second"
      BTFType: "mystruct"

And the event was as expected:

{"process_uprobe":{"process":{"exec_id":"YnVpbGRlcjo5OTIzODU1MDc5MDYwNzc4OjQxNTg3NDE=","pid":4158741,"uid":1000,"cwd":"/home/andy/build/src","binary":"/home/andy/build/src/nested-union-test","flags":"execve clone","start_time":"2026-05-08T20:32:48.215629845Z","auid":1000,"parent_exec_id":"YnVpbGRlcjoyNzUzMDAwMDAwMDoxNjEz","refcnt":1,"tid":4158741,"in_init_tree":false},"parent":{"exec_id":"YnVpbGRlcjoyNzUzMDAwMDAwMDoxNjEz","pid":1613,"uid":1000,"cwd":"/home/andy/build/src","binary":"/usr/bin/bash","flags":"procFS","start_time":"2026-01-13T23:55:40.666568747Z","auid":1000,"parent_exec_id":"YnVpbGRlcjoyNzQ2MDAwMDAwMDoxNTU4","tid":1613,"in_init_tree":false},"path":"/home/andy/src/nested-union-test","symbol":"passit","policy_name":"uprobe","args":[{"int_arg":7}],"action":"KPROBE_ACTION_POST"},"node_name":"builder","time":"2026-05-08T20:32:48.215891402Z"}

I also tested another variant of that program, where pointers are used, and that works too:

#include <stdio.h>
#include <stdint.h>

struct yetanotherstruct {
        char *first;
        int second;
};

struct anotherstruct {
        int someint;
        struct {
                char achar;
        };
        struct {
                uint32_t someuint;
                struct {
                        char * charp;
                        struct {
                                int first;
                                struct yetanotherstruct *second;
                        };
                };
        };
};


struct mystruct {
        int someint;
        struct {
                struct {
                        int anotherint;
                        struct anotherstruct *another;
                };
        };
};



void passit(struct mystruct *s) {
        printf("%d\n", s->another->second->second);
}


int main(int argc, char **argv) {
        struct mystruct mys;
        struct anotherstruct mya;
        struct yetanotherstruct myy;

        mys.another = &mya;

        mya.second = &myy;

        myy.second = 7;
        passit(&mys);
}

@kkourt kkourt added the needs-backport/1.7 This PR needs backporting to 1.7 label May 11, 2026
@kkourt kkourt merged commit 531dfe3 into cilium:main May 11, 2026
51 of 54 checks passed
@kkourt kkourt added the needs-backport/1.6 This PR needs backporting to v1.6 label May 11, 2026
@kkourt
Copy link
Copy Markdown
Contributor

kkourt commented May 11, 2026

Added a needs-backport label for both 1.7 and 1.6. I am not sure how easy the backport is for 1.6 though.

@kkourt
Copy link
Copy Markdown
Contributor

kkourt commented May 11, 2026

I tested this change with:
[...]

Thanks, @andrewstrohman!

Do you think we can add these tests in our go tests?

@kkourt kkourt mentioned this pull request May 11, 2026
@kkourt kkourt added backport-pending/1.7 PR backport done and removed needs-backport/1.7 This PR needs backporting to 1.7 labels May 11, 2026
@kkourt
Copy link
Copy Markdown
Contributor

kkourt commented May 11, 2026

Added a needs-backport label for both 1.7 and 1.6. I am not sure how easy the backport is for 1.6 though.

At least for 1.7, I had to backport #4900 as well.

@kkourt kkourt added backport-done/1.7 PR backport done and removed backport-pending/1.7 PR backport done labels May 11, 2026
@andrewstrohman
Copy link
Copy Markdown
Contributor

Do you think we can add these tests in our go tests?

Here is the PR to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-done/1.7 PR backport done needs-backport/1.6 This PR needs backporting to v1.6 release-note/bug This PR fixes an issue in a previous release of Tetragon.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resolve fails with structs that have unions

3 participants