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

tools.py: Allow args dereference #1936

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

prasad-joshi
Copy link

libc functions like for example eventfd_read()

int eventfd_read(int fd, eventfd_t *value);

may be passed with pointer argument.

Applying star ('*') to such argument must safely copy a memory location
to the BPF stack.

Signed-off-by: Prasad Joshi prasadjoshi.linux@gmail.com

libc functions like for example eventfd_read()

    int eventfd_read(int fd, eventfd_t *value);

may be passed with pointer argument.

Applying star ('*') to such argument must safely copy a memory location
to the BPF stack.

Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
@prasad-joshi
Copy link
Author

This is the smallest (all error handling removed) eventfd_read program I tested with.

$ cat eventfd.c 
#include <sys/eventfd.h>
#include <stdint.h>
#include <unistd.h>

int main() {
	int fd;
	eventfd_t count;

	fd = eventfd(0, EFD_CLOEXEC);
	while (1) {
		eventfd_write(fd, 1);
		eventfd_read(fd, &count);
		sleep(1);
	}

	return 0;
}

Here is the output of trace.py

$ sudo ./trace.py 'r:c:eventfd_read (arg1 == 3) "fd = %d, count = %llu", arg1, *arg2'
PID     TID     COMM            FUNC             -
1225    1225    eventfd         eventfd_read     fd = 3, count = 1
1225    1225    eventfd         eventfd_read     fd = 3, count = 1
1225    1225    eventfd         eventfd_read     fd = 3, count = 1
1225    1225    eventfd         eventfd_read     fd = 3, count = 1
1225    1225    eventfd         eventfd_read     fd = 3, count = 1
  • Tested the code by passing NULL pointer to eventfd_read()
  • On my VM,

python test_tools_smoke.py

reported 7 failures with and without my patches

Uploading smoke test results.

WithoutPatchFailures.txt
WithPatchFailures.txt

return text + """
if (%s != 0) {
bpf_probe_read(&__data.v%d, sizeof(__data.v%d), (void *)%s);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we do not need the if condition in the above. User has a dereference and we should just honor it. bpf_probe_read won't crash the kernel. This will be consistent with other rewriter changes related to
bpf_probe_read as we do not really check return value for bpf_probe_read.

Typically, rewriter should handle * expressions properly. The exception is for anonymous function parameters like arg1, arg2, etc, which are replaced with PT_REGS_PARM1, etc. where *PT_REGS_PARM1(ctx) won't work due to type mismatch. One way could be to only handle arg1/arg2/arg3/arg4/arg5/arg/retval here and leave others to be handled by rewriter. But I am okay to handle all cases of *<...> here for simplicity as done in the above.

Could you put an example in trace.py and trace_example.txt? For example, the command line for your example in this pull request?

@palmtenor
Copy link
Member

BTW your title should be "trace.py: ..."

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.

4 participants