[LTP] [PATCH] syscalls/fanotify: test fanotify_init new flags FAN_EVENT_INFO_TID
Amir Goldstein
amir73il@gmail.com
Tue Sep 25 16:37:58 CEST 2018
On Tue, Sep 25, 2018 at 12:31 PM nixiaoming <nixiaoming@huawei.com> wrote:
>
> fanotify_info_tid:
> test fanotify_init with FAN_EVENT_INFO_TID
> data->pid is ID(pid) of the thread that caused the event
> fanotify_info_tgid:
> test fanotify_init without FAN_EVENT_INFO_TID
> data->pid is ID(tgid) of the process that caused the event
Nixiaoming,
These two tests are identical except for probably a single if condition.
They should be implemented as a single test with two test cases.
See test fanotify10 I posted as an example.
Please name your new test fanotify11. In LTP tests are usually numbered
and not "named".
>
> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
> ---
> testcases/kernel/syscalls/fanotify/Makefile | 3 +-
> .../kernel/syscalls/fanotify/fanotify_info_tgid.c | 200 ++++++++++++++++++++
> .../kernel/syscalls/fanotify/fanotify_info_tid.c | 204 +++++++++++++++++++++
Please see my same patch for required changes for new tests to:
runtest/syscalls
testcases/kernel/syscalls/fanotify/.gitignore
> 3 files changed, 406 insertions(+), 1 deletion(-)
> create mode 100644 testcases/kernel/syscalls/fanotify/fanotify_info_tgid.c
> create mode 100644 testcases/kernel/syscalls/fanotify/fanotify_info_tid.c
>
> diff --git a/testcases/kernel/syscalls/fanotify/Makefile b/testcases/kernel/syscalls/fanotify/Makefile
> index bb58878..d85c755 100644
> --- a/testcases/kernel/syscalls/fanotify/Makefile
> +++ b/testcases/kernel/syscalls/fanotify/Makefile
> @@ -17,7 +17,8 @@
> #
>
> top_srcdir ?= ../../../..
> -
> +fanotify_info_tid: CFLAGS+=-pthread
> +fanotify_info_tgid: CFLAGS+=-pthread
> include $(top_srcdir)/include/mk/testcases.mk
>
> include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify_info_tgid.c b/testcases/kernel/syscalls/fanotify/fanotify_info_tgid.c
> new file mode 100644
> index 0000000..3b10d79
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fanotify/fanotify_info_tgid.c
> @@ -0,0 +1,200 @@
> +/*
> + * Copyright (c) 2013 SUSE. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> + *
> + * Further, this software is distributed without any warranty that it is
> + * free of the rightful claim of any third person regarding infringement
> + * or the like. Any license provided herein, whether implied or
> + * otherwise, applies only to this software file. Patent licenses, if
> + * any, provided herein do not apply to combinations of this program with
> + * other software, or any other product whatsoever.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *
> + * Started by nixiaoming <nixiaoming@huawei.com>
> + *
> + * DESCRIPTION
> + * After fanotify_init without FAN_EVENT_INFO_TID,
> + * check whether the program can only show tgid
> + * in the multithreaded program triggered the event..
> + *
> + * This is a regression test for commit 55c5b2ed10331c3b22:
This is not a regression test. Regression means there was a bug and
a fix and this test verifies a fix.
This test checks for correctness of event->pid value before and after
the new FAN_EVENT_INFO_TID feature.
> + *
> + * fanotify: support reporting thread id instead of process id
> + */
> +#define _GNU_SOURCE
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/syscall.h>
> +#include <pthread.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <linux/limits.h>
> +#include "tst_test.h"
> +#include "fanotify.h"
> +
> +#if defined(HAVE_SYS_FANOTIFY_H)
> +#include <sys/fanotify.h>
> +
> +
> +#define BUF_SIZE 256
> +#define gettid() syscall(SYS_gettid)
> +
> +static char fname[BUF_SIZE];
> +int fd_notify;
> +
> +static int tid;
> +static int tgid;
> +static char tid_file[BUF_SIZE];
> +
> +void *thread_create_file(void *arg LTP_ATTRIBUTE_UNUSED)
> +{
> + int fd;
> +
> + tid = gettid();
> + snprintf(tid_file, sizeof(tid_file), "%s/test_tid_%d", fname, tid);
> + sleep(1);
Why sleep? Can't call thread_create_file() after setting the mark?
> + fd = SAFE_OPEN(tid_file, O_WRONLY|O_CREAT, 0600);
> + tst_res(TINFO, "open file=%s. fd=%d, tid=%i\n", tid_file, fd, tid);
> + SAFE_WRITE(1, fd, tid_file, sizeof(tid_file));
> + tst_res(TINFO, "write file=%s. fd=%d, tid=%i\n", tid_file, fd, tid);
> + SAFE_CLOSE(fd);
> + tst_res(TINFO, "close file=%s. fd=%d, tid=%i\n", tid_file, fd, tid);
SAFE_FILE_PRINTF(tid_file, "1");
instead of all of the above?
and all those TINFO seems too much to me.
> + SAFE_UNLINK(tid_file);
You don't really need to unlink the file - it is created in a temp dir
and even if 2nd iteration uses same tid, events would still be created.
> + tst_res(TINFO, "unlink file=%s. fd=%d, tid=%i\n", tid_file, fd, tid);
> + sleep(1);
> + pthread_exit(0);
> +}
> +
> +static const char* mask2str(uint64_t mask)
> +{
> + static char buffer[10];
> + int offset = 0;
> +
> + if (mask & FAN_ACCESS)
> + buffer[offset++] = 'R';
> + if (mask & FAN_ONDIR)
> + buffer[offset++] = 'D';
> + if (mask & FAN_OPEN)
> + buffer[offset++] = 'O';
> + if (mask & FAN_CLOSE_WRITE || mask & FAN_CLOSE_NOWRITE)
> + buffer[offset++] = 'C';
> + if (mask & FAN_MODIFY || mask & FAN_CLOSE_WRITE)
> + buffer[offset++] = 'W';
> + buffer[offset] = '\0';
> +
> + return buffer;
> +}
> +
> +
> +static void show_data(const struct fanotify_event_metadata *data)
> +{
> + static char printbuf[100];
> + static char pathname[PATH_MAX];
> + struct stat st;
> + int len;
> +
> + snprintf(printbuf, sizeof(printbuf), "/proc/self/fd/%i", data->fd);
> + len = readlink(printbuf, pathname, sizeof(pathname));
> + if (len < 0) {
> + /* fall back to the device/inode */
> + if (fstat(data->fd, &st) < 0) {
> + perror("stat");
> + exit(1);
> + }
> + snprintf(pathname, sizeof(pathname), "device %i:%i inode %ld\n",
> + major(st.st_dev), minor(st.st_dev), st.st_ino);
> + } else {
> + pathname[len] = '\0';
> + }
> + tst_res(TINFO, "envnt: %i %s %s\n", data->pid, mask2str(data->mask),
> + pathname);
> +}
I find all this a bit TMI (too much information) that is not relevant
for this test.
> +
> +void test01(void)
> +{
> + int res;
> + int ret;
> + char buffer[4096] = {0};
You should not define a 4k buffer on the stack but its enough for you
to use a single
struct fanotify_event_metadata event;
as the buffer for this test.
> + struct fanotify_event_metadata *data;
> +
> + ret = fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_CLOSE | FAN_OPEN | FAN_EVENT_ON_CHILD, AT_FDCWD, fname);
You don't really need all these event masks. FAN_MODIFY should do.
> + if (ret != 0) {
> + tst_brk(TBROK, "fanotify_mark (%d, FAN_MARK_ADD, FAN_CLOSE | FAN_OPEN |FAN_EVENT_ON_CHILD, AT_FDCWD, %s) "
> + "failed, ret=%d\n", fd_notify, fname, ret);
> + }
> + tst_res(TINFO, "add notify mark\n");
> + res = SAFE_READ(0, fd_notify, buffer, 4096);
> + data = (struct fanotify_event_metadata *) buffer;
> + while (FAN_EVENT_OK(data, res)) {
> + show_data(data);
> + if (data->pid == tid)
> + tst_res(TFAIL, "data->pid == envent task thread id %u\n", tid);
> + else if (data->pid == tgid)
> + tst_res(TPASS, "data->pid %u != envent task thread id %u\n", data->pid, tid);
> + else
> + tst_res(TINFO, "Environmental interference....\n");
That should definitely result in TFAIL.
> + close(data->fd);
if (data->fd != FAN_NOFD)
SAFE_CLOSE(data->fd);
> + data = FAN_EVENT_NEXT(data, res);
> + }
> + fflush(stdout);
> +
> + /* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
> + ret = fanotify_mark(fd_notify, FAN_MARK_REMOVE, FAN_CLOSE | FAN_OPEN | FAN_EVENT_ON_CHILD, AT_FDCWD, fname);
> + if (ret != 0) {
> + tst_brk(TBROK, "fanotify_mark (%d, FAN_MARK_REMOVE, FAN_CLOSE | FAN_OPEN | FAN_EVENT_ON_CHILD, AT_FDCWD, %s) "
> + "failed, ret=%d\n", fd_notify, fname, ret);
> + }
> + tst_res(TINFO, "remove notify mark\n");
> +}
> +
> +static void setup(void)
> +{
> + pthread_t p_id;
> +
> + tgid = getpid();
> + sprintf(fname, "tfile_%d", tgid);
> + mkdir(fname, 0755);
> +
> + fd_notify = SAFE_FANOTIFY_INIT(0, 0);
To simplify using test cases, better move fanotify_init into the test itself
and keep SAFE_CLOSE(fd_notify) both at end of test and in cleanup().
In each test cases your test would use different fanotify_init flags and
you should specify FAN_CLASS_NOTIF even though is it 0.
> + if (fd_notify < 0)
> + tst_brk(TCONF, "no support\n");
This check is moot because safe_fanotify_init already exists the test
in this case, but for this reason you cannot use SAFE_FANOTIFY_INIT
in your test. You need to use fanotify_init() and check for errno == EINVAL
when you use flag FAN_EVENT_INFO_TID, so you can report TCONF
for feature not supported in kernel - see my change to fanotify01 to
test new init flag FAN_UNPRIVILEGED on branch fanotify_unpriv
in my LTP tree.
> + pthread_create(&p_id, NULL, thread_create_file, NULL);
> +}
> +
Please look into using SAFE_PTHREAD_CREATE
(and SAFE_PTHREAD_JOIN in cleanup()).
Thanks,
Amir.
More information about the ltp
mailing list