linux-kernel - Re: [PATCH v2 0/3] fanotify accounting for fs/splice.c

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]

Message-ID: <CAOQ4uxh7i_s4R9pFJPENALdWGG5-dDhqPLEUXuJqSoHraktFiA@mail.gmail.com>
Date:   Tue, 27 Jun 2023 09:14:21 +0300
From:   Amir Goldstein <amir73il@...il.com>
To:     наб <nabijaczleweli@...ijaczleweli.xyz>
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>
Subject: Re: [PATCH v2 0/3] fanotify accounting for fs/splice.c

On Tue, Jun 27, 2023 at 2:08 AM наб <nabijaczleweli@...ijaczleweli.xyz> wrote:
>
> "people just forget to add inotify hooks to their I/O routines as a rule"?
> Guess what I did, fully knowing that some are missing in this file :)
>
> ==> te.c <==
> #define _GNU_SOURCE
> #include <fcntl.h>
> #include <stdio.h>
> int main() {
>   ssize_t rd, acc = 0;
>   while ((rd = tee(0, 1, 128 * 1024 * 1024, 0)) > 0)
>     acc += rd;
>   fprintf(stderr, "te=%zd: %m\n", acc);
> }
>
> ==> vm.c <==
> #define _GNU_SOURCE
> #include <fcntl.h>
> #include <stdio.h>
> #include <string.h>
> static char sb[1024 * 1024];
> int main() {
>   memcpy(sb, "żupan", sizeof("żupan"));
>   ssize_t rd =
>       vmsplice(1, &(struct iovec){.iov_base = sb, .iov_len = sizeof(sb)}, 1,
>                SPLICE_F_GIFT);
>   fprintf(stderr, "vm=%zd: %m\n", rd);
> }
>
>
> echo zupa | ./te > fifo tees a few times and then blocks when the pipe
> fills, at which point we get into the broken state.
>
> Similarly, ./vm > fifo (with the default 64k F_GETPIPE_SZ) enters that
> same state instantly.
>
> With 2/3 and 3/3, they instead do
>   1: mask=2, cook=0, len=0, name=
>   rd=80
>   1: mask=2, cook=0, len=0, name=
>   rd=80
>   ...
> in a loop, as-expected, and
>   # ./vm > fifo
>   vm=65200: Success
>   1: mask=2, cook=0, len=0, name=
>   rd=65200
>
> I took the liberty of marking 2/3 and 3/3 as Fixes: of the original
> fanotify-in-splice commit as well, I think they fit the bill.
>

Thank you for doing this thorough research on all the variants!

It would be great if you could add test coverage for these syscalls.

Simplest would be to clone an LTP inotify test, e.g.
ltp/testcases/kernel/syscalls/inotify/inotify01

for the different splice syscall variants (sendfile as well).

LTP already has other tests for all those syscalls, so there are plenty
of examples of how to use the LTP helpers to test those syscalls.

You can either clone one inotify test per syscall, or clone one inotify
test that creates a fifo instead of a file that inotify watches
and use a test cases array for the different syscalls to test
(see example of test cases array in inotify10 test).

Thanks,
Amir.