Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory: Swap probe_read to kernel or user version #2213

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Memory: Swap probe_read to kernel or user version #2213

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
31f4c79
CRD: Add IsUserspaceData flag for args
kevsecurity Mar 12, 2024
5fed43c
Tracing: Add UserspaceData flag to arg meta
kevsecurity Mar 12, 2024
7d51904
Memory: Swap probe_read to kernel or user version
kevsecurity Apr 9, 2024
0cb1f70
Memory: Add probe_read_kernel_or_user helpers
kevsecurity Apr 9, 2024
e1bcd2e
Memory: Update bpf_core_read.h
kevsecurity Apr 9, 2024
4a2fe97
Memory: Add raw_syscall flag on arg meta
kevsecurity Apr 10, 2024
9c4d85e
Memory: Use probe_read_kernel_or_user helpers
kevsecurity Apr 10, 2024
e2f0091
Retprobe: Use correct key for retprobe_map
kevsecurity Apr 10, 2024
1bc49cd
Memory: Read kernel or user for return arguments
kevsecurity Apr 10, 2024
70a327e
Memory: Add tail call to retkprobe
kevsecurity Apr 10, 2024
179404a
Raw_syscalls: fix selector indices in policies
kevsecurity Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions pkg/sensors/tracing/args.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
argReturnCopyBit = 1 << 4
argMaxDataBit = 1 << 5
argUserspaceDataBit = 1 << 6
argRawSyscallsBit = 1 << 7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a different flag?

From what I can tell from the code the flag is used in

tetragon/bpf/process/types/basic.h

Lines 2697 to 2702 in 179404a

// the const_buf_type that represents an array of arguments for raw_syscalls
// is special, as the array contents are in kernel memory, but they point
// to userspace memory. In this case, they will be marked as userspace, but
// we actually want to read kernel memory. The raw_syscalls bit of the meta
// value indicates when the arg is special in this way.
probe_read_kernel_or_user_masked(args, size, 0x3ff, (char *)arg, userspace && !raw_syscalls);

And set in

tetragon/pkg/sensors/tracing/generictracepoint.go

Line 251 in 179404a

metaTp, err := getMetaValue(specArg, syscall, rawSyscalls && specArg.Index == 5 && (specArg.Type == ""))

Cant' we just do

userspaceDataDefault = syscall &&  !(rawSyscalls && specArg.Index == 5 && (specArg.Type == ""))
getMetaValue(arg, userSpaceDataDefault)

And use only one flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was to prepare for being able to filter on arguments in raw_syscalls, and therefore arrays of arguments which could potentially be pointers. I thought it was a bit much to include that in this PR, but I was already thinking ahead for a following PR. Can do as you suggest if you prefer though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do as you suggest if you prefer though.

Thanks! I would prefer to keep the code as simple as possible, even if we end up extending this for filtering.

I've also thought about filtering raw_syscalls.

One way would be to do as you suggest, and use the arguments but this seems tricky because we need to pass types for everything.

I think a better approach would be to do two-level filtering where we in raw_syscall event we to filter the type of the syscall and then we enable hooks on syscalls/ for filtering on the arguments. This seems simpler to me and it re-uses the arg filtering we already have. We would need to have a way to pass information from the raw_syscall hook to the specific syscall hook, but we already have something similar Jiri's notify enforcer action.

In any case, that's for another PR.

)

func argReturnCopy(meta int) bool {
Expand All @@ -47,9 +48,10 @@ func argReturnCopy(meta int) bool {
// 4 : ReturnCopy
// 5 : MaxData
// 6 : UserspaceData
// 7-15 : reserved
// 7 : RawSyscalls
// 8-15 : reserved
// 16-31 : size for const_buf
func getMetaValue(arg *v1alpha1.KProbeArg, userspaceDataDefault bool) (int, error) {
func getMetaValue(arg *v1alpha1.KProbeArg, userspaceDataDefault bool, rawSyscalls bool) (int, error) {
meta := 0

if arg.SizeArgIndex > 0 {
Expand All @@ -75,6 +77,9 @@ func getMetaValue(arg *v1alpha1.KProbeArg, userspaceDataDefault bool) (int, erro
meta = meta | argUserspaceDataBit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: meta |= ...

}
}
if rawSyscalls {
meta = meta | argRawSyscallsBit
}
return meta, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sensors/tracing/generickprobe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (id idt
}
}
// For kprobes, args default to userspace memory for syscalls, and kernel memory otherwise.
argMValue, err := getMetaValue(&a, f.Syscall)
argMValue, err := getMetaValue(&a, f.Syscall, false)
if err != nil {
return errFn(err)
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/sensors/tracing/generictracepoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ func (out *genericTracepointArg) getGenericTypeId() (int, error) {
func buildGenericTracepointArgs(info *tracepoint.Tracepoint, specArgs []v1alpha1.KProbeArg) ([]genericTracepointArg, error) {
ret := make([]genericTracepointArg, 0, len(specArgs))
nfields := uint32(len(info.Format.Fields))
syscall := info.Subsys == "syscalls" || info.Subsys == "raw_syscalls"
rawSyscalls := info.Subsys == "raw_syscalls"
syscall := rawSyscalls || info.Subsys == "syscalls"

for argIdx := range specArgs {
specArg := &specArgs[argIdx]
Expand All @@ -247,7 +248,7 @@ func buildGenericTracepointArgs(info *tracepoint.Tracepoint, specArgs []v1alpha1
}
field := info.Format.Fields[specArg.Index]
// Syscall tracepoint arguments are in userspace memory.
metaTp, err := getMetaValue(specArg, syscall)
metaTp, err := getMetaValue(specArg, syscall, rawSyscalls && specArg.Index == 5 && (specArg.Type == ""))
if err != nil {
return nil, fmt.Errorf("tracepoint %s/%s getMetaValue error: %w", info.Subsys, info.Event, err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sensors/tracing/genericuprobe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func addUprobe(spec *v1alpha1.UProbeSpec, ids []idtable.EntryID, in *addUprobeIn
return nil, fmt.Errorf("Arg(%d) type '%s' unsupported", i, a.Type)
}
// For uprobes, args default to userspace memory.
argMValue, err := getMetaValue(&a, true)
argMValue, err := getMetaValue(&a, true, false)
if err != nil {
return nil, err
}
Expand Down