[LU-10502] Address of a local variable is returned through global variable 'progname'.

• Status: Open
• Project: Lustre
• Affects Version/s: Lustre 2.11.0
• Type: Bug
• Priority: Minor
• Reporter: Dmitry Eremin (Inactive)
• Assignee: Jian Yu
• Resolution: Unresolved
• Labels: kw, patch
• Severity: 3

Description

6559	static int lfs_mirror(int argc, char **argv)
6560	{
6561		char cmd[PATH_MAX];
6562		int rc = 0;
6563	 
6564		setlinebuf(stdout);
6565	 
6566		Parser_init("lfs-mirror > ", mirror_cmdlist);
6567	 
6568		snprintf(cmd, sizeof(cmd), "%s %s", progname, argv[0]);
6569		progname = cmd;
6570		program_invocation_short_name = cmd;
6571		if (argc > 1)
6572			rc = Parser_execarg(argc - 1, argv + 1, mirror_cmdlist);
6573		else
6574			rc = Parser_commands();
6575	 
6576		return rc < 0 ? -rc : rc;
6577	}

TRACEBACK

An event which alters the program's state, leading to the defect
lfs.c:6569: Local address 'cmd' is stored into 'progname'

An event which alters the program's state, leading to the defect
lfs.c:6576: 'progname' can be used outside

Address of a local variable is returned through global variable 'program_invocation_short_name'.
TRACEBACK

An event which alters the program's state, leading to the defect
lfs.c:6570: Local address 'cmd' is stored into 'program_invocation_short_name'

An event which alters the program's state, leading to the defect
lfs.c:6576: 'program_invocation_short_name' can be used outside

Comments

[Jian Yu]

For "lfs mirror" commands, the progname needs to be replaced with "lfs mirror" instead of "lfs", so progname was reassigned with cmd.

[Dmitry Eremin (Inactive)]

This issue here is a string is generated into local buffer! So, this local variable will be destroyed after exit from this function. I see a lot of places where progname is used. So, unexpectedly we can use garbage from stack instead of progname.

[Jinshan Xiong (Inactive)]

The current implementation won't have any issue per-se because as long as the function is returned progname won't be used any more. However, I still think we should fix this issue otherwise it will throw a warning everything the static analysis tool is executed.

The fix can be as simple as redefining progname as:

static char progname[PATH_MAX];

[Jian Yu]

I tried to redefine progname in lfs.c but hit the following errors:

lfs.c:77:13: error: conflicting types for ‘progname’
 static char progname[PATH_MAX];
             ^
In file included from lfs.c:64:0:
lfs_project.h:39:13: note: previous declaration of ‘progname’ was here
 const char *progname;
             ^

[Jinshan Xiong (Inactive)]

the progname has been moved to lfs_project.c by patch LU-10030(the change seems to be wrong). We should restore the change to progname and let lfs_project.c and lfs.c declare their own progname.

[Gerrit Updater]

Jian Yu ([e-mail]) uploaded a new patch: https://review.whamcloud.com/30928
Subject: LU-10502 flr: redefine progname in lfs.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 91b1a3d24e46ac7cecbf66c08f92bb4a4a6b1500