SnP SnP - 28 days ago 13
C Question

Valgrind - snprintf : Conditional jump or move depends on uninitialised value(s)

After launching a program thru valgrind I got the following message :

==9290== Conditional jump or move depends on uninitialised value(s)
==9290== at 0x4E82A03: vfprintf (vfprintf.c:1661)
==9290== by 0x4EA9578: vsnprintf (vsnprintf.c:119)
==9290== by 0x4E8B531: snprintf (snprintf.c:33)
==9290== by 0x400820: _function (in /home/snp/prog/TEST)
==9290== by 0x4006D5: start (in /home/snp/prog/TEST)
==9290== by 0x40085C: main (in /home/snp/prog/TEST)
==9290== Uninitialised value was created by a heap allocation
==9290== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9290== by 0x400715: init (in /home/snp/prog/TEST)
==9290== by 0x400857: main (in /home/snp/prog/TEST)


The following code reproduce the error :

#include <net/if.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <syslog.h>

#define TARGET "8.8.8.8"
#define DEVICE "eth0"

static int _function(void);

struct remote
{
char *target;
char device[IFNAMSIZ];
};

struct remote * st_args;

int start(void)
{
return (_function());
}

int init(void)
{
st_args = malloc (sizeof (struct remote));
if (st_args == NULL)
return (-1);

st_args->target = malloc (sizeof (TARGET)+1);
if (st_args->target == NULL)
{
free (st_args);
return (-1);
}

strncpy(st_args->target, TARGET , sizeof(TARGET)-1);
strncpy(st_args->device, DEVICE, IFNAMSIZ-1);

return 0;
}

void stop(void)
{
if (st_args != NULL)
{
free (st_args->target);
free (st_args);
}
}

static int _function(void)
{
char cmd[256];

memset(cmd, 0, sizeof(cmd));

snprintf(cmd, sizeof(cmd), "ping -I %s %s", st_args->device, st_args->target);

return 0;
}

int main(int argc, char **argv)
{
init();
start();
stop();
return 0;
}


I still does not understand the issue, why valgrind does not accept the
snprintf
command. In addition, the array contains the expected string after the line execution.

Answer

Valgrind's message,

==9290== Conditional jump or move depends on uninitialised value(s)

is reasonably self-explanatory: the program is observed to be relying on on uninitialized memory to make a decision. Happening in a standard library function as it does as it does, it is natural to suppose that there is something wrong with the function arguments. Since you're specifically printing strings, the most likely cause is that one of the string arguments is unterminated.

And indeed, at least one is. Consider this code:

#define TARGET "8.8.8.8"

[...]

strncpy(st_args->target, TARGET , sizeof(TARGET)-1);

In trying to be safe, you have shot yourself in the foot. strncpy() copies at most the specified number of bytes, but it does not append a terminator afterwards. Thus, its Linux manual page contains this warning:

Warning: If there is no null byte among the first n bytes of src, the string placed in dest will not be null terminated.

You have ensured that the situation described in that warning takes place -- no null terminator is written, and the last byte allocated for st_args->target remains uninitialized.

Since you are careful to allocate enough space for the full string to begin with, including the terminator, the strncpy() is overkill anyway. Just use strcpy(). Or indeed, if your system has strdup() or you're willing to write an implementation, then strdup() is much cleaner than malloc() + strcpy().

Alternatively, if you want to use strncpy() then it's a good idea to ensure that the destination string is terminated by following up each strncpy() call by manually writing a terminator to the last byte of the destination. In this case, that would be

st_args->target[sizeof(TARGET)] = '\0';

Note also that you actually allocate one more byte than you need, for the sizeof a string literal includes the terminator. The code just above is written for that actual one-byte-too-many allocation.

Comments