On Thu, May 5, 2022 at 10:22 AM guowei du <duguoweisz@xxxxxxxxx> wrote: > > Ok, I understand. > > All replies are very important to me,I need to make some changes for my mistakes. > I should not do the same wrong things for now. One more newbie mistake to correct - no "top posting" please. Answers below the questions. See one more important comment below. > > > thanks very much. > > On Thu, May 5, 2022 at 1:23 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> >> On Thu, May 5, 2022 at 6:28 AM guowei du <duguoweisz@xxxxxxxxx> wrote: >> > >> > thanks very much for your replay. >> > >> > I am a starter for kernel filesystem study, i see the newest plan with the 'pre modify events', >> > I think the plan is great and meaningful,I am looking forward to it. >> >> Welcome. Since you are new let me start with some basics. >> I don't know what generated the long list of CC that you used, >> I suppose it was get_maintainer.pl - this list is way too long and irrelevant >> I cut it down to the list and maintainers listed in the MAINTAINERS file. >> >> > >> > for the legacy modify events, i mean to implement most blocking events which are not yet >> > done for now, maybe the permission model is old or legacy, and,sure ,expending the >> > blocking events such as unlink/rmdir/rename will do pollute EVENTS namespace in part. >> > but, it is only a little ,maybe all usual blocking events are open/access/rmdir/unlink/rename, >> > they cover some usecases,and other modify events can be only audited or notified. >> >> Sorry, I don't understand what you mean. >> >> > >> > With the fanotify, open/access/rmdir/unlink/rename need to occupy a fsnotify bit to explicitly >> > separate from others.if Blocking event is denied,then there will be no normal events to notify. >> > >> >> Sorry, I don't understand what you mean. >> What I meant is that adding different bits for FAN_OPEN and FAN_OPEN_PERM >> was a mistake that was done a long time ago and we need to live with it. >> We do NOT need to repeat the same mistake again. >> >> We need to initialize fanotify with class FAN_CLASS_PERM and then when >> setting events FAN_UNLINK|FAN_RMDIR in mask they will be blocking events >> which reader will need to allow/deny. >> >> Here is my old example implementation of dir modify permission events that use >> just one more bit in mask: >> https://github.com/amir73il/linux/commits/fsnotify_dirent_perm >> As you can see in the branch above, this is similar to your patch but more generic. However, I did not like this approach especially if exporting blocking events to userspace because the hooks are called with directory inode locked. That means that as long as the monitoring program does not approve an unlink, creation of files in that directory are blocked as well. This could also result in deadlock because userland is not aware of directory locking order. So IF we are going to support those blocking events in userspace and be aware that it is not at all certina that we will, then those events better be called without the inode lock held as in my pre_modify patches. I can split #define FS_NAME_INTENT 0x02000000 /* Subfile about to be linked/unlinked */ To FS_LINK_INTENT and FS_UNLINK_INTENT if needed that is not a problem, but separate events for RMDIR and UNLINK is an overkill. You will be able to use FAN_ONDIR to request or ignore FS_UNLINK_INTENT events on directories. I expect to post these patches within month or two. Thanks, Amir.
