[LTP] [PATCH 1/4] syscalls/chroot&creat: fix undefined behavior

[LTP] [PATCH 1/4] syscalls/chroot&creat: fix undefined behavior

[Wei,Jiangang]

The results are undefined if source and destination
buffers overlap when calling s[n]printf().
Such as,
    sprintf(buf, "%s some further text", buf);
The above will not produce the expected results.

This patch enssures that produce output as expected.

Signed-off-by: Wei,Jiangang <weijg.fnst@cn.fujitsu.com>
---
 testcases/kernel/syscalls/chroot/chroot03.c | 4 +++-
 testcases/kernel/syscalls/creat/creat04.c   | 9 +++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/chroot/chroot03.c b/testcases/kernel/syscalls/chroot/chroot03.c
index 9376892..69fead4 100644
--- a/testcases/kernel/syscalls/chroot/chroot03.c
+++ b/testcases/kernel/syscalls/chroot/chroot03.c
@@ -151,7 +151,9 @@ static void setup(void)
 	 * set up good_dir to test whether chroot() is setting ENOENT if the
 	 * directory does not exist.
 	 */
-	(void)sprintf(good_dir, "%s.%d", good_dir, getpid());
+	good_dir_dup = strdup(good_dir);
+	(void)sprintf(good_dir, "%s.%d", good_dir_dup, getpid());
+	free(good_dir_dup);
 
 #if !defined(UCLINUX)
 	bad_addr = mmap(0, 1, PROT_NONE,
diff --git a/testcases/kernel/syscalls/creat/creat04.c b/testcases/kernel/syscalls/creat/creat04.c
index 0268e77..44c249f 100644
--- a/testcases/kernel/syscalls/creat/creat04.c
+++ b/testcases/kernel/syscalls/creat/creat04.c
@@ -192,10 +192,11 @@ void setup(void)
 
 	/* make a temporary directory and cd to it */
 	tst_tmpdir();
-
-	sprintf(good_dir, "%s.%d", good_dir, getpid());
-	sprintf(fname1, "%s/file1.%d", good_dir, getpid());
-	sprintf(fname, "%s/file.%d", good_dir, getpid());
+	good_dir_dup = strdup(good_dir);
+	sprintf(good_dir, "%s.%d", good_dir_dup, getpid());
+	sprintf(fname1, "%s/file1.%d", good_dir_dup, getpid());
+	sprintf(fname, "%s/file.%d", good_dir_dup, getpid());
+	free(good_dir_dup);
 }
 
 /*
-- 
1.9.3


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 2/4] cpuset/cpuset_lib/cpuinfo.c: fix FILE pointer leak

[Wei,Jiangang]

Signed-off-by: Wei,Jiangang <weijg.fnst@cn.fujitsu.com>
---
 testcases/kernel/controllers/cpuset/cpuset_lib/cpuinfo.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/controllers/cpuset/cpuset_lib/cpuinfo.c b/testcases/kernel/controllers/cpuset/cpuset_lib/cpuinfo.c
index 54fa1e5..c2a60b6 100644
--- a/testcases/kernel/controllers/cpuset/cpuset_lib/cpuinfo.c
+++ b/testcases/kernel/controllers/cpuset/cpuset_lib/cpuinfo.c
@@ -231,11 +231,15 @@ static int get_sched_domains(void)
 			if (!cpus[ci].sched_domain) {
 				cpus[ci].sched_domain =
 				    bitmask_alloc(cpus_nbits);
-				if (!cpus[ci].sched_domain)
+				if (!cpus[ci].sched_domain) {
+					fclose(fp);
 					return -1;
+				}
 			}
-			if (bitmask_parsehex(str2, cpus[ci].sched_domain))
+			if (bitmask_parsehex(str2, cpus[ci].sched_domain)) {
+				fclose(fp);
 				return -1;
+			}
 		}
 	}
 
-- 
1.9.3


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 2/4] cpuset/cpuset_lib/cpuinfo.c: fix FILE pointer leak

[Cyril Hrubis]

Hi!
Pushed.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 3/4] pan/symbol.c: fix memory leak for failure case

[Wei,Jiangang]

Signed-off-by: Wei,Jiangang <weijg.fnst@cn.fujitsu.com>
---
 pan/symbol.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/pan/symbol.c b/pan/symbol.c
index d7b90af..ca9964a 100644
--- a/pan/symbol.c
+++ b/pan/symbol.c
@@ -112,6 +112,7 @@ static struct sym *mknode(struct sym *next, char *key, void *data)
 
 	if (n->key == NULL) {
 		sym_error = "sym node strdup(key) failed!";
+		free(n);
 		return (NULL);
 	}
 	return (n);
@@ -204,8 +205,10 @@ int sym_put(SYM sym, char *key, void *data, int flags)
 	nkey = strdup(key);
 	keys = splitstr(key, ",", NULL);
 
-	if (keys == NULL)
+	if (keys == NULL) {
+		free(nkey);
 		return (EINVAL);
+	}
 
 	for (kk = (char **)keys, csym = sym;
 	     *kk != NULL && (nsym = find_key1(csym->sym, *kk)) != NULL;
-- 
1.9.3


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 3/4] pan/symbol.c: fix memory leak for failure case

[Cyril Hrubis]

Hi!
Pushed as well, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 4/4] pan/splitstr.c: free memory allocated by strdup()

[Wei,Jiangang]

Signed-off-by: Wei,Jiangang <weijg.fnst@cn.fujitsu.com>
---
 pan/splitstr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pan/splitstr.c b/pan/splitstr.c
index fbcd697..decfbf5 100644
--- a/pan/splitstr.c
+++ b/pan/splitstr.c
@@ -122,6 +122,7 @@ const char **splitstr(const char *str, const char *separator, int *argcount)
 		}
 	}
 	arg_array[num_toks] = NULL;
+	free(arg_string);
 
 	/*
 	 * If there are any spaces left in our array, make them NULL
-- 
1.9.3


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 4/4] pan/splitstr.c: free memory allocated by strdup()

[Cyril Hrubis]

Hi!
>  pan/splitstr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/pan/splitstr.c b/pan/splitstr.c
> index fbcd697..decfbf5 100644
> --- a/pan/splitstr.c
> +++ b/pan/splitstr.c
> @@ -122,6 +122,7 @@ const char **splitstr(const char *str, const char *separator, int *argcount)
>  		}
>  	}
>  	arg_array[num_toks] = NULL;
> +	free(arg_string);

As far as I can see the arg_string is destructively parsed with strtok()
and parts of the string are saved into the array that gets returned
at the end of the function. So freeing it will cause crash.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/4] syscalls/chroot&creat: fix undefined behavior

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/syscalls/chroot/chroot03.c b/testcases/kernel/syscalls/chroot/chroot03.c
> index 9376892..69fead4 100644
> --- a/testcases/kernel/syscalls/chroot/chroot03.c
> +++ b/testcases/kernel/syscalls/chroot/chroot03.c
> @@ -151,7 +151,9 @@ static void setup(void)
>  	 * set up good_dir to test whether chroot() is setting ENOENT if the
>  	 * directory does not exist.
>  	 */
> -	(void)sprintf(good_dir, "%s.%d", good_dir, getpid());
> +	good_dir_dup = strdup(good_dir);
> +	(void)sprintf(good_dir, "%s.%d", good_dir_dup, getpid());
> +	free(good_dir_dup);

