[LTP] [PATCH] inotify: clean up build and make check findings

[LTP] [PATCH] inotify: clean up build and make check findings

[Jinseok Kim]

Fix various issues reported by `make` and `make check`:

- Drop ENOSYS-based logic and rely on TERRNO reporting
- Silence known format-truncation warnings locally
- Clean up string handling and permissions
- Fix minor style and prototype issues

Signed-off-by: Jinseok Kim <always.starving0@gmail.com>
---
 testcases/kernel/syscalls/inotify/Makefile    |  1 +
 testcases/kernel/syscalls/inotify/inotify.h   | 13 +++-------
 testcases/kernel/syscalls/inotify/inotify01.c |  4 ++-
 testcases/kernel/syscalls/inotify/inotify02.c | 26 ++++++++++---------
 testcases/kernel/syscalls/inotify/inotify03.c |  5 ++--
 testcases/kernel/syscalls/inotify/inotify04.c | 23 ++++++++--------
 testcases/kernel/syscalls/inotify/inotify05.c |  2 +-
 testcases/kernel/syscalls/inotify/inotify07.c | 16 ++++++------
 testcases/kernel/syscalls/inotify/inotify08.c |  5 ++--
 testcases/kernel/syscalls/inotify/inotify10.c | 18 +++++++------
 10 files changed, 58 insertions(+), 55 deletions(-)

diff --git a/testcases/kernel/syscalls/inotify/Makefile b/testcases/kernel/syscalls/inotify/Makefile
index 16d430864..57124c739 100644
--- a/testcases/kernel/syscalls/inotify/Makefile
+++ b/testcases/kernel/syscalls/inotify/Makefile
@@ -4,6 +4,7 @@
 top_srcdir		?= ../../../..

 include $(top_srcdir)/include/mk/testcases.mk
+inotify02: CFLAGS+=-Wno-format-truncation
 inotify09: CFLAGS+=-pthread
 inotify09: LDLIBS+=-lrt
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/inotify/inotify.h b/testcases/kernel/syscalls/inotify/inotify.h
index dbf814902..1bc36defa 100644
--- a/testcases/kernel/syscalls/inotify/inotify.h
+++ b/testcases/kernel/syscalls/inotify/inotify.h
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
+/* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
  * inotify testcase common definitions.
  *
@@ -33,14 +33,8 @@
 static inline int safe_myinotify_init(const char *file, const int lineno, int fd)
 {
 	if (fd < 0) {
-		if (errno == ENOSYS) {
-			tst_brk(TCONF,
-				"%s:%d: inotify is not configured in this kernel",
+		tst_brk(TBROK | TERRNO, "%s:%d: inotify_init failed",
 				file, lineno);
-		} else {
-			tst_brk(TBROK | TERRNO, "%s:%d: inotify_init failed",
-				file, lineno);
-		}
 	}
 	return fd;
 }
@@ -51,7 +45,8 @@ static inline int safe_myinotify_init(const char *file, const int lineno, int fd
 #define SAFE_MYINOTIFY_INIT1(flags) \
 	safe_myinotify_init(__FILE__, __LINE__, myinotify_init1(flags))

-static inline int safe_myinotify_watch(const char *file, const int lineno, int wd, int fd, const char* fname, const char* mask)
+static inline int safe_myinotify_watch(const char *file, const int lineno, int wd,
+				       int fd, const char *fname, const char *mask)
 {
 	if (wd < 0) {
 		tst_brk(TBROK | TERRNO,
diff --git a/testcases/kernel/syscalls/inotify/inotify01.c b/testcases/kernel/syscalls/inotify/inotify01.c
index 972b1025e..12aadf862 100644
--- a/testcases/kernel/syscalls/inotify/inotify01.c
+++ b/testcases/kernel/syscalls/inotify/inotify01.c
@@ -40,7 +40,7 @@ static unsigned int event_set[EVENT_MAX];

 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int test_cnt = 0;

@@ -79,6 +79,7 @@ void verify_inotify(void)
 	 * get list of events
 	 */
 	int len, i = 0, test_num = 0;
+
 	len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);

 	/*
@@ -86,6 +87,7 @@ void verify_inotify(void)
 	 */
 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
diff --git a/testcases/kernel/syscalls/inotify/inotify02.c b/testcases/kernel/syscalls/inotify/inotify02.c
index 314c1bd49..5f288b270 100644
--- a/testcases/kernel/syscalls/inotify/inotify02.c
+++ b/testcases/kernel/syscalls/inotify/inotify02.c
@@ -43,6 +43,7 @@ struct event_t {
 	char name[BUF_SIZE];
 	unsigned int mask;
 };
+
 #define FILE_NAME1 "test_file1"
 #define FILE_NAME2 "test_file2"

@@ -50,7 +51,7 @@ static struct event_t event_set[EVENT_MAX];

 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	unsigned int stored_cookie = UINT_MAX;

@@ -61,40 +62,40 @@ void verify_inotify(void)
 	 */
 	SAFE_CHMOD(".", 0755);
 	event_set[test_cnt].mask = IN_ISDIR | IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 	test_cnt++;

 	fd = SAFE_CREAT(FILE_NAME1, 0755);
 	event_set[test_cnt].mask = IN_CREATE;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_OPEN;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;

 	SAFE_CLOSE(fd);
 	event_set[test_cnt].mask = IN_CLOSE_WRITE;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;

 	SAFE_RENAME(FILE_NAME1, FILE_NAME2);
 	event_set[test_cnt].mask = IN_MOVED_FROM;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_MOVED_TO;
-	strcpy(event_set[test_cnt].name, FILE_NAME2);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME2);
 	test_cnt++;

 	SAFE_GETCWD(fname1, BUF_SIZE);
 	snprintf(fname2, BUF_SIZE, "%s.rename1", fname1);
 	SAFE_RENAME(fname1, fname2);
 	event_set[test_cnt].mask = IN_MOVE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 	test_cnt++;

 	SAFE_UNLINK(FILE_NAME2);
 	event_set[test_cnt].mask = IN_DELETE;
-	strcpy(event_set[test_cnt].name, FILE_NAME2);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME2);
 	test_cnt++;

 	/*
@@ -108,14 +109,16 @@ void verify_inotify(void)

 	SAFE_RENAME(fname3, fname1);
 	event_set[test_cnt].mask = IN_MOVE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 	test_cnt++;

 	int len, i = 0, test_num = 0;
+
 	len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);

 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
@@ -125,8 +128,7 @@ void verify_inotify(void)
 				event->cookie, event->len, event->len,
 				event->name);

-		} else if ((event_set[test_num].mask == event->mask)
-				&&
+		} else if ((event_set[test_num].mask == event->mask) &&
 				(!strncmp
 				 (event_set[test_num].name, event->name,
 				  event->len))) {
diff --git a/testcases/kernel/syscalls/inotify/inotify03.c b/testcases/kernel/syscalls/inotify/inotify03.c
index a7974dd57..8a5103bd8 100644
--- a/testcases/kernel/syscalls/inotify/inotify03.c
+++ b/testcases/kernel/syscalls/inotify/inotify03.c
@@ -40,12 +40,12 @@ static unsigned int event_set[EVENT_MAX];

 static char event_buf[EVENT_BUF_LEN];

-#define DIR_MODE	(S_IRWXU | S_IRUSR | S_IXUSR | S_IRGRP | S_IXGRP)
+#define DIR_MODE	0750

 static char *mntpoint = "mntpoint";
 static int mount_flag;

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int ret;
 	int len, i, test_num;
@@ -81,6 +81,7 @@ void verify_inotify(void)
 	i = 0;
 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= (test_cnt - 1)) {
 			tst_res(TFAIL,
diff --git a/testcases/kernel/syscalls/inotify/inotify04.c b/testcases/kernel/syscalls/inotify/inotify04.c
index 947623952..18a1c4a6e 100644
--- a/testcases/kernel/syscalls/inotify/inotify04.c
+++ b/testcases/kernel/syscalls/inotify/inotify04.c
@@ -35,7 +35,6 @@
 /* reasonable guess as to size of 1024 events */
 #define EVENT_BUF_LEN        (EVENT_MAX * (EVENT_SIZE + 16))

-
 #define BUF_SIZE 256

 struct event_t {
@@ -46,11 +45,11 @@ struct event_t {
 #define	TEST_DIR	"test_dir"
 #define	TEST_FILE	"test_file"

-struct event_t event_set[EVENT_MAX];
+static struct event_t event_set[EVENT_MAX];

-char event_buf[EVENT_BUF_LEN];
+static char event_buf[EVENT_BUF_LEN];

-int fd_notify, reap_wd_file, reap_wd_dir, wd_dir, wd_file;
+static int fd_notify, reap_wd_file, reap_wd_dir, wd_dir, wd_file;

 static void cleanup(void)
 {
@@ -73,7 +72,7 @@ static void setup(void)
 	fd_notify = SAFE_MYINOTIFY_INIT();
 }

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int i = 0, test_num = 0, len;
 	int test_cnt = 0;
@@ -91,10 +90,10 @@ void verify_inotify(void)
 	reap_wd_dir = 0;

 	event_set[test_cnt].mask = IN_DELETE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 	test_cnt++;
 	event_set[test_cnt].mask = IN_IGNORED;
-	strcpy(event_set[test_cnt].name, "");
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 	test_cnt++;

 	SAFE_UNLINK(TEST_FILE);
@@ -108,20 +107,21 @@ void verify_inotify(void)
 	 * understand how Unix works.
 	 */
 	event_set[test_cnt].mask = IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 	test_cnt++;

 	event_set[test_cnt].mask = IN_DELETE_SELF;
-	strcpy(event_set[test_cnt].name, TEST_FILE);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", TEST_FILE);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_IGNORED;
-	strcpy(event_set[test_cnt].name, "");
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 	test_cnt++;

 	len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);

 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
@@ -130,8 +130,7 @@ void verify_inotify(void)
 				"name=\"%.*s\"", event->wd, event->mask,
 				event->cookie, event->len, event->len, event->name);

-		} else if ((event_set[test_num].mask == event->mask)
-			   &&
+		} else if ((event_set[test_num].mask == event->mask) &&
 			   (!strncmp
 			    (event_set[test_num].name, event->name,
 			     event->len))) {
diff --git a/testcases/kernel/syscalls/inotify/inotify05.c b/testcases/kernel/syscalls/inotify/inotify05.c
index 82a4c7bdc..1a38056f0 100644
--- a/testcases/kernel/syscalls/inotify/inotify05.c
+++ b/testcases/kernel/syscalls/inotify/inotify05.c
@@ -36,7 +36,7 @@ static int max_events;

 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int i;
 	int len, stop;
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index b4000f353..258e14b92 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -65,7 +65,7 @@ static const char mntpoint[] = OVL_BASE_MNTPOINT;
 static struct event_t event_set[EVENT_MAX];
 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int test_cnt = 0;

@@ -74,25 +74,26 @@ void verify_inotify(void)
 	 */
 	SAFE_CHMOD(DIR_PATH, 0755);
 	event_set[test_cnt].mask = IN_ISDIR | IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 	test_cnt++;

 	SAFE_TOUCH(FILE_PATH, 0644, NULL);
 	event_set[test_cnt].mask = IN_OPEN;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_CLOSE_WRITE;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;

 	int len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
-
 	int i = 0, test_num = 0;
+
 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
@@ -101,8 +102,7 @@ void verify_inotify(void)
 				"name=\"%.*s\"", event->wd, event->mask,
 				event->cookie, event->len, event->len,
 				event->name);
-		} else if ((event_set[test_num].mask == event->mask)
-				&&
+		} else if ((event_set[test_num].mask == event->mask) &&
 				(!strncmp
 				 (event_set[test_num].name, event->name,
 				  event->len))) {
diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
index e0837cac3..154654ce3 100644
--- a/testcases/kernel/syscalls/inotify/inotify08.c
+++ b/testcases/kernel/syscalls/inotify/inotify08.c
@@ -63,7 +63,7 @@ static const char mntpoint[] = OVL_BASE_MNTPOINT;
 static struct event_t event_set[EVENT_MAX];
 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int test_cnt = 0;

@@ -87,10 +87,11 @@ void verify_inotify(void)
 	SAFE_TOUCH(OVL_UPPER"/"FILE_NAME, 0644, NULL);

 	int len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
-
 	int i = 0, test_num = 0;
+
 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
diff --git a/testcases/kernel/syscalls/inotify/inotify10.c b/testcases/kernel/syscalls/inotify/inotify10.c
index 4c3a1d116..838ae29a3 100644
--- a/testcases/kernel/syscalls/inotify/inotify10.c
+++ b/testcases/kernel/syscalls/inotify/inotify10.c
@@ -32,7 +32,6 @@
 #define EVENT_SIZE  (sizeof(struct inotify_event))
 #define EVENT_BUF_LEN        (EVENT_MAX * (EVENT_SIZE + 16))

-
 #define BUF_SIZE 256

 struct event_t {
@@ -75,11 +74,11 @@ static struct tcase {
 	},
 };

-struct event_t event_set[EVENT_MAX];
+static struct event_t event_set[EVENT_MAX];

-char event_buf[EVENT_BUF_LEN];
+static char event_buf[EVENT_BUF_LEN];

-int fd_notify, fd_notify_other;
+static int fd_notify, fd_notify_other;

 static void verify_inotify(unsigned int n)
 {
@@ -121,25 +120,27 @@ static void verify_inotify(unsigned int n)
 	if (wd_parent && (tc->parent_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_parent;
 		event_set[test_cnt].mask = tc->parent_mask | IN_ISDIR;
-		strcpy(event_set[test_cnt].name, TEST_DIR);
+		snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name),
+			 "%s", TEST_DIR);
 		test_cnt++;
 	}
 	if (wd_subdir && (tc->subdir_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_subdir;
 		event_set[test_cnt].mask = tc->subdir_mask | IN_ISDIR;
-		strcpy(event_set[test_cnt].name, "");
+		snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 		test_cnt++;
 	}
 	if (wd_parent && (tc->parent_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_parent;
 		event_set[test_cnt].mask = tc->parent_mask;
-		strcpy(event_set[test_cnt].name, TEST_FILE);
+		snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name),
+			 "%s", TEST_FILE);
 		test_cnt++;
 	}
 	if (wd_child && (tc->child_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_child;
 		event_set[test_cnt].mask = tc->child_mask;
-		strcpy(event_set[test_cnt].name, "");
+		snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");
 		test_cnt++;
 	}

@@ -148,6 +149,7 @@ static void verify_inotify(unsigned int n)
 	while (i < len) {
 		struct event_t *expected = &event_set[test_num];
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
--
2.43.0

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] inotify: clean up build and make check findings

[Andrea Cervesato via ltp]

Hi!

> Fix various issues reported by `make` and `make check`:
> 
> - Drop ENOSYS-based logic and rely on TERRNO reporting
> - Silence known format-truncation warnings locally
> - Clean up string handling and permissions
> - Fix minor style and prototype issues

This is a lot of stuff for a single commit :-) we might need to separate
these changes into multiple commits.

> 
> Signed-off-by: Jinseok Kim <always.starving0@gmail.com>
> ---
>  testcases/kernel/syscalls/inotify/Makefile    |  1 +
>  testcases/kernel/syscalls/inotify/inotify.h   | 13 +++-------
>  testcases/kernel/syscalls/inotify/inotify01.c |  4 ++-
>  testcases/kernel/syscalls/inotify/inotify02.c | 26 ++++++++++---------
>  testcases/kernel/syscalls/inotify/inotify03.c |  5 ++--
>  testcases/kernel/syscalls/inotify/inotify04.c | 23 ++++++++--------
>  testcases/kernel/syscalls/inotify/inotify05.c |  2 +-
>  testcases/kernel/syscalls/inotify/inotify07.c | 16 ++++++------
>  testcases/kernel/syscalls/inotify/inotify08.c |  5 ++--
>  testcases/kernel/syscalls/inotify/inotify10.c | 18 +++++++------
>  10 files changed, 58 insertions(+), 55 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/inotify/Makefile b/testcases/kernel/syscalls/inotify/Makefile
> index 16d430864..57124c739 100644
> --- a/testcases/kernel/syscalls/inotify/Makefile
> +++ b/testcases/kernel/syscalls/inotify/Makefile
> @@ -4,6 +4,7 @@
>  top_srcdir		?= ../../../..
> 
>  include $(top_srcdir)/include/mk/testcases.mk
> +inotify02: CFLAGS+=-Wno-format-truncation

uhm? Is it really needed?

>  inotify09: CFLAGS+=-pthread
>  inotify09: LDLIBS+=-lrt
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/inotify/inotify.h b/testcases/kernel/syscalls/inotify/inotify.h
> index dbf814902..1bc36defa 100644
> --- a/testcases/kernel/syscalls/inotify/inotify.h
> +++ b/testcases/kernel/syscalls/inotify/inotify.h
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>  /*
>   * inotify testcase common definitions.
>   *
> @@ -33,14 +33,8 @@
>  static inline int safe_myinotify_init(const char *file, const int lineno, int fd)
>  {
>  	if (fd < 0) {
> -		if (errno == ENOSYS) {
> -			tst_brk(TCONF,
> -				"%s:%d: inotify is not configured in this kernel",

Here we are suppressing a TCONF and threating everything as TERRNO. This
needs to be reverted.

> +		tst_brk(TBROK | TERRNO, "%s:%d: inotify_init failed",
>  				file, lineno);
> -		} else {
> -			tst_brk(TBROK | TERRNO, "%s:%d: inotify_init failed",
> -				file, lineno);
> -		}
>  	}
>  	return fd;
>  }
> @@ -51,7 +45,8 @@ static inline int safe_myinotify_init(const char *file, const int lineno, int fd
>  #define SAFE_MYINOTIFY_INIT1(flags) \
>  	safe_myinotify_init(__FILE__, __LINE__, myinotify_init1(flags))
> 
> -static inline int safe_myinotify_watch(const char *file, const int lineno, int wd, int fd, const char* fname, const char* mask)
> +static inline int safe_myinotify_watch(const char *file, const int lineno, int wd,
> +				       int fd, const char *fname, const char *mask)
>  {
>  	if (wd < 0) {
>  		tst_brk(TBROK | TERRNO,
> diff --git a/testcases/kernel/syscalls/inotify/inotify01.c b/testcases/kernel/syscalls/inotify/inotify01.c
> index 972b1025e..12aadf862 100644
> --- a/testcases/kernel/syscalls/inotify/inotify01.c
> +++ b/testcases/kernel/syscalls/inotify/inotify01.c
> @@ -40,7 +40,7 @@ static unsigned int event_set[EVENT_MAX];
> 
>  static char event_buf[EVENT_BUF_LEN];
> 
> -void verify_inotify(void)
> +static void verify_inotify(void)
>  {
>  	int test_cnt = 0;
> 
> @@ -79,6 +79,7 @@ void verify_inotify(void)
>  	 * get list of events
>  	 */
>  	int len, i = 0, test_num = 0;
> +
>  	len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
> 
>  	/*
> @@ -86,6 +87,7 @@ void verify_inotify(void)
>  	 */
>  	while (i < len) {
>  		struct inotify_event *event;
> +
>  		event = (struct inotify_event *)&event_buf[i];
>  		if (test_num >= test_cnt) {
>  			tst_res(TFAIL,
> diff --git a/testcases/kernel/syscalls/inotify/inotify02.c b/testcases/kernel/syscalls/inotify/inotify02.c
> index 314c1bd49..5f288b270 100644
> --- a/testcases/kernel/syscalls/inotify/inotify02.c
> +++ b/testcases/kernel/syscalls/inotify/inotify02.c
> @@ -43,6 +43,7 @@ struct event_t {
>  	char name[BUF_SIZE];
>  	unsigned int mask;
>  };
> +
>  #define FILE_NAME1 "test_file1"
>  #define FILE_NAME2 "test_file2"
> 
> @@ -50,7 +51,7 @@ static struct event_t event_set[EVENT_MAX];
> 
>  static char event_buf[EVENT_BUF_LEN];
> 
> -void verify_inotify(void)
> +static void verify_inotify(void)
>  {
>  	unsigned int stored_cookie = UINT_MAX;
> 
> @@ -61,40 +62,40 @@ void verify_inotify(void)
>  	 */
>  	SAFE_CHMOD(".", 0755);
>  	event_set[test_cnt].mask = IN_ISDIR | IN_ATTRIB;
> -	strcpy(event_set[test_cnt].name, "");
> +	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", "");

This is quite an overkill. To reset a string in C you just need

  buff[0] = '\0';

And the same for the others.. also make sure that boundaries are
correct, because I have the feeling we might easily overbound memories
when using snprintf() here.

Kind regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH] inotify: clean up build and make check findings

[Jinseok Kim]

Hi!

Thanks for the review.

For the -Wformat-truncation warning in inotify02.c, how about fixing it
by adjusting the snprintf() call?

For example:
  snprintf(fname3, BUF_SIZE, "%s.rename2", fname1);
  snprintf(fname3, BUF_SIZE, "%.*s.rename2", (int)(BUF_SIZE -
                                            sizeof(".rename2")), fname1);

Also, regarding the make check warning about ENOSYS: if I revert the code,
the warning will remain. Should this just be left as-is, or is there
another way to handle it?

  inotify.h:36: WARNING: ENOSYS means 'invalid syscall nr' and nothing else

Thanks,
Jinseok.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] inotify: clean up build and make check findings

[Cyril Hrubis]

Hi!
> For the -Wformat-truncation warning in inotify02.c, how about fixing it
> by adjusting the snprintf() call?
> 
> For example:
>   snprintf(fname3, BUF_SIZE, "%s.rename2", fname1);
>   snprintf(fname3, BUF_SIZE, "%.*s.rename2", (int)(BUF_SIZE -
>                                             sizeof(".rename2")), fname1);
> 
> Also, regarding the make check warning about ENOSYS: if I revert the code,
> the warning will remain. Should this just be left as-is, or is there
> another way to handle it?
> 
>   inotify.h:36: WARNING: ENOSYS means 'invalid syscall nr' and nothing else

The easiest solution is to make fname2 and fname3 array larger:

diff --git a/testcases/kernel/syscalls/inotify/inotify02.c b/testcases/kernel/syscalls/inotify/inotify02.c
index 314c1bd49..21a29676a 100644
--- a/testcases/kernel/syscalls/inotify/inotify02.c
+++ b/testcases/kernel/syscalls/inotify/inotify02.c
@@ -35,7 +35,7 @@
 #define EVENT_BUF_LEN        (EVENT_MAX * (EVENT_SIZE + 16))

 #define BUF_SIZE 256
-static char fname1[BUF_SIZE], fname2[BUF_SIZE], fname3[BUF_SIZE];
+static char fname1[BUF_SIZE], fname2[BUF_SIZE+32], fname3[BUF_SIZE + 32];
 static int fd, fd_notify, reap_wd;
 static int wd;

@@ -86,7 +86,7 @@ void verify_inotify(void)
        test_cnt++;

        SAFE_GETCWD(fname1, BUF_SIZE);
-       snprintf(fname2, BUF_SIZE, "%s.rename1", fname1);
+       snprintf(fname2, sizeof(fname2), "%s.rename1", fname1);
        SAFE_RENAME(fname1, fname2);
        event_set[test_cnt].mask = IN_MOVE_SELF;
        strcpy(event_set[test_cnt].name, "");
@@ -103,7 +103,7 @@ void verify_inotify(void)
         * we can correct determine kernel bug which exist before
         * 2.6.25. See comment below.
         */
-       snprintf(fname3, BUF_SIZE, "%s.rename2", fname1);
+       snprintf(fname3, sizeof(fname3), "%s.rename2", fname1);
        SAFE_RENAME(fname2, fname3);

        SAFE_RENAME(fname3, fname1);


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH] inotify: clean up build and make check findings

[Jinseok Kim]

Hi,

That makes sense. I'll update the arrays as suggested.

Thanks,
Jinseok.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/2] inotify: replace strcpy() with snprintf()

[Jinseok Kim]

Use snprintf() instead of strcpy() to avoid potential buffer
overflows when constructing file names.

Signed-off-by: Jinseok Kim <always.starving0@gmail.com>
---
 testcases/kernel/syscalls/inotify/inotify02.c | 24 +++++++++----------
 testcases/kernel/syscalls/inotify/inotify04.c | 10 ++++----
 testcases/kernel/syscalls/inotify/inotify07.c |  8 +++----
 testcases/kernel/syscalls/inotify/inotify10.c | 10 ++++----
 4 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/testcases/kernel/syscalls/inotify/inotify02.c b/testcases/kernel/syscalls/inotify/inotify02.c
index 314c1bd49..423676bf9 100644
--- a/testcases/kernel/syscalls/inotify/inotify02.c
+++ b/testcases/kernel/syscalls/inotify/inotify02.c
@@ -35,7 +35,7 @@
 #define EVENT_BUF_LEN        (EVENT_MAX * (EVENT_SIZE + 16))

 #define BUF_SIZE 256
-static char fname1[BUF_SIZE], fname2[BUF_SIZE], fname3[BUF_SIZE];
+static char fname1[BUF_SIZE], fname2[BUF_SIZE+32], fname3[BUF_SIZE+32];
 static int fd, fd_notify, reap_wd;
 static int wd;

@@ -61,40 +61,40 @@ void verify_inotify(void)
 	 */
 	SAFE_CHMOD(".", 0755);
 	event_set[test_cnt].mask = IN_ISDIR | IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	fd = SAFE_CREAT(FILE_NAME1, 0755);
 	event_set[test_cnt].mask = IN_CREATE;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_OPEN;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;

 	SAFE_CLOSE(fd);
 	event_set[test_cnt].mask = IN_CLOSE_WRITE;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;

 	SAFE_RENAME(FILE_NAME1, FILE_NAME2);
 	event_set[test_cnt].mask = IN_MOVED_FROM;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_MOVED_TO;
-	strcpy(event_set[test_cnt].name, FILE_NAME2);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME2);
 	test_cnt++;

 	SAFE_GETCWD(fname1, BUF_SIZE);
