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

fix path rename signature #317

Merged
merged 1 commit into from
Nov 14, 2024
Merged

fix path rename signature #317

merged 1 commit into from
Nov 14, 2024

Conversation

banditopazzo
Copy link
Member

@banditopazzo banditopazzo commented Oct 29, 2024

On newer kernel you need to specify the full signature, even if you don't use for last parameters

Ref: #318

Copy link
Member

@vadorovsky vadorovsky left a comment

Choose a reason for hiding this comment

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

Looks like it fails on older kernels. I will try to think of some solutions, but so far the only one I have in mind is to unpack arguments directly from the void *ctx and skip the ones we don't need (flags). ctx is just an array of pointers, so we can try to treat it as size_t *.

Aya handles arguments of LSM program by treating the context as *const usize, so that should work:

https://github.com/aya-rs/aya/blob/f9b34fe76fe8a1f39c4897f2d53703858a5c6527/ebpf/aya-ebpf/src/args.rs#L33-L39

We could do that manually like:

static __always_inline void on_path_rename(void *ctx) {
  size_t *args = (size_t*) ctx;

  struct path *old_dir = (struct path*) args[0];
  struct dentry *old_dentry = (struct dentry*) args[1];
  struct path *new_dir = (struct path*) args[2];
  struct dentry *new_dentry (struct dentry*) args[3];

  pid_t tgid = tracker_interesting_tgid(&GLOBAL_INTEREST_MAP);
  if (tgid < 0)
    return;

  [...]
}

I think we would need to modify PULSAR_LSM_HOOK macro or make some other one which skips the argument unpacking.

@banditopazzo banditopazzo force-pushed the fix-path-rename-signature branch 3 times, most recently from d081a97 to 88e2c69 Compare November 14, 2024 00:23
@banditopazzo banditopazzo marked this pull request as ready for review November 14, 2024 01:12
Copy link
Member

@vadorovsky vadorovsky left a comment

Choose a reason for hiding this comment

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

Just one nit.

Great to see that it works and also nice to know that BPF_PROG macro doesn't mask the ctx variable!

// On kernel >= 5.19 there is another parameter before:
// `unsigned int flags` in `ctx[4]`;
// so ret it located foward
if ((LINUX_KERNEL_VERSION >= KERNEL_VERSION(5, 19, 0))) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ((LINUX_KERNEL_VERSION >= KERNEL_VERSION(5, 19, 0))) {
if (LINUX_KERNEL_VERSION >= KERNEL_VERSION(5, 19, 0)) {

@banditopazzo banditopazzo force-pushed the fix-path-rename-signature branch from 88e2c69 to 89be71e Compare November 14, 2024 11:46
@vadorovsky vadorovsky merged commit b95fd73 into main Nov 14, 2024
21 checks passed
@vadorovsky vadorovsky deleted the fix-path-rename-signature branch November 14, 2024 13:12
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.

3 participants