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.

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

ProgramInfo provides CreatedByUid #1006

Merged
merged 1 commit into from
Apr 17, 2023
Merged

Conversation

zachcheu
Copy link
Contributor

@zachcheu zachcheu commented Apr 10, 2023

The CreatedByUid provides some helpful context for the source of the BPF program. Making it public in ProgramInfo

@zachcheu zachcheu force-pushed the expose-created-by-uid branch 2 times, most recently from 09132de to a9ca3c6 Compare April 10, 2023 23:20
info.go Outdated
@@ -93,6 +93,8 @@ type ProgramInfo struct {
Tag string
// Name as supplied by user space at load time. Available from 4.15.
Name string
// CreatedByUid specifies the uid of the process that loaded the program.
CreatedByUid uint32
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which kernel version introduced this field? If it's after 4.9 this needs to be unexported and accessed via a getter instead, similar to Program.ID(). The reason is that users need to be able to tell if the information is present or not. For example, does CreatedByUid == 0 mean that root created it, or that the kernel didn't provide that information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like it was introduced in 4.15. I don't see how a getter would help determine whether the CreateByUid == 0 is because the root was created or because the kernel didn't populate this field. Is there a way to enforce that this function is called on kernel 4.15+?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Luckily it was introduced in the same commit as nr_map_ids, which we already have (feature probe) support for:

ebpf/info.go

Lines 129 to 139 in f591f6f

if info.NrMapIds > 0 {
pi.maps = make([]MapID, info.NrMapIds)
info2.NrMapIds = info.NrMapIds
info2.MapIds = sys.NewPointer(unsafe.Pointer(&pi.maps[0]))
} else if haveProgramInfoMapIDs() == nil {
// This program really has no associated maps.
pi.maps = make([]MapID, 0)
} else {
// The kernel doesn't report associated maps.
pi.maps = nil
}

I suggest the following:

  • Add MapInfo.createdByUID uint32 and MapInfo.haveCreatedByUID bool.
  • Populate that from the place I linked above. I think it needs to happen from both the if and the else if branch.
  • Add MapInfo.CreatedByUID() (uint32, bool) { return mi.createyByUID, mi.haveCreatedByUID }

Plus some tests would be great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm confused about why CreatedByUID fields and functions should be added under MapInfo since CreatedByUID is Program Info specific.

If fields MapInfo.createdByUID and MapInfo.haveCreatedByUID are created then how should they be populated at

ebpf/info.go

Lines 45 to 53 in f591f6f

return &MapInfo{
MapType(info.Type),
MapID(info.Id),
info.KeySize,
info.ValueSize,
info.MaxEntries,
uint32(info.MapFlags),
unix.ByteSliceToString(info.Name[:]),
}, nil

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I meant ProgramInfo. My bad!

@zachcheu zachcheu force-pushed the expose-created-by-uid branch 2 times, most recently from 2f7e11f to 4f99548 Compare April 14, 2023 21:34
Signed-off-by: zach.cheung <zach.cheung@uber.com>
@lmb lmb force-pushed the expose-created-by-uid branch from 4f99548 to 6c8a976 Compare April 17, 2023 09:35
@lmb
Copy link
Collaborator

lmb commented Apr 17, 2023

Fixed up the tests to work on old kernels. Thanks for the PR!

@lmb lmb merged commit 53e5c5a into cilium:master Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants