Tetragon does not raise an event when the resolved value is null

[tdaudi]

What happened?

Let's take an example to illustrate this with the below policy. To trigger this policy, run any command in bash.

In security_bprm_check the bprm.interpreter attribute is null, but Tetragon does not raise the error
https://elixir.bootlin.com/linux/v6.14-rc3/source/include/linux/binfmts.h#L54

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: "lsm"
spec:
  lsmhooks:
  - hook: "bprm_check_security"
    args:
    - index: 0
      type: "string"
      resolve: "executable.f_path.dentry.d_name.name"
    selectors:
    - matchActions:
      - action: Post

No events are raised when this policy is applied.
If you try

    - index: 0
      type: "uint32"
      resolve: "executable.f_mode"

A 0 is raised where it should not, because here this is not the f_mode that is null but the executable.

This happened in the below code.

tetragon/bpf/process/generic_calls.h

Lines 70 to 78 in e3c2b34

	extract_arg_depth(u32 i, struct extract_arg_data *data)
	{
	if (i >= MAX_BTF_ARG_DEPTH || !data->btf_config[i].is_initialized)
	return 1;
	*data->arg = *data->arg + data->btf_config[i].offset;
	if (data->btf_config[i].is_pointer)
	probe_read((void *)data->arg, sizeof(char *), (void *)*data->arg);
	return 0;
	}

We do not track errors on the extract or later on the copy value.

Tetragon Version

latest

Kernel Version

6.14

[andrewstrohman]

Hi @tdaudi. Thanks for creating this issue and giving me you insights in #4327. It's great getting your feedback early.

In security_bprm_check the bprm.interpreter attribute is null,

I think you meant bprm.executable here, not bprm.interpreter

but Tetragon does not raise the error

Does this mean generate an event? I see you said that in your next sentence. I just want to make sure that I correctly understand the expectation.

A 0 is raised where it should not, because here this is not the f_mode that is null but the executable.

My understanding is that the executable member/attribute of bprm is NULL in this example. So the arg values of both "executable.f_path.dentry.d_name.name" and "executable.f_mode" are not valid for this same reason. The difference in behavior is not about the nested position within executable, but rather, how "string" args are read compared to int types. string types can have this outcome, a negative return value. Negative return values cause the event to be discarded. On the other hand, an int type will never cause a negative return value from read_arg() because dereferencing is not necessary, and as such probe_read won't fail.

What do you think? I need to continue thinking about this more, and I'll keep you updated.

[andrewstrohman]

I think we need to treat the two issues that you described here independently.

1. tetragon does not raise an event: This is title of this issue and the first problem you described here. It's not specific to resolve. If a probed function signature has a char * argument, and NULL is passed, no event will fire, even though resolve is not being used in this more simple scenario.
2. tetragon does not detect NULL pointers / page faults when dereferencing during resolving: This applies to both the first and second problem being described here. The result of this issue are events with inaccurate arg values. This can cause further complications when considering the matchArgs selector.

I want to keep the resolve logic and the terminal type parsing in read_arg() decoupled. Put another way, I don't want to try to compensate for problems in read_arg() but putting type specific logic in extract_arg_depth as that would be difficult to reason about. I think the combination of both these approaches will fix both issues described here, but they both don't need to happen at the same time.

There is overlap between these two issues, in that both resolutions need a way to signal that they could not dereference, and as such the arg value can not be determined. They both need to be able to handle NULL pointers / page faults seen by bpf_probe_read() without preventing an event from firing.