[CRIU] Re: [PATCH zdtm] zdtm: new testcase for unhashed proc entries

[Andrey Vagin]

Thanks for this test case, a few minor comments are below

On Tue, Sep 11, 2012 at 04:36:29PM +0400, Konstantin Khlebnikov wrote:
> Testcase: fork child, chdir into /proc/$pid and kill child.
> 
> test for http://bugzilla.openvz.org/show_bug.cgi?id=2315
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov at openvz.org>
> ---
>  test/zdtm/live/static/Makefile        |    1 
>  test/zdtm/live/static/unhashed_proc.c |   78 +++++++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+)
>  create mode 100644 test/zdtm/live/static/unhashed_proc.c
> 
> diff --git a/test/zdtm/live/static/Makefile b/test/zdtm/live/static/Makefile
> index bffab9e..7f7ed0e 100644
> --- a/test/zdtm/live/static/Makefile
> +++ b/test/zdtm/live/static/Makefile
> @@ -57,6 +57,7 @@ TST_NOFILE	=				\
>  		netns				\
>  		session01			\
>  		socket-ext			\
> +		unhashed_proc			\
>  #		jobctl00			\
>  
>  TST_FILE	=				\
> diff --git a/test/zdtm/live/static/unhashed_proc.c b/test/zdtm/live/static/unhashed_proc.c
> new file mode 100644
> index 0000000..b407b1e
> --- /dev/null
> +++ b/test/zdtm/live/static/unhashed_proc.c
> @@ -0,0 +1,78 @@
> +#include <errno.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <signal.h>
> +#include <string.h>
> +
> +#include "zdtmtst.h"
> +
> +const char *test_doc	= "Chdir into unhashed proc entry";
> +const char *test_author	= "Konstantin Khlebnikov <khlebnikov at openvz.org>";
> +
> +int main(int argc, char ** argv)
> +{
> +	int pid, len;
> +	char cwd1[4096], cwd2[4096];
PATH_MAX
> +
> +	test_init(argc, argv);
> +
> +	pid = fork();
> +	if (pid < 0) {
> +		err("Fork failed %m\n");
> +		exit(1);
> +	} else if (!pid) {
> +		pause();
> +		return 0;
> +	}
> +
> +	sprintf(cwd1, "/proc/%d", pid);
> +
> +	if (chdir(cwd1) < 0) {
> +		kill(pid, SIGKILL);
> +		err("Chdir failed %m\n");
> +		exit(1);
> +	}
> +
> +	kill(pid, SIGKILL);
> +	waitpid(pid, NULL, 0);
> +
> +	if (getcwd(cwd1, sizeof(cwd1))) {
> +		fail("successfull getcwd: %s\n", cwd1);
Use err() instead of fail(), because it's a preparation action.
> +		exit(1);
> +	} else if (errno != ENOENT) {
> +		fail("wrong errno: %m\n");
> +		exit(1);
> +	}
> +
> +	len = readlink("/proc/self/cwd", cwd1, sizeof(cwd1));
> +	if (len < 0) {
> +		fail("can't read cwd symlink %m\n");
> +		exit(1);
> +	}
> +	cwd1[len] = 0;
> +
> +	test_daemon();
> +	test_waitsig();
> +
> +	if (getcwd(cwd2, sizeof(cwd2))) {
> +		fail("successfull getcwd: %s\n", cwd2);
> +	} else if (errno != ENOENT) {
> +		fail("wrong errno: %m\n");
> +	}
> +
> +	len = readlink("/proc/self/cwd", cwd2, sizeof(cwd2));
> +	if (len < 0) {
> +		fail("can't read cwd symlink %m\n");
> +		exit(1);
> +	}
> +	cwd2[len] = 0;
here is buffer overflow, len can be sizeof(cwd2)
> +
> +	if (strcmp(cwd1, cwd2))
> +		test_msg("cwd differs: %s != %s\n", cwd1, cwd2);
> +
> +	pass();
> +
> +	return 0;
> +}
>