On Wed, Nov 14, 2018 at 01:02:47PM +0100, Jan Kara wrote: > > > > > LTP tests for this feature are on my 'fanotify-exec' branch here: > > > > > https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files > > > > > that contains the test cases are provided below: > > > > > > > > > > syscalls/fanotify03: test cases have been updated to cover > > > > > FAN_OPEN_EXEC_PERM events > > > > > syscalls/fanotify12: newly introduced LTP test file to cover > > > > > FAN_OPEN_EXEC events > > > > > > > > I have been wondering for a while why the testcases passed when ignore mask > > > > hasn't been properly treated in fanotify_group_event_mask() but then I > > > > realized that the generic code will not even call to fanotify if ignore > > > > masks result in clearing the event. > > > > > > So does that means we have missing test coverage? > > > > > > > This is covered by test case #3 of Matthew's proposed LTP test. > > https://github.com/matthewbobrowski/ltp/commit/9e350fe15a5423d896ed0e8e147edc15bee13b42#diff-2bb8ddff24b3a031be0f64354262e587R76 > > This testcase does not catch the bug we had in fanotify_group_event_mask() > because the masking by mark->mask already hides the fact that we failed to > apply the ignore mask. > > What does catch this kind of bug (tested) is a testcase (admittedly > somewhat silly) like this: > > { > "inode mark, FAN_OPEN | FAN_OPEN_EXEC events, ignore FAN_OPEN_EXEC", > INIT_FANOTIFY_MARK_TYPE(INODE), > FAN_OPEN | FAN_OPEN_EXEC, > FAN_OPEN_EXEC, > 2, > {FAN_OPEN, FAN_OPEN} > }, I've incorporated this^ test as part of my test cases. All tests, this one included, are passing on kernel built on your 'fsnotify' branch. You can find the updated test case file here: https://github.com/matthewbobrowski/ltp/commit/d1d57d5bda8db49a26624c7737c2db88ea90f9db > A real variant of this would be FAN_OPEN | FAN_OPEN_EXEC on mount, ignore > FAN_OPEN on inode. Then we should just get one FAN_OPEN_EXEC but with the bug > we'd get FAN_OPEN | FAN_OPEN_EXEC. > > But creating such test would be slightly more involved. But probably it is > worth it. Matthew? Yeah, this shouldn't be too difficult to add at all, but as Amir pointed out, I'd probably be in favour of putting this into a different test case i.e. one which deals with mounts/filesystem mark types. > Also I have noticed that fanotify12 test has a bug that it reports: > > fanotify12.c:220: FAIL: Received event: mask=1020, pid=5142 (expected 5142), fd=5 > > i.e., it reports expected pid instead of expected mask when mask does not > match. Can you please fix it Matthew? Sure, a fix for this has also been applied here: https://github.com/matthewbobrowski/ltp/commit/d1d57d5bda8db49a26624c7737c2db88ea90f9db -- Matthew Bobrowski
