All of lore.kernel.org
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: "Filip Štědronský" <r.lkml@regnarg.cz>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Marko Rauhamaa" <marko.rauhamaa@f-secure.com>
Subject: Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Date: Mon, 20 Mar 2017 07:40:41 -0400	[thread overview]
Message-ID: <CAOQ4uxjeNpuo_zY+k9k-PabtKJWunmXZxO-9USrxo8r8Wkj0ng@mail.gmail.com> (raw)
In-Reply-To: <20170319180413.GC1844@quack2.suse.cz>

On Sun, Mar 19, 2017 at 2:04 PM, Jan Kara <jack@suse.cz> wrote:
> On Sun 19-03-17 11:37:39, Filip Štědronský wrote:
>> On Sun, Mar 19, 2017 at 11:19:43AM +0100, Jan Kara wrote:
>> > However if you can really call fsnotify hooks with 'path' available in all
>> > the places, it should be equally hard to just pass 'path' to
>> > vfs_(create|mkdir|...) and that way we don't have to sprinkle fsnotify
>> > calls into several call sites but keep them local to vfs_(create|mkdir|...)
>> > helpers. Hmm?
>>
>> the problem is: not absolutely all. One illuminating example is the use
>> of vfs_mknod in devtmpfs. There a struct path is not only unavailable
>> but makes not semantic sense: the changes do not go thru any mountpoint.
>
> How come? In current kernel the call looks like:
>
> vfs_mknod(d_inode(path.dentry), dentry, mode, dev->devt);
>
> so the path is available there... I've actually quickly checked all
> vfs_mknod() callers and they all seem to have path available.
>
>> And in general I think there will be situations where you would need
>> to call VFS functions without paths.
>>
>> Thus I suggested either
>> (a) wrapping the VFS functions with path variants, or
>> (b) giving them an optional vfsmount argument that can be set to NULL
>>     when it does not make sense
>
> So my first take is that fsnotify calls happen still relatively high in the
> call stack where we should mostly have mount point available from the path
> lookup. That being said there may be places where we've lost that
> information and it will not be easy to propagate it there and that would
> have to be dealt with on case-by-case basis. But mountpoint is needed for
> other stuff like security checks these days as well so we should have it
> available in principle.
>

I agree that propagating the path to fsnotify seem like the right thing to do.
fsnotify_inoderemove() is an example (the only one I know of) where path
is not available (when called down from from dput()), but frankly, I can't
think of any use cases that really needs to make use of the
FS_DELETE_SELF event in that case.

d_delete() which also calls fsnotify_inoderemove() already calls
fsnotify_nameremove() hook with the exact same dentry, so
the FS_DELETE_SELF event can be generated by that hook as well
as the FS_DELETE event.

  reply	other threads:[~2017-03-20 11:40 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-13 23:02 [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Filip Štědronský
2017-03-13 23:03 ` [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes Filip Štědronský
2017-03-14 11:18   ` Amir Goldstein
2017-03-14 14:58     ` Filip Štědronský
2017-03-14 15:35       ` Amir Goldstein
2017-03-15  8:19       ` Marko Rauhamaa
2017-03-15 13:39         ` Jan Kara
2017-03-15 14:18           ` Marko Rauhamaa
2017-03-15 14:44           ` Amir Goldstein
2017-03-19 10:19     ` Jan Kara
2017-03-19 10:37       ` Filip Štědronský
2017-03-19 18:04         ` Jan Kara
2017-03-20 11:40           ` Amir Goldstein [this message]
2017-03-20 11:52           ` Filip Štědronský
2017-03-21 15:38       ` J. Bruce Fields
2017-03-21 16:41         ` Jan Kara
2017-03-21 17:45           ` J. Bruce Fields
2017-03-13 23:16 ` [RFC 1/2] fanotify: new event FAN_MODIFY_DIR Filip Štědronský
2017-03-14 10:40   ` Amir Goldstein
2017-03-14 13:46     ` Filip Štědronský
2017-03-14 15:07       ` Amir Goldstein
2017-03-20 12:10       ` Amir Goldstein
2017-03-14 10:11 ` Amir Goldstein
2017-03-14 12:41   ` Filip Štědronský
2017-03-14 13:55     ` Amir Goldstein
2017-03-14 14:48       ` Filip Štědronský
2017-03-14 22:30         ` Amir Goldstein
2017-03-15 14:05   ` Jan Kara
2017-03-15 14:34     ` Amir Goldstein
2017-03-16 10:38       ` Jan Kara
2017-03-15  4:52 ` Michael Kerrisk
2017-03-15  4:52   ` Michael Kerrisk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOQ4uxjeNpuo_zY+k9k-PabtKJWunmXZxO-9USrxo8r8Wkj0ng@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marko.rauhamaa@f-secure.com \
    --cc=r.lkml@regnarg.cz \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.