-	snprintf(fname2, BUF_SIZE, "%s.rename1", fname1);
+	snprintf(fname2, sizeof(fname2), "%s.rename1", fname1);
 	SAFE_RENAME(fname1, fname2);
 	event_set[test_cnt].mask = IN_MOVE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	SAFE_UNLINK(FILE_NAME2);
 	event_set[test_cnt].mask = IN_DELETE;
-	strcpy(event_set[test_cnt].name, FILE_NAME2);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME2);
 	test_cnt++;

 	/*
@@ -103,12 +103,12 @@ void verify_inotify(void)
 	 * we can correct determine kernel bug which exist before
 	 * 2.6.25. See comment below.
 	 */
-	snprintf(fname3, BUF_SIZE, "%s.rename2", fname1);
+	snprintf(fname3, sizeof(fname3), "%s.rename2", fname1);
 	SAFE_RENAME(fname2, fname3);

 	SAFE_RENAME(fname3, fname1);
 	event_set[test_cnt].mask = IN_MOVE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	int len, i = 0, test_num = 0;
diff --git a/testcases/kernel/syscalls/inotify/inotify04.c b/testcases/kernel/syscalls/inotify/inotify04.c
index 947623952..2d7f34bae 100644
--- a/testcases/kernel/syscalls/inotify/inotify04.c
+++ b/testcases/kernel/syscalls/inotify/inotify04.c
@@ -91,10 +91,10 @@ void verify_inotify(void)
 	reap_wd_dir = 0;

 	event_set[test_cnt].mask = IN_DELETE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;
 	event_set[test_cnt].mask = IN_IGNORED;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	SAFE_UNLINK(TEST_FILE);
