linux-kernel - Re: [PATCH v4 1/3] splice: always fsnotify_access(in), fsnotify

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:   Wed, 28 Jun 2023 22:18:53 +0200
From:   Ahelenia Ziemiańska 
        <nabijaczleweli@...ijaczleweli.xyz>
To:     Amir Goldstein <amir73il@...il.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jan Kara <jack@...e.cz>,
        Chung-Chiang Cheng <cccheng@...ology.com>, ltp@...ts.linux.it
Subject: Re: [PATCH v4 1/3] splice: always fsnotify_access(in),
 fsnotify_modify(out) on success

On Wed, Jun 28, 2023 at 09:38:03PM +0300, Amir Goldstein wrote:
> On Wed, Jun 28, 2023 at 8:09 PM Ahelenia Ziemiańska
> <nabijaczleweli@...ijaczleweli.xyz> wrote:
> > On Wed, Jun 28, 2023 at 09:33:43AM +0300, Amir Goldstein wrote:
> > > I think we need to add a rule to fanotify_events_supported() to ban
> > > sb/mount marks on SB_KERNMOUNT and backport this
> > > fix to LTS kernels (I will look into it) and then we can fine tune
> > > the s_fsnotify_connectors optimization in fsnotify_parent() for
> > > the SB_KERNMOUNT special case.
> > > This may be able to save your patch for the faith of NACKed
> > > for performance regression.
> > This goes over my head, but if Jan says it makes sense
> > then it must do.
> Here you go:
> https://github.com/amir73il/linux/commits/fsnotify_pipe
> 
> I ended up using SB_NOUSER which is narrower than
> SB_KERNMOUNT.
> 
> Care to test?
> 1) Functionally - that I did not break your tests.
) | gzip -d > inotify13; chmod +x inotify13; exec ./inotify13
tst_test.c:1560: TINFO: Timeout per run is 0h 00m 30s
inotify13.c:260: TINFO: file_to_pipe
inotify13.c:269: TPASS: ок
inotify13.c:260: TINFO: file_to_pipe
inotify13.c:269: TPASS: ок
inotify13.c:260: TINFO: splice_pipe_to_file
inotify13.c:269: TPASS: ок
inotify13.c:260: TINFO: pipe_to_pipe
inotify13.c:269: TPASS: ок
inotify13.c:260: TINFO: pipe_to_pipe
inotify13.c:269: TPASS: ок
inotify13.c:260: TINFO: vmsplice_pipe_to_mem
inotify13.c:269: TPASS: ок
inotify13.c:260: TINFO: vmsplice_mem_to_pipe
inotify13.c:269: TPASS: ок

Summary:
passed   7
failed   0
broken   0
skipped  0
warnings 0

The discrete tests from before also work as expected,
both to a fifo and an anon pipe.

> 2) Optimization - that when one anon pipe has an inotify watch
> write to another anon pipe stops at fsnotify_inode_has_watchers()
> and does not get to fsnotify().
Yes, I can confirm this as well: fsnotify_parent() only continues to
fsnotify() for the watched pipe; writes to other pipes early-exit.

To validate the counterfactual, I reverted "fsnotify: optimize the case
of anonymous pipe with no watches" and fsnotify() was being called
for each anon pipe write, so long as any anon pipe watches were registered.

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)