WIP: Introduce Ftrace in the syscall section #180
Conversation
nickchen120235
force-pushed
the
master
branch
from
a17514c
to
2ad67b4
Compare
nickchen120235
force-pushed
the
master
branch
from
2ad67b4
to
f7cc340
Compare
jserv
changed the title
- uid should be initialized - update comments - add uid check in our_sys_openat - format
|
|
||
| \begin{figure}[h] | ||
| \centering | ||
| \includegraphics[width=\textwidth]{assets/syscall/flow.jpg} |
There was a problem hiding this comment.
Utilize TikZ for drawing. See https://texample.net/tikz/examples/pgf-umlsd/
Avoid putting bitmap files.
There was a problem hiding this comment.
TikZ examples: https://texample.net/tikz/examples/
There was a problem hiding this comment.
This is the best I got so far. Unfortunately pgf-umlsd doesn't support returning to functions other than the caller, so the hooking part isn't accurately represented.
There was a problem hiding this comment.
Also, I have to rotate the figure by 90 degrees to minimize the overflow although it overflows anyway. Will it affect the output of the website or should I rotate it back and let it overflow?
There was a problem hiding this comment.
Alternatively, you can just render the partial sequences.
There was a problem hiding this comment.
I don't think I can rework this diagram in latex because it lacks features I need, like returning to functions other than caller (this is the most important one), annotation. (I'm not the creator of this sequence diagram so obtaining the "original" file is not possible either.)
I think the resolution of that jpeg is good enough for even printing, so I think I may left it as is.
The latex code I've written and the result
\begin{sequencediagram}
\newthread{do_syscall_64}{do\_syscall\_64}
\newinst[1.5]{sys_execve}{sys\_execve}
\newinst[1.5]{ftrace}{[ftrace]}
\newinst[1]{fh_ftrace_thunk}{fh\_ftrace\_thunk}
\newinst[1]{fh_sys_execve}{fh\_sys\_execve}
\postlevel \postlevel \postlevel
\begin{call}{do_syscall_64}{\shortstack{
\cpp|regs-ax=|\\
\cpp|sys_call_table[nr]|\\
\cpp|(regs->di,regs->si|\\
\cpp|regs->dx,regs->r10|\\
\cpp|regs->r8,regs->r9)|
}}{sys_execve}{}
\begin{call}{sys_execve}{call \cpp|__fentry__|}{ftrace}{}
\begin{call}{ftrace}{}{fh_ftrace_thunk}{}
\postlevel
\end{call}
\end{call}
\begin{call}{sys_execve}{hooking}{fh_sys_execve}{\cpp|real_sys_execve()|}
\postlevel
\end{call}
\postlevel
\begin{call}{sys_execve}{call \cpp|__fentry__|}{ftrace}{}
\begin{call}{ftrace}{}{fh_ftrace_thunk}{}
\postlevel
\end{call}
\end{call}
\begin{call}{sys_execve}{}{fh_sys_execve}{}
\end{call}
\end{call}
\end{sequencediagram}
There was a problem hiding this comment.
Here is the warning, please fix it too.
/home/runner/work/lkmpg/lkmpg/examples/syscall-ftrace.c:190:9: warning: ignoring return value of ‘copy_from_user’, declared with attribute warn_unused_result [-Wunused-result]
190 | if (copy_from_user(kfilename, (char __user *)regs->si, MAX_FILENAME_SIZE) < 0) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
|
- remove obsolete comments - use `pr_fmt` to clean kprintf - remove clang-format comments - `static` declarations - fix ignored return value warning correct the comment `nr` refers to syscall "number", not "name"
nickchen120235
force-pushed
the
master
branch
from
6821a2b
to
2df343f
Compare
- fix comment style - new line after declaration - fix incorrect parameter order of kmalloc
| char *kfilename; | ||
| int errcode = 0; | ||
|
|
||
| if (current->cred->uid.val != uid) |
There was a problem hiding this comment.
Still have the warning.
/home/runner/work/lkmpg/lkmpg/examples/syscall-ftrace.c:180:16: warning: dereference of noderef expression
| We need both \cpp|FTRACE_OPS_FL_SAVE_REGS| and \cpp|FTRACE_OPS_FL_IPMODIFY| because we're modifying \cpp|ip|. | ||
| Inside \cpp|ftrace_thunk| is what the magic happens. | ||
| We check if it is called from within the module, | ||
| if not then it modifies the instruction pointer to our ``spying'' function. |
There was a problem hiding this comment.
Ditto.
Don't separate the line with comma.
| Alright let's write some code. | ||
| Below is the source code of the example from above, but rewritten using \verb|ftrace|. | ||
| The main difference is the \cpp|install_hook| function, | ||
| which prepares our tracee function (\cpp|sys_openat|), |
There was a problem hiding this comment.
Don't separate the line with comma.
| if not then it modifies the instruction pointer to our ``spying'' function. | ||
| The check is performed by checking whether \cpp|parent_ip| is within this module. | ||
| During the first call, \cpp|parent_ip| points to somewhere within the kernel, | ||
| while during the second call it points to somewhere in our ``spying'' function, which is within the module. |
There was a problem hiding this comment.
I didn't point out all of the cases. Please check again.
| and we have access to CPU registers, | ||
| maybe we can ``hijack'' the traced function by modifying the instruction pointer? | ||
| Yes, this is possible by enabling \cpp|FTRACE_OPS_FL_IPMODIFY| flag when registering a trace. | ||
| It will allow us to modify the instruction pointer register, which will become an unconditional jump after the \verb|ftrace| function. |
There was a problem hiding this comment.
The clause introduced by which is restrictive. So, omit the comma.
| Do notice that in kernel version later than v5.11, this is replaced with \cpp|struct ftrace_regs *fregs|, with the original \cpp|pt_regs| accessible by \cpp|fregs->regs|. | ||
| \end{itemize} | ||
|
|
||
| Internally, there's a 5-byte \cpp|call| to \cpp|__fentry__| at the beginning (BEFORE function prologue) of a traceable kernel function, which is converted to \cpp|nop| during boot to prevent overhead. When a trace is registered, it is changed back to \cpp|__fentry__| and the registered callback will be executed accordingly. |
There was a problem hiding this comment.
Separate the line with sentences.
|
Changing this back to draft since I won't be able to work on this for a while. Reviews are still welcomed, though they won't be resolved until I come back. |
Closes #175