@@ -108,14 +108,14 @@ void verify_inotify(void)
 	 * understand how Unix works.
 	 */
 	event_set[test_cnt].mask = IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	event_set[test_cnt].mask = IN_DELETE_SELF;
-	strcpy(event_set[test_cnt].name, TEST_FILE);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", TEST_FILE);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_IGNORED;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index b4000f353..f0acd9e91 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -74,18 +74,18 @@ void verify_inotify(void)
 	 */
 	SAFE_CHMOD(DIR_PATH, 0755);
 	event_set[test_cnt].mask = IN_ISDIR | IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	SAFE_TOUCH(FILE_PATH, 0644, NULL);
 	event_set[test_cnt].mask = IN_OPEN;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_CLOSE_WRITE;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;

 	int len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
diff --git a/testcases/kernel/syscalls/inotify/inotify10.c b/testcases/kernel/syscalls/inotify/inotify10.c
index 4c3a1d116..0a94ead15 100644
--- a/testcases/kernel/syscalls/inotify/inotify10.c
+++ b/testcases/kernel/syscalls/inotify/inotify10.c
@@ -121,25 +121,27 @@ static void verify_inotify(unsigned int n)
 	if (wd_parent && (tc->parent_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_parent;
 		event_set[test_cnt].mask = tc->parent_mask | IN_ISDIR;
-		strcpy(event_set[test_cnt].name, TEST_DIR);
+		snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name),
+			 "%s", TEST_DIR);
 		test_cnt++;
 	}
 	if (wd_subdir && (tc->subdir_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_subdir;
 		event_set[test_cnt].mask = tc->subdir_mask | IN_ISDIR;
-		strcpy(event_set[test_cnt].name, "");
+		event_set[test_cnt].name[0] = '\0';
 		test_cnt++;
 	}
 	if (wd_parent && (tc->parent_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_parent;
 		event_set[test_cnt].mask = tc->parent_mask;
-		strcpy(event_set[test_cnt].name, TEST_FILE);
+		snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name),
+			 "%s", TEST_FILE);
 		test_cnt++;
 	}
 	if (wd_child && (tc->child_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_child;
 		event_set[test_cnt].mask = tc->child_mask;
-		strcpy(event_set[test_cnt].name, "");
+		event_set[test_cnt].name[0] = '\0';
 		test_cnt++;
 	}

--
2.43.0

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/2] inotify: fix make check findings

[Jinseok Kim]

Fix various issues reported by `make check` in inotify syscall tests:

- Fix SPDX license comment style
- Make local functions and variables static
- Replace DIR_MODE macro with an octal permission value
- Fix minor style and prototype issues

Signed-off-by: Jinseok Kim <always.starving0@gmail.com>
---
 testcases/kernel/syscalls/inotify/inotify.h   |  5 +++--
 testcases/kernel/syscalls/inotify/inotify01.c |  4 +++-
 testcases/kernel/syscalls/inotify/inotify02.c |  8 +++++---
 testcases/kernel/syscalls/inotify/inotify03.c |  5 +++--
 testcases/kernel/syscalls/inotify/inotify04.c | 13 ++++++-------
 testcases/kernel/syscalls/inotify/inotify05.c |  2 +-
 testcases/kernel/syscalls/inotify/inotify07.c |  8 ++++----
 testcases/kernel/syscalls/inotify/inotify08.c |  5 +++--
 testcases/kernel/syscalls/inotify/inotify10.c |  8 ++++----
 9 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/testcases/kernel/syscalls/inotify/inotify.h b/testcases/kernel/syscalls/inotify/inotify.h