Rather than that we can simply use "nonexistent" as the directory name.

Because the test creates unique test temporary directory at the start
and changes working directory to it. Then any local path points to a
nonexistent directory.

Also naming the variable good_dir is a bad idea. It should be named
nonexistent_dir or so.

>  #if !defined(UCLINUX)
>  	bad_addr = mmap(0, 1, PROT_NONE,
> diff --git a/testcases/kernel/syscalls/creat/creat04.c b/testcases/kernel/syscalls/creat/creat04.c
> index 0268e77..44c249f 100644
> --- a/testcases/kernel/syscalls/creat/creat04.c
> +++ b/testcases/kernel/syscalls/creat/creat04.c
> @@ -192,10 +192,11 @@ void setup(void)
>  
>  	/* make a temporary directory and cd to it */
>  	tst_tmpdir();
> -
> -	sprintf(good_dir, "%s.%d", good_dir, getpid());
> -	sprintf(fname1, "%s/file1.%d", good_dir, getpid());
> -	sprintf(fname, "%s/file.%d", good_dir, getpid());
> +	good_dir_dup = strdup(good_dir);
> +	sprintf(good_dir, "%s.%d", good_dir_dup, getpid());
> +	sprintf(fname1, "%s/file1.%d", good_dir_dup, getpid());
> +	sprintf(fname, "%s/file.%d", good_dir_dup, getpid());
> +	free(good_dir_dup);

There is no need to append pid to the directory and file name, the test
temporary directory create by tst_tmpdir() is unique allready.

So rather than doing ugly workarounds like that, what about removing the
sprintf() and usign static directory and file names?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH v2] syscalls/chroot&creat: fix undefined behavior

