I am developing an application which uses fanotify{_init,_mark). When I run it in valgrind I get ENOSYS because these functions are no listed anywhere coregring/m_syswrap/* (well, they are, but commented out). I developed the included patch according to README_MISSING_SYSCALL_OR_IOCTL and tested it as well. Reproducible: Always Steps to Reproduce: 1. write a simple application which calls fanotify_init() 2. run it in valgrind 3. see the "system call unimplemented" (ENOSYS) in the application log
Created attachment 80393 [details] fanotify patch I have not tested if the proposed implementation actually catches programming errors (unaccesible memory and such)
Created attachment 80852 [details] Patch to support fanotify API on AMD64 architecture I independently came up with the appended patch. Unofficial man pages for the fanotify API are provided at: http://www.xypron.de/projects/fanotify-manpages/ In the previous patch there seem to be some errors concerning the types and a lack of error handling (POST(sys_fanotify_init)).
For testing the tool "fatrace" can be used. It is available at https://launchpad.net/fatrace After building call sudo /usr/local/bin/valgrind --tool=callgrind --trace-syscalls=yes ./fatrace
Created attachment 80857 [details] Enable fanotify API on x86 32bit platform This additional patch will enable the fanotify API on the 32bit x86 platform.
It is important not to use glibc headers in Valgrind source files. Please rework the second patch such that "#include <stdint.h>" is removed. And since the third argument of fanotify_mark() has type "unsigned long long", how can the PRINT() statement in PRE(sys_fanotify_mark) be correct ?
Hello Bart, thank you for you review. Unfortunately I dont get what you mean: >> And since the third argument of fanotify_mark() has type "unsigned long long", how can the PRINT() statement in PRE(sys_fanotify_mark) be correct The statement is PRINT( "sys_fanotify_mark ( %ld, %lu, %llu, %ld, %#lx(%s))", ARG1, ARG2, (ULong) ARG3, ARG4, ARG5, (char *) ARG5); ARG1, int, %ld ARG2, unsigned int, %lu ARG3, uint64_t, %llu ARG4, int, &ld ARG5, char *, %#lx ARG5, char *, %s In libvex_basictypes.h you find the definition of ULong: /* Always 64 bits. */ typedef unsigned long long int ULong; Why do you think that the format code %llu for ARG3 does not match ULong? >> It is important not to use glibc headers in Valgrind source files. The include can be avoided using ULong instead of uint64_t. Best regards Heinrich Schuchardt
Created attachment 80860 [details] [PATCH 4/4] Avoid glibc include, use ULong instead of uint64_t Avoid glibc include, use ULong instead of uint64_t. This patch hat to be applied after 80852: Patch to support fanotify API on AMD64 architecture 80857: Enable fanotify API on x86 32bit platform
You can ignore my comment about the PRINT() statement. But there is another issue: your patches do not apply against the latest Valgrind SVN version. Please regenerate the patches against the latest Valgrind SVN version. $ patch -p1 <../p1 patching file coregrind/m_syswrap/priv_syswrap-linux.h Hunk #1 succeeded at 261 (offset 1 line). patching file coregrind/m_syswrap/syswrap-amd64-linux.c Hunk #1 succeeded at 1068 with fuzz 2 (offset -361 lines). patching file coregrind/m_syswrap/syswrap-linux.c Hunk #1 succeeded at 4649 (offset 923 lines). $ patch -p1 <../p2 patching file coregrind/m_syswrap/priv_syswrap-linux.h Hunk #1 succeeded at 275 (offset 2 lines). patching file coregrind/m_syswrap/syswrap-amd64-linux.c Hunk #1 FAILED at 1068. 1 out of 1 hunk FAILED -- saving rejects to file coregrind/m_syswrap/syswrap-amd64-linux.c.rej patching file coregrind/m_syswrap/syswrap-linux.c
... but patches 2, 3 and four apply cleanly if I leave out patch 1. The "[PATCH 4/4]" prefix had confused me.
Created attachment 80869 [details] ARG5 of fanotify_mark is allowed to be NULL. As described on http://www.xypron.de/projects/fanotify-manpages/man2/fanotify_mark.2.html If pathname (ARG5) is NULL dfd (ARG4) defines the path to be marked. Apply this patch after 80852: Patch to support fanotify API on AMD64 architecture 80857: Enable fanotify API on x86 32bit platform 80860: [PATCH 4/4] Avoid glibc include, use ULong instead of uint64_t
Unfortunately the patch is still incomplete: with --track-fds=yes not closing fanotify_event_metadata.fd does not cause memcheck to complain about an fd leak.
Hello Bart, the file descriptor returned in struct fanotify_event_metadata is created by function create_fd in file fs/notify/fanotify/fanotify_user.c of the kernel source. http://lxr.free-electrons.com/source/fs/notify/fanotify/fanotify_user.c#L62 Reading http://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.wrapping it is still unclear to me if it is possible to wrap this static function. Best regards Heinrich Schuchardt
Maybe it's possible to mark any file descriptor returned by fanotify_init() in Valgrind and to modify the post-section of any system call that reads from such file descriptors such that Valgrind becomes aware that a new file descriptor has been added to the client ? It might be challenging to get this right though.
Hello Bart, >> It might be challenging to get this right though. If you could upload the proposed patches to trunk this would be an incomplete solution but at least users would be able to analyze programs containing fanotify API calls. >> modify the post-section of any system call that reads There is no guarantee for the length of structure fanotify_event_metadata. Instead fanotify_event_metadata->event_len is used to find the next entry in the read buffer with macro FAN_EVENT_NEXT(meta, len). The solution desired hence would require to reference <sys/fanotify.h> as Above you said "It is important not to use glibc headers in Valgrind source files." What is your view on this? Could you, please, update the status of this bug to confirmed. Best regards Heinrich Schuchardt
Regarding including <sys/fanotify.h>: please don't do that but add the necessary structures in the proper include/vki/*.h header file.
What's the status of this now? I am getting the impression that the patch(es) need a bit more work -- is that right?
Hello Julian, >> What's the status of this now? With the patches supplied a program containing calls to fanotify_init and fanotify_mark can be run via valgrind. fanotify_init creates a "primary" file descriptor (fanotify_fd). With the patches supplied the closing of this file descriptor is monitored. fanotify_mark can be used to mark a file, a directory, or a mount to be monitored. One type of monitoring allows to control permission to access after first inspecting the accessed file. In the case of permission checks when a file is tried to be accessed/opened, above file descriptor (fanotify_fd) can be read yielding a structure fanotify_event_metadata which in the case of a permission event contains a file descriptor (fd). Your program will answer by writing to said first file descriptor (fanotify_fd) a structure fanotify_response and afterwards closing said second file descriptor (fd). Closing the primary file descriptor (fanotify_fd) will close all open secondary file descriptors (fd). Using the patches proposed by me --track-fds=yes would only monitor (fanotify_fd). But if this shows no error you can be sure that all secondary file descriptors (fd) were closed. Bart was suggesting not only to monitor the primary file descriptor (fanotify_fd) but also the secondary file descriptors (fd). The additional information provided would be the secondary file descriptors (fd) left open because the primary file descriptor (fanotify_fd) was not closed. It would require major coding changes: Struct OpenFd would have to changed such that each primary file descriptor (fanotify_fd) points to a set of secondary file descriptors (fd). Each secondary file descriptor would have to have a pointer to point to its primary file descriptor (fanotify_fd). sys_read would check if the file descriptor is a fanotify file descriptor. If yes the output of the system call would have to parsed and new secondary file descriptor would have to be created. When closing a secondary file descriptor it has to be removed from the set in the primary file descriptor. When closing the primary file descriptor all secondary file descriptors would have to be marked as successfully closed. And of cause the reporting routines would have to be adjusted. As the existing patches would already indicate if you forgot to close the primary file descriptor the additional information will not identify an additional problem. I suggest to move the patches to trunk as is. As reference you may use http://www.xypron.de/projects/fanotify-manpages/ Best regards Heinrich Schuchardt
Committed as r13460 with some fixes for 32 bit platforms where fanotify_mark takes an extra argument because the 64 bit mask has to be split in two.