Analyze valgrind output: "invalid free()"
I have this strange error found by valgrind on a (stupid) authentication module which makes some on heap allocations.
==8009== Invalid free() / delete / delete[] / realloc()
==8009== at 0x4C2A739: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8009== by 0x40263F: authenticate (server_utils.c:109)
==8009== by 0x401A27: main (server.c:240)
==8009== Address 0x51f1310 is 0 bytes inside a block of size 18 free'd
==8009== at 0x4C2A739: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8009== by 0x402633: authenticate (server_utils.c:108)
==8009== by 0x401A27: main (server.c:240)
=8009==
==8009== Invalid free() / delete / delete[] / realloc()
==8009== at 0x4C2A739: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8009== by 0x40264B: authenticate (server_utils.c:110)
==8009== by 0x401A27: main (server.c:240)
==8009== Address 0x51f1319 is 9 bytes inside a block of size 18 free'd
==8009== at 0x4C2A739: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8009== by 0x402633: authenticate (server_utils.c:108)
==8009== by 0x401A27: main (server.c:240)
==8009==
==8009== Invalid free() / delete / delete[] / realloc()
==8009== at 0x4C2A739: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8009== by 0x402657: authenticate (server_utils.c:111)
==8009== by 0x401A27: main (server.c:240)
==8009== Address 0x51f131e is 14 bytes inside a block of size 18 free'd
==8009== at 0x4C2A739: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8009== by 0x402633: authenticate (server_utils.c:108)
==8009== by 0x401A27: main (server.c:240)
==8009== ...
From what I understand he says I do three invalid free() on lines 109-110-111. The error should be that I try to free up more space than is actually allocated, but I can not decide how much space deallocate. Also I do not understand why then it refers to line 108 (which is also a free()).
This is from the documentation (Invalid free):
Memcheck keeps track of the blocks allocated by your program with malloc/new, so it can know exactly whether or not the argument to free/delete is legitimate or not. Here, this test program has freed the same block twice. As with the illegal read/write errors, Memcheck attempts to make sense of the address freed. If, as here, the address is one which has previously been freed, you wil be told that -- making duplicate frees of the same block easy to spot.
You will also get this message if you try to free a pointer that doesn't point to the start of a heap block.
I can not really imagine how be in one of these cases.
/* Authentication through <user_id:password:flag>
* Returns 1 on success, -1 if user_id doesn't exist, -2 on password mismatch
* On success sets access_permissions
*/
int authenticate(USR_PSW *received, int *access_permissions) {
/*Users file opening*/
FILE *fd;
fd = fopen(USERS_FILE, "r");
if (fd == NULL) {
fprintf(stderr, "Users file opening error\n");
exit(EXIT_FAILURE);
}
char *usr_psw_line = malloc(USR_SIZE + PSW_SIZE + 3 + 1);
if (usr_psw_line == NULL) {
fprintf(stderr, "Dynamic alloc error\n");
exit(EXIT_FAILURE);
}
char *usr_tok = malloc(USR_SIZE);
if (usr_tok == NULL) {
free(usr_psw_line);
fprintf(stderr, "Dynamic alloc error\n");
exit(EXIT_FAILURE);
}
char *psw_tok = malloc(PSW_SIZE);
if (psw_tok == NULL) {
free(usr_psw_line);
free(usr_tok);
fprintf(stderr, "Dynamic alloc error\n");
exit(EXIT_FAILURE);
}
char *flg_tok = malloc(sizeof (char) *2);
if (flg_tok == NULL) {
free(usr_psw_line);
free(usr_tok);
free(psw_tok);
fprintf(stderr, "Dynamic alloc error\n");
exit(EXIT_FAILURE);
}
/*Reading from file <user_id:password:flag> */
while (fgets(usr_psw_line, USR_SIZE - 1 + PSW_SIZE - 1 + 3 + 1, fd) != NULL) {
usr_tok = strtok(usr_psw_line, ":");
if (strcmp(usr_tok, received->user_id) == 0) {
/*user_id found, password check*/
psw_tok = strtok(NULL, ":");
/*password match*/
if (strcmp(psw_tok, received->password) == 0) {
flg_tok = strtok(NULL, ":");
*access_permissions = atoi(flg_tok);
free(usr_psw_line); //108
free(usr_tok);//109
free(psw_tok);//110
free(flg_tok);//111
fclose(fd);
return AUTHENTICATED;
} else { //password unmatch
free(usr_psw_line);
free(usr_tok);
free(psw_tok);
free(flg_tok);
fclose(fd);
return INVALID_PSW;
}
} else {
fseek(fd, 1, SEEK_CUR);
continue;
}
}
/*EOF Reached without match*/
free(usr_psw_line);
free(usr_tok);
free(psw_tok);
free(flg_tok);
fclose(fd);
return INVALID_USR;
}
The header file:
#ifndef __SERVER_UTILS_H__
#define __SERVER_UTILS_H__
#define USR_SIZE 9
#define PSW_SIZE 5
/*Authentication data structure definition*/
typedef struct{
char user_id[USR_SIZE]; // eg: user_123
char password[PSW_SIZE]; // eg: a1b2
} USR_PSW;
#define NM_MAX_SIZE 17
#define NR_MAX_SIZE 11
/*System record structure*/
typedef struct{
char first_name[NM_MAX_SIZE];
char last_name[NM_MAX_SIZE];
char number[NR_MAX_SIZE];
}TBOOK_RECORD;
/*Session status flags*/
#define NOT_AUTHENTICATED 0
#define AUTHENTICATED 1
#define INVALID_USR 2
#define INVALID_PSW 3
/*Persmissions flags*/
#define NO_PERM 0
#define READ_WRITE 1
#define O_WRITE 2
#define O_READ 3
/*Contains <user_id:password,flag> triplets stored in server */
#define USERS_FILE "users.txt"
/*Contains all telbook records <first_name:last_name:phone_number>*/
#define RECORDS_FILE "records.txt"
/*Single records file line max length (before \n) */
#define RECFILE_LINE_MAX_LEN 2*NM_MAX_SIZE+NR_MAX_SIZE+3+1;
/*Client operation*/
#define NO_OP 0
#define SEARCH_BY_NAME 1
#define SEARCH_BY_NUMBER 2
#define INSERT_RECORD 3
/*
* Returns number of bytes copied into buffer (excluding terminating null byte),
* or 0 on EOF, or -1 on error.
* size_t : used for sizes of objects.
* ssize_t: used for a count of bytes or an error indication (-1).
*/
ssize_t readline(int fd, void *buffer, size_t n);
/* Authentication through <user_id:password:flag>
* Returns 1 on success, -1 if user_id doesn't exist, -2 on password mismatch
* On success sets access_permissions
*/
int authenticate(USR_PSW *received, int *access_permissions);
int close_session(int sock_ds);
int search_by__(int op_code, TBOOK_RECORD *rc_rcvd, TBOOK_RECORD *rc_rspn
, FILE *fd, char *file_line);
int insert_record(TBOOK_RECORD *rc_rcvd);
#endif /* __SERVER_UTILS_H__ */
Answer
usr_tok = strtok(usr_psw_line, ":");
...
psw_tok = strtok(NULL, ":");
You are overwriting your pointer to your malloced memory (thereby leaking it) with a pointer elsewhere. Then you are trying to free that pointer you received from elsewhere, which is (as valgrind says) invalid.