[Wei,Jiangang]

The results are undefined if source and destination
buffers overlap when calling s[n]printf().
Such as,
    sprintf(buf, "%s some further text", buf);
The above will not produce the expected results.

This patch enssures that produce output as expected.

Signed-off-by: Wei,Jiangang <weijg.fnst@cn.fujitsu.com>
---
 testcases/kernel/syscalls/chroot/chroot03.c | 14 ++++++--------
 testcases/kernel/syscalls/creat/creat04.c   | 13 +++++--------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/testcases/kernel/syscalls/chroot/chroot03.c b/testcases/kernel/syscalls/chroot/chroot03.c
index 9376892..3d51056 100644
--- a/testcases/kernel/syscalls/chroot/chroot03.c
+++ b/testcases/kernel/syscalls/chroot/chroot03.c
@@ -53,7 +53,11 @@ char *TCID = "chroot03";
 
 static int fd;
 static char fname[255];
-static char good_dir[100] = "/tmp/testdir";
+/*
+ *  set up nonexistent_dir to test whether chroot() is setting ENOENT
+ *  if the directory does not exist.
+ */
+static char nonexistent_dir[100] = "testdir";
 static char bad_dir[] = "abcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
 static char symbolic_dir[] = "sym_dir1";
 
@@ -78,7 +82,7 @@ struct test_case_t {
 	     * does not exist.
 	     */
 	{
-	good_dir, ENOENT},
+	nonexistent_dir, ENOENT},
 #if !defined(UCLINUX)
 	    /*
 	     * attempt to chroot to a path pointing to an invalid address
@@ -147,12 +151,6 @@ static void setup(void)
 	if (fd == -1)
 		tst_brkm(TBROK, cleanup, "Failed to creat a temp file");
 
-	/*
-	 * set up good_dir to test whether chroot() is setting ENOENT if the
-	 * directory does not exist.
-	 */
-	(void)sprintf(good_dir, "%s.%d", good_dir, getpid());
-
 #if !defined(UCLINUX)
 	bad_addr = mmap(0, 1, PROT_NONE,
 			MAP_PRIVATE_EXCEPT_UCLINUX | MAP_ANONYMOUS, 0, 0);
diff --git a/testcases/kernel/syscalls/creat/creat04.c b/testcases/kernel/syscalls/creat/creat04.c
index 0268e77..aa4498a 100644
--- a/testcases/kernel/syscalls/creat/creat04.c
+++ b/testcases/kernel/syscalls/creat/creat04.c
@@ -68,8 +68,9 @@ void cleanup(void);
 #define FMODE	0444
 #define DMODE	00700
 
-char good_dir[40] = "testdir";
-char fname[40], fname1[40];
+static char fname_dir[] = "testdir";
+static char fname[] = "testdir/file";
+static char fname1[] = "testdir/file1";
 
 static uid_t nobody_uid;
 
@@ -103,7 +104,7 @@ int main(int ac, char **av)
 		}
 
 		if (pid == 0) {	/* first child */
-			if (mkdir(good_dir, DMODE) != 0) {
+			if (mkdir(fname_dir, DMODE) != 0) {
 				perror("mkdir() failed");
 				exit(1);
 			}
@@ -157,7 +158,7 @@ int main(int ac, char **av)
 			/* clean up things in case we are looping */
 			unlink(fname);
 			unlink(fname1);
-			rmdir(good_dir);
+			rmdir(fname_dir);
 			exit(retval);
 
 		} else {	/* parent */
@@ -192,10 +193,6 @@ void setup(void)
 
 	/* make a temporary directory and cd to it */
 	tst_tmpdir();
-
-	sprintf(good_dir, "%s.%d", good_dir, getpid());
-	sprintf(fname1, "%s/file1.%d", good_dir, getpid());
-	sprintf(fname, "%s/file.%d", good_dir, getpid());
 }
 
 /*
-- 
1.9.3


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] syscalls/chroot&creat: fix undefined behavior

[Cyril Hrubis]

Hi!
>  static int fd;
>  static char fname[255];
> -static char good_dir[100] = "/tmp/testdir";
> +/*
> + *  set up nonexistent_dir to test whether chroot() is setting ENOENT
> + *  if the directory does not exist.
> + */

I've removed this comment and pushed the patch. IMHO the directory name
is clear enough now.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list