Add kernel stack traces to kprobes events

[mtardy]

See individual commits for info.

See related documentation preview.

Preview

Looks like this with the following tracing policy:

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: syscall
spec:
  kprobes:
    - call: kfree_skb_reason
      selectors:
      - matchActions:
        - action: post
          stackTrace: true

And doing

nc -vz spooky.com 443

With

./tetra getevents -o compact --processes nc

The output looks like this (with color):

Add a new kernel stack traces alpha feature to kprobes events.

[netlify]

✅ Deploy Preview for tetragon ready!

Name	Link
🔨 Latest commit	8c8c274
🔍 Latest deploy log	https://app.netlify.com/sites/tetragon/deploys/652653ebb9e9240008d19401
😎 Deploy Preview	https://deploy-preview-1429--tetragon.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[olsajiri]

Looks like this with the following tracing policy:

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: syscall
spec:
  kprobes:
    - call: kfree_skb_reason
      selectors:
      - matchActions:
        - action: post
          stackTrace: true

bit late to the discussion, so sorry if I missed something.. but seems like this kind of setup will trigger the filtering code even if it's not necessary, because the selector is empty and actions tail call does not need to be called

could we perhaps instead add flag/bool to config_map and call get_stackid in generic_output? with spec like:

spec:
  kprobes:
    - call: kfree_skb_reason
      stackTrace: true

[tixxdz]

Looks like this with the following tracing policy:

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: syscall
spec:
  kprobes:
    - call: kfree_skb_reason
      selectors:
      - matchActions:
        - action: post
          stackTrace: true

bit late to the discussion, so sorry if I missed something.. but seems like this kind of setup will trigger the filtering code even if it's not necessary, because the selector is empty and actions tail call does not need to be called

could we perhaps instead add flag/bool to config_map and call get_stackid in generic_output? with spec like:

spec:
  kprobes:
    - call: kfree_skb_reason
      stackTrace: true

+1 for @olsajiri suggestion, as this should apply to all actions or triggered probes anyway

Another thing inside the doc please state clearly that this feature prints kernel addresses, so please ensure to use unix socket for the API when loading tracing policies....

The kernel has some protections to restrict unprivileged from printing kernel addresses... hmm we need to think about this

[mtardy]

Another thing inside the doc please state clearly that this feature prints kernel addresses, so please ensure to use unix socket for the API when loading tracing policies....

The kernel has some protections to restrict unprivileged from printing kernel addresses... hmm we need to think about this

This is already in the doc! 😄

[mtardy]

About putting this thing out of selectors, indeed it makes sense. We had discussions offline about moving that thing from another action to a parameter to post with @kkourt and @tixxdz, but indeed moving it out of selectors makes sense. I think here we have a "UI" vs "how things work" clash a little bit.

[kkourt]

Thanks, I have some minor comments but this LGTM.

[kkourt]

Looks like this with the following tracing policy:

apiVersion: cilium.io/v1alpha1
kind: TracingPolicy
metadata:
  name: syscall
spec:
  kprobes:
    - call: kfree_skb_reason
      selectors:
      - matchActions:
        - action: post
          stackTrace: true

bit late to the discussion, so sorry if I missed something.. but seems like this kind of setup will trigger the filtering code even if it's not necessary, because the selector is empty and actions tail call does not need to be called

could we perhaps instead add flag/bool to config_map and call get_stackid in generic_output? with spec like:

spec:
  kprobes:
    - call: kfree_skb_reason
      stackTrace: true

The downside of having it as a global option is that you don't get to have a different option for different selectors. I'd also expect the selector filtering code to be lightweight if there are nothing to match, but could be wrong on that. I'd prefer having it as a part of the selector. If performance is an issue, I think we can fix it by having a global option and pass it to bpf if all selectors have the stack trace enabled.

[jrfastab]

That warning about the addrs I find a bit alarming. I don't actually think its as serious of an issue as the warning states however. How about just blinding the addresses for now by mask and hash?

[mtardy]

That warning about the addrs I find a bit alarming. I don't actually think its as serious of an issue as the warning states however. How about just blinding the addresses for now by mask and hash?

do we want to use the same hash as the kernel or we don't care? I think the kernel does something like:

+/* Maps a pointer to a 32 bit unique identifier. */
+static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
+{
+	unsigned long hashval;
+	const int default_width = 2 * sizeof(void *);
+
+	if (unlikely(!have_filled_random_ptr_key)) {
+		spec.field_width = default_width;
+		/* string length must be less than default_width */
+		return string(buf, end, "(ptrval)", spec);
+	}
+
+#ifdef CONFIG_64BIT
+	hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key);
+	/*
+	 * Mask off the first 32 bits, this makes explicit that we have
+	 * modified the address (and 32 bits is plenty for a unique ID).
+	 */
+	hashval = hashval & 0xffffffff;
+#else
+	hashval = (unsigned long)siphash_1u32((u32)ptr, &ptr_key);
+#endif
+
+	spec.flags |= SMALL;
+	if (spec.field_width == -1) {
+		spec.field_width = default_width;
+		spec.flags |= ZEROPAD;
+	}
+	spec.base = 16;
+
+	return number(buf, end, hashval, spec);
+}

[mtardy]

I converted it to a draft because we need to fix something with the multi-kprobe support with @olsajiri first. But it's 99% ready! :)

[mtardy]

Merging this, the only CI fails is because https://www.virtualbox.org/ is, as often, down. I need to add to the link checker ignore list.