linux-kernel - Re: [PATCH v2 1/1] ftruncate, truncate: create fanotify events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 11 Nov 2014 20:55:26 +0100
From:	Heinrich Schuchardt <xypron.glpk@....de>
To:	Jan Kara <jack@...e.cz>, Christoph Hellwig <hch@...radead.org>
CC:	Eric Paris <eparis@...isplace.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
	Jeff Layton <jlayton@...chiereds.net>,
	"J. Bruce Fields" <bfields@...ldses.org>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] ftruncate, truncate: create fanotify events

On 11.11.2014 12:09, Jan Kara wrote:
> On Mon 10-11-14 23:34:15, Christoph Hellwig wrote:
>> On Mon, Nov 10, 2014 at 09:30:29PM +0100, Jan Kara wrote:
>>>    So what I somewhat dislike about this patch is that notify_change() is
>>> sometimes called with dentry and sometimes with path. That way it's not
>>> completely clear when fanotify events will be generated and when not.

No. With my patch notify_change is still only called with dentry.
It is only new function notify_change_path which will be called with a path.
And this function will only be called from do_truncate up to now.

>>> Sadly it isn't easy to provide struct path in all the places where we are
>>> calling notify_change() so I'm not sure what would a better solution look
>>> like either :(

We only want to create FAN_MODIFY events for ATTR_SIZE. So only for 
these events we need a path.

To my knowledge notify_change is called with ATTR_SIZE from 
do_truncate(), ecryptfs_truncate() and will be called with ATTR_SIZE 
from ovl_setattr() for a truncation.

ecryptfs_dentry_to_lower_path() could be used in ecryptfs_truncate() to 
obtain a path.
ovl_path_upper() could be used in ovl_setattr() to obtain a path.

>>
>> I suspect the right thing to do is to split out the truncate path
>> from notify_change, as it's fairly different anyway.
>    Yeah, that would make sense. I wanted to say it's quite a lot of work to
> change all the filesystems (where the separation of truncate path makes
> sense as well IMHO) but actually it's possible to just do the separation at
> the VFS level and still call ->setattr() fs callback for now. Heinrich will
> you look into this?

You seem to agree that struct path has to passed to do_truncate() and 
further to the notification layer.

Currently do_truncate() calls notify_change() which does not accept a 
path argument.

Here the size change is implemented as an attribute change.
Furthermore time attributes are changed.

What do you exactly mean by
 > just do the separation at the virtual file system level
 > and still call ->setattr() fs callback for now.

Do you want to duplicate the logic of notify_change() to a
new function notify_truncate() which will at least have to handle 
ATTR_SIZE, ATTR_FORCE, ATTR_KILL_SUID, ATTR_KILL_SGID, ATTR_FILE and 
time attributes?

And from notify_truncate() call new function fsnotify_truncate() with a 
logic like fsnotify_modify() but accepting a path?

This would result in a lot of code duplication.

In which respect would such a patch be preferable?

Best regards

Heinrich Schuchardt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/