index dbf814902..7ac4f4908 100644
--- a/testcases/kernel/syscalls/inotify/inotify.h
+++ b/testcases/kernel/syscalls/inotify/inotify.h
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
+/* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
  * inotify testcase common definitions.
  *
@@ -51,7 +51,8 @@ static inline int safe_myinotify_init(const char *file, const int lineno, int fd
 #define SAFE_MYINOTIFY_INIT1(flags) \
 	safe_myinotify_init(__FILE__, __LINE__, myinotify_init1(flags))

-static inline int safe_myinotify_watch(const char *file, const int lineno, int wd, int fd, const char* fname, const char* mask)
+static inline int safe_myinotify_watch(const char *file, const int lineno, int wd,
+				       int fd, const char *fname, const char *mask)
 {
 	if (wd < 0) {
 		tst_brk(TBROK | TERRNO,
diff --git a/testcases/kernel/syscalls/inotify/inotify01.c b/testcases/kernel/syscalls/inotify/inotify01.c
index 972b1025e..12aadf862 100644
--- a/testcases/kernel/syscalls/inotify/inotify01.c
+++ b/testcases/kernel/syscalls/inotify/inotify01.c
@@ -40,7 +40,7 @@ static unsigned int event_set[EVENT_MAX];

 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int test_cnt = 0;

@@ -79,6 +79,7 @@ void verify_inotify(void)
 	 * get list of events
 	 */
 	int len, i = 0, test_num = 0;
+
 	len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);

 	/*
@@ -86,6 +87,7 @@ void verify_inotify(void)
 	 */
 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
diff --git a/testcases/kernel/syscalls/inotify/inotify02.c b/testcases/kernel/syscalls/inotify/inotify02.c
index 423676bf9..67e01d14a 100644
--- a/testcases/kernel/syscalls/inotify/inotify02.c
+++ b/testcases/kernel/syscalls/inotify/inotify02.c
@@ -43,6 +43,7 @@ struct event_t {
 	char name[BUF_SIZE];
 	unsigned int mask;
 };
+
 #define FILE_NAME1 "test_file1"
 #define FILE_NAME2 "test_file2"

@@ -50,7 +51,7 @@ static struct event_t event_set[EVENT_MAX];

 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	unsigned int stored_cookie = UINT_MAX;

@@ -112,10 +113,12 @@ void verify_inotify(void)
 	test_cnt++;

 	int len, i = 0, test_num = 0;
+
 	len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);

 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
@@ -125,8 +128,7 @@ void verify_inotify(void)
 				event->cookie, event->len, event->len,
 				event->name);

-		} else if ((event_set[test_num].mask == event->mask)
-				&&
+		} else if ((event_set[test_num].mask == event->mask) &&
 				(!strncmp
 				 (event_set[test_num].name, event->name,
 				  event->len))) {
diff --git a/testcases/kernel/syscalls/inotify/inotify03.c b/testcases/kernel/syscalls/inotify/inotify03.c
index a7974dd57..8a5103bd8 100644
--- a/testcases/kernel/syscalls/inotify/inotify03.c
+++ b/testcases/kernel/syscalls/inotify/inotify03.c
@@ -40,12 +40,12 @@ static unsigned int event_set[EVENT_MAX];

 static char event_buf[EVENT_BUF_LEN];

-#define DIR_MODE	(S_IRWXU | S_IRUSR | S_IXUSR | S_IRGRP | S_IXGRP)
+#define DIR_MODE	0750

 static char *mntpoint = "mntpoint";
 static int mount_flag;

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int ret;
 	int len, i, test_num;
@@ -81,6 +81,7 @@ void verify_inotify(void)
 	i = 0;
 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= (test_cnt - 1)) {
 			tst_res(TFAIL,
diff --git a/testcases/kernel/syscalls/inotify/inotify04.c b/testcases/kernel/syscalls/inotify/inotify04.c
index 2d7f34bae..ed6ba5d36 100644
--- a/testcases/kernel/syscalls/inotify/inotify04.c
+++ b/testcases/kernel/syscalls/inotify/inotify04.c
@@ -35,7 +35,6 @@
 /* reasonable guess as to size of 1024 events */
 #define EVENT_BUF_LEN        (EVENT_MAX * (EVENT_SIZE + 16))

-
 #define BUF_SIZE 256

 struct event_t {
@@ -46,11 +45,11 @@ struct event_t {
 #define	TEST_DIR	"test_dir"
 #define	TEST_FILE	"test_file"

-struct event_t event_set[EVENT_MAX];
+static struct event_t event_set[EVENT_MAX];

-char event_buf[EVENT_BUF_LEN];
+static char event_buf[EVENT_BUF_LEN];

-int fd_notify, reap_wd_file, reap_wd_dir, wd_dir, wd_file;
+static int fd_notify, reap_wd_file, reap_wd_dir, wd_dir, wd_file;

 static void cleanup(void)
 {
@@ -73,7 +72,7 @@ static void setup(void)
 	fd_notify = SAFE_MYINOTIFY_INIT();
 }

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int i = 0, test_num = 0, len;
 	int test_cnt = 0;
@@ -122,6 +121,7 @@ void verify_inotify(void)

 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
@@ -130,8 +130,7 @@ void verify_inotify(void)
 				"name=\"%.*s\"", event->wd, event->mask,
 				event->cookie, event->len, event->len, event->name);

-		} else if ((event_set[test_num].mask == event->mask)
-			   &&
+		} else if ((event_set[test_num].mask == event->mask) &&
 			   (!strncmp
 			    (event_set[test_num].name, event->name,
 			     event->len))) {
diff --git a/testcases/kernel/syscalls/inotify/inotify05.c b/testcases/kernel/syscalls/inotify/inotify05.c
index 82a4c7bdc..1a38056f0 100644
--- a/testcases/kernel/syscalls/inotify/inotify05.c
+++ b/testcases/kernel/syscalls/inotify/inotify05.c
@@ -36,7 +36,7 @@ static int max_events;

 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int i;
 	int len, stop;
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index f0acd9e91..3da45fc6c 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -65,7 +65,7 @@ static const char mntpoint[] = OVL_BASE_MNTPOINT;
 static struct event_t event_set[EVENT_MAX];
 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int test_cnt = 0;

@@ -89,10 +89,11 @@ void verify_inotify(void)
 	test_cnt++;

 	int len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
-
 	int i = 0, test_num = 0;
+
 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
@@ -101,8 +102,7 @@ void verify_inotify(void)
 				"name=\"%.*s\"", event->wd, event->mask,
 				event->cookie, event->len, event->len,
 				event->name);
-		} else if ((event_set[test_num].mask == event->mask)
-				&&
+		} else if ((event_set[test_num].mask == event->mask) &&
 				(!strncmp
 				 (event_set[test_num].name, event->name,
 				  event->len))) {
diff --git a/testcases/kernel/syscalls/inotify/inotify08.c b/testcases/kernel/syscalls/inotify/inotify08.c
index e0837cac3..154654ce3 100644
--- a/testcases/kernel/syscalls/inotify/inotify08.c
+++ b/testcases/kernel/syscalls/inotify/inotify08.c
@@ -63,7 +63,7 @@ static const char mntpoint[] = OVL_BASE_MNTPOINT;
 static struct event_t event_set[EVENT_MAX];
 static char event_buf[EVENT_BUF_LEN];

-void verify_inotify(void)
+static void verify_inotify(void)
 {
 	int test_cnt = 0;

@@ -87,10 +87,11 @@ void verify_inotify(void)
 	SAFE_TOUCH(OVL_UPPER"/"FILE_NAME, 0644, NULL);

 	int len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
-
 	int i = 0, test_num = 0;
+
 	while (i < len) {
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
diff --git a/testcases/kernel/syscalls/inotify/inotify10.c b/testcases/kernel/syscalls/inotify/inotify10.c
index 0a94ead15..80b7a825b 100644
--- a/testcases/kernel/syscalls/inotify/inotify10.c
+++ b/testcases/kernel/syscalls/inotify/inotify10.c
@@ -32,7 +32,6 @@
 #define EVENT_SIZE  (sizeof(struct inotify_event))
 #define EVENT_BUF_LEN        (EVENT_MAX * (EVENT_SIZE + 16))

-
 #define BUF_SIZE 256

 struct event_t {
@@ -75,11 +74,11 @@ static struct tcase {
 	},
 };

-struct event_t event_set[EVENT_MAX];
+static struct event_t event_set[EVENT_MAX];

-char event_buf[EVENT_BUF_LEN];
+static char event_buf[EVENT_BUF_LEN];

-int fd_notify, fd_notify_other;
+static int fd_notify, fd_notify_other;

 static void verify_inotify(unsigned int n)
 {
@@ -150,6 +149,7 @@ static void verify_inotify(unsigned int n)
 	while (i < len) {
 		struct event_t *expected = &event_set[test_num];
 		struct inotify_event *event;
+
 		event = (struct inotify_event *)&event_buf[i];
 		if (test_num >= test_cnt) {
 			tst_res(TFAIL,
--
2.43.0

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/2] inotify: fix make check findings

[Andrea Cervesato via ltp]

LGTM

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] inotify: replace strcpy() with snprintf()

[Andrea Cervesato via ltp]

Hi!

>  #define BUF_SIZE 256
> -static char fname1[BUF_SIZE], fname2[BUF_SIZE], fname3[BUF_SIZE];
> +static char fname1[BUF_SIZE], fname2[BUF_SIZE+32], fname3[BUF_SIZE+32];

This is a hack around the actual PATH_MAX feature. We need to use that one
instead of defining a new BUF_SIZE that might overflow.

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3] inotify: replace strcpy() with snprintf()

[Jinseok Kim]

Use snprintf() instead of strcpy() to avoid potential buffer
overflows when constructing file names.

Signed-off-by: Jinseok Kim <always.starving0@gmail.com>
---
 testcases/kernel/syscalls/inotify/inotify02.c | 24 +++++++++----------
 testcases/kernel/syscalls/inotify/inotify04.c | 10 ++++----
 testcases/kernel/syscalls/inotify/inotify07.c |  8 +++----
 testcases/kernel/syscalls/inotify/inotify10.c | 10 ++++----
 4 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/testcases/kernel/syscalls/inotify/inotify02.c b/testcases/kernel/syscalls/inotify/inotify02.c
index 314c1bd49..61217cac3 100644
--- a/testcases/kernel/syscalls/inotify/inotify02.c
+++ b/testcases/kernel/syscalls/inotify/inotify02.c
@@ -35,7 +35,7 @@
 #define EVENT_BUF_LEN        (EVENT_MAX * (EVENT_SIZE + 16))

 #define BUF_SIZE 256
-static char fname1[BUF_SIZE], fname2[BUF_SIZE], fname3[BUF_SIZE];
+static char fname1[BUF_SIZE], fname2[PATH_MAX], fname3[PATH_MAX];
 static int fd, fd_notify, reap_wd;
 static int wd;

@@ -61,40 +61,40 @@ void verify_inotify(void)
 	 */
 	SAFE_CHMOD(".", 0755);
 	event_set[test_cnt].mask = IN_ISDIR | IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	fd = SAFE_CREAT(FILE_NAME1, 0755);
 	event_set[test_cnt].mask = IN_CREATE;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_OPEN;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;

 	SAFE_CLOSE(fd);
 	event_set[test_cnt].mask = IN_CLOSE_WRITE;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;

 	SAFE_RENAME(FILE_NAME1, FILE_NAME2);
 	event_set[test_cnt].mask = IN_MOVED_FROM;
-	strcpy(event_set[test_cnt].name, FILE_NAME1);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME1);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_MOVED_TO;
-	strcpy(event_set[test_cnt].name, FILE_NAME2);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME2);
 	test_cnt++;

 	SAFE_GETCWD(fname1, BUF_SIZE);
-	snprintf(fname2, BUF_SIZE, "%s.rename1", fname1);
+	snprintf(fname2, sizeof(fname2), "%s.rename1", fname1);
 	SAFE_RENAME(fname1, fname2);
 	event_set[test_cnt].mask = IN_MOVE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	SAFE_UNLINK(FILE_NAME2);
 	event_set[test_cnt].mask = IN_DELETE;
-	strcpy(event_set[test_cnt].name, FILE_NAME2);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME2);
 	test_cnt++;

 	/*
@@ -103,12 +103,12 @@ void verify_inotify(void)
 	 * we can correct determine kernel bug which exist before
 	 * 2.6.25. See comment below.
 	 */
-	snprintf(fname3, BUF_SIZE, "%s.rename2", fname1);
+	snprintf(fname3, sizeof(fname3), "%s.rename2", fname1);
 	SAFE_RENAME(fname2, fname3);

 	SAFE_RENAME(fname3, fname1);
 	event_set[test_cnt].mask = IN_MOVE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	int len, i = 0, test_num = 0;
diff --git a/testcases/kernel/syscalls/inotify/inotify04.c b/testcases/kernel/syscalls/inotify/inotify04.c
index 947623952..2d7f34bae 100644
--- a/testcases/kernel/syscalls/inotify/inotify04.c
+++ b/testcases/kernel/syscalls/inotify/inotify04.c
@@ -91,10 +91,10 @@ void verify_inotify(void)
 	reap_wd_dir = 0;

 	event_set[test_cnt].mask = IN_DELETE_SELF;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;
 	event_set[test_cnt].mask = IN_IGNORED;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	SAFE_UNLINK(TEST_FILE);
@@ -108,14 +108,14 @@ void verify_inotify(void)
 	 * understand how Unix works.
 	 */
 	event_set[test_cnt].mask = IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	event_set[test_cnt].mask = IN_DELETE_SELF;
-	strcpy(event_set[test_cnt].name, TEST_FILE);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", TEST_FILE);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_IGNORED;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
diff --git a/testcases/kernel/syscalls/inotify/inotify07.c b/testcases/kernel/syscalls/inotify/inotify07.c
index b4000f353..f0acd9e91 100644
--- a/testcases/kernel/syscalls/inotify/inotify07.c
+++ b/testcases/kernel/syscalls/inotify/inotify07.c
@@ -74,18 +74,18 @@ void verify_inotify(void)
 	 */
 	SAFE_CHMOD(DIR_PATH, 0755);
 	event_set[test_cnt].mask = IN_ISDIR | IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, "");
+	event_set[test_cnt].name[0] = '\0';
 	test_cnt++;

 	SAFE_TOUCH(FILE_PATH, 0644, NULL);
 	event_set[test_cnt].mask = IN_OPEN;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_CLOSE_WRITE;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;
 	event_set[test_cnt].mask = IN_ATTRIB;
-	strcpy(event_set[test_cnt].name, FILE_NAME);
+	snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name), "%s", FILE_NAME);
 	test_cnt++;

 	int len = SAFE_READ(0, fd_notify, event_buf, EVENT_BUF_LEN);
