Skip to content

new: Support for memfd_create syscall - #1127

Merged
poiana merged 11 commits into
falcosecurity:masterfrom
Rohith-Raju:memfd-create
Jun 15, 2023
Merged

new: Support for memfd_create syscall#1127
poiana merged 11 commits into
falcosecurity:masterfrom
Rohith-Raju:memfd-create

Conversation

@Rohith-Raju

@Rohith-Raju Rohith-Raju commented May 31, 2023

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area driver-kmod
/area driver-bpf
/area driver-modern-bpf
/area libscap
/area tests

Does this PR require a change in the driver versions?
/version driver-SCHEMA-version-minor

What this PR does / why we need it:

This PR adds support to memfd_create syscall

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new: support for memfd_syscall

Signed-off-by: Rohith Raju <rohithraju488@gmail.com>
@poiana

poiana commented May 31, 2023

Copy link
Copy Markdown
Contributor

Welcome @Rohith-Raju! It looks like this is your first PR to falcosecurity/libs 🎉

@poiana poiana added the size/L label May 31, 2023
@poiana
poiana requested review from LucaGuerra and hbrueckner May 31, 2023 05:23
@github-actions

github-actions Bot commented May 31, 2023

Copy link
Copy Markdown

Please double check driver/SCHEMA_VERSION file. See versioning.

Signed-off-by: Rohith Raju <rohithraju488@gmail.com>
@poiana poiana added size/XL and removed size/L labels May 31, 2023
@FedeDP

FedeDP commented May 31, 2023

Copy link
Copy Markdown
Contributor

Thank you for this PR! Since we are in feature freeze for the next libs tag (for Falco 0.35), i will put this in the next milestone.
/milestone next-driver

@poiana poiana added this to the next-driver milestone May 31, 2023
Signed-off-by: Rohith Raju <rohithraju488@gmail.com>

@Andreagit97 Andreagit97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a first pass of comments, amazing job, thank you!

Comment thread driver/bpf/fillers.h Outdated
Comment thread driver/bpf/fillers.h Outdated
Comment thread driver/bpf/fillers.h Outdated
Comment thread driver/bpf/fillers.h Outdated
Comment thread driver/event_table.c Outdated
Comment thread driver/ppm_flag_helpers.h Outdated
Comment thread driver/ppm_flag_helpers.h Outdated
Comment thread driver/syscall_table.c Outdated
Comment thread test/drivers/test_suites/syscall_exit_suite/memfd_create_x.cpp Outdated
Comment thread test/drivers/test_suites/syscall_exit_suite/memfd_create_x.cpp
@jasondellaluce

Copy link
Copy Markdown
Contributor

TODO for ourselves: either here or in another PR we should update parsers.cpp to account for this new syscall and extract+update the fd name & flags.

Comment thread driver/bpf/fillers.h Outdated
Signed-off-by: Rohith Raju <rohithraju488@gmail.com>
@Rohith-Raju

Copy link
Copy Markdown
Contributor Author

@Andreagit97 @hbrueckner @jasondellaluce, SyscallExit.memfd_createX_success fails only for moderbpf. Double checked memfd_create.bpf.c file. Am i missing something ?
https://app.circleci.com/pipelines/github/falcosecurity/libs/1229/workflows/c92b34f7-5f53-4f0b-a002-fda7b72aaa0d/jobs/3412?invite=true#step-105-188

Comment thread driver/ppm_flag_helpers.h Outdated
Signed-off-by: Rohith Raju <rohithraju488@gmail.com>
Signed-off-by: Rohith Raju <rohithraju488@gmail.com>
Comment thread driver/event_table.c Outdated
@Andreagit97

Copy link
Copy Markdown
Member

ok so probably we need 2 fixes:

  1. we need #include <uapi/linux/memfd.h> but with an ifdef so:
#ifdef __NR_io_memfd_create
#include <uapi/linux/memfd.h>
#endif
  1. This one new: Support for memfd_create syscall #1127 (comment)

Signed-off-by: Rohith Raju <rohithraju488@gmail.com>
@Rohith-Raju

Rohith-Raju commented Jun 14, 2023

Copy link
Copy Markdown
Contributor Author

ok so probably we need 2 fixes:

  1. we need #include <uapi/linux/memfd.h> but with an ifdef so:
#ifdef __NR_io_memfd_create
#include <uapi/linux/memfd.h>
#endif

@Andreagit97 I tried doing this, same weird problem mentioned above, either way, let me give it a try!!
#1127 (comment)

@Rohith-Raju Rohith-Raju changed the title Feat: Support for memfd_create syscall New: Support for memfd_create syscall Jun 14, 2023
@Rohith-Raju Rohith-Raju changed the title New: Support for memfd_create syscall new: Support for memfd_create syscall Jun 14, 2023
@Rohith-Raju
Rohith-Raju requested a review from Andreagit97 June 14, 2023 15:47
Signed-off-by: Rohith Raju <rohithraju488@gmail.com>
Signed-off-by: Rohith Raju <rohithraju488@gmail.com>
@Rohith-Raju

Copy link
Copy Markdown
Contributor Author

@Andreagit97 So here is what's happening:

we need #include <uapi/linux/memfd.h> but with an ifdef so:

#ifdef __NR_io_memfd_create
#include <uapi/linux/memfd.h>
#endif

If we do this our tests will fail (current patch) : https://app.circleci.com/pipelines/github/falcosecurity/libs/1233/workflows/80868ad6-6b6e-4e2c-b09d-c27379843619/jobs/3425?invite=true#step-105-221


if we just include <uapi/linux/memfd.h> without the macros

#include <uapi/linux/memfd.h>

Our tests won't fail anymore. But the build-drivers workflow will fail.
https://github.com/falcosecurity/libs/actions/runs/5269263474/jobs/9527576287#step:3:105

Signed-off-by: Rohith Raju <rohithraju488@gmail.com>

@Andreagit97 Andreagit97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

with this patch all should work fine, i'm just trying it locally to understand why we need the uapi prefix

Comment thread driver/ppm_flag_helpers.h Outdated
@Andreagit97

Andreagit97 commented Jun 15, 2023

Copy link
Copy Markdown
Member

uhh I got it, the actual header in /usr/include/linux/memfd.h is shadowed by /usr/src/linux-hwe-5.19-headers-5.19.0-43/include/linux/memfd.h (on my distro of course) and the second header file doesn't include any flag definition, ok so we can use <uapi/linux/memfd.h>!
After the ifdef fix we should be ready to go 🎉

Signed-off-by: Rohith Raju <rohithraju488@gmail.com>

@FedeDP FedeDP left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/approve

@poiana

poiana commented Jun 15, 2023

Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: c67e33b791d5b97c5332f450316265c96c161656

@Andreagit97 Andreagit97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/approve

@poiana

poiana commented Jun 15, 2023

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, Rohith-Raju

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@FedeDP

FedeDP commented Jul 28, 2023

Copy link
Copy Markdown
Contributor

/milestone 5.1.0+driver

@poiana poiana modified the milestones: next-driver, 5.1.0+driver Jul 28, 2023
@Rohith-Raju
Rohith-Raju deleted the memfd-create branch July 31, 2023 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants