mbox series

[v2,0/3] fanotify accounting for fs/splice.c

Message ID pw3ljisf6ctpku2o44bdy3aaqdt4ofnedrdt4a4qylhasxsli6@wxhy3nsjcwn4
Headers
Series fanotify accounting for fs/splice.c |

Message

Ahelenia Ziemiańska June 26, 2023, 11:08 p.m. UTC 
  "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.

Ahelenia Ziemiańska (3):
  splice: always fsnotify_access(in), fsnotify_modify(out) on success
  splice: fsnotify_modify(fd) in vmsplice
  splice: fsnotify_access(in), fsnotify_modify(out) on success in tee

 fs/splice.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

Comments

Amir Goldstein June 27, 2023, 6:14 a.m. UTC | #1 
On Tue, Jun 27, 2023 at 2:08 AM наб <nabijaczleweli@nabijaczleweli.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.