diff --git a/testcases/kernel/syscalls/inotify/inotify10.c b/testcases/kernel/syscalls/inotify/inotify10.c
index 4c3a1d116..0a94ead15 100644
--- a/testcases/kernel/syscalls/inotify/inotify10.c
+++ b/testcases/kernel/syscalls/inotify/inotify10.c
@@ -121,25 +121,27 @@ static void verify_inotify(unsigned int n)
 	if (wd_parent && (tc->parent_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_parent;
 		event_set[test_cnt].mask = tc->parent_mask | IN_ISDIR;
-		strcpy(event_set[test_cnt].name, TEST_DIR);
+		snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name),
+			 "%s", TEST_DIR);
 		test_cnt++;
 	}
 	if (wd_subdir && (tc->subdir_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_subdir;
 		event_set[test_cnt].mask = tc->subdir_mask | IN_ISDIR;
-		strcpy(event_set[test_cnt].name, "");
+		event_set[test_cnt].name[0] = '\0';
 		test_cnt++;
 	}
 	if (wd_parent && (tc->parent_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_parent;
 		event_set[test_cnt].mask = tc->parent_mask;
-		strcpy(event_set[test_cnt].name, TEST_FILE);
+		snprintf(event_set[test_cnt].name, sizeof(event_set[test_cnt].name),
+			 "%s", TEST_FILE);
 		test_cnt++;
 	}
 	if (wd_child && (tc->child_mask & IN_ATTRIB)) {
 		event_set[test_cnt].wd = wd_child;
 		event_set[test_cnt].mask = tc->child_mask;
-		strcpy(event_set[test_cnt].name, "");
+		event_set[test_cnt].name[0] = '\0';
 		test_cnt++;
 	}

--
2.43.0

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] inotify: replace strcpy() with snprintf()

[Andrea Cervesato via ltp]

Hi!

> 
>  #define BUF_SIZE 256
> -static char fname1[BUF_SIZE], fname2[BUF_SIZE], fname3[BUF_SIZE];
> +static char fname1[BUF_SIZE], fname2[PATH_MAX], fname3[PATH_MAX];

Still using BUF_SIZE.....

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3] inotify: replace strcpy() with snprintf()

[Jinseok Kim]

Hi,

Is replacing

  static char fname1[BUF_SIZE], fname2[BUF_SIZE], fname3[BUF_SIZE];

with

  static char fname1[PATH_MAX], fname2[PATH_MAX], fname3[PATH_MAX];

considered sufficient here?

The -Wformat-truncation warning still occurs:

  snprintf(fname2, sizeof(fname2), "%s.rename1", fname1);
  snprintf(fname3, sizeof(fname3), "%s.rename2", fname1);

Since fname1 can already be PATH_MAX bytes long, appending a suffix
like ".rename1" or ".rename2" may exceed PATH_MAX.

Is it acceptable to keep this warning as-is, or should this be handled
differently?

Thanks,
Jinseok.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] inotify: replace strcpy() with snprintf()

[Andrea Cervesato via ltp]

Hi,

> 
> The -Wformat-truncation warning still occurs:
> 
>   snprintf(fname2, sizeof(fname2), "%s.rename1", fname1);
>   snprintf(fname3, sizeof(fname3), "%s.rename2", fname1);

Indeed, compiler might complains in this case.

`snprintf()` can't overflow, but it can stop adding characters after `PATH_MAX`.
We need to verify the return value, but `PATH_MAX` can be still a valid return
value. I will cc @Cyril to ask for an opinion about it.

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp