tag 645839 + patch
thanks
I wrote :
> apt-spy: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char
> *)
> &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk,
> fd))))
> && old_size == 0) || ((unsigned long) (old_size) >= (unsigned
> long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 *
> (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size
> &
> 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed.
> Aborted
I worked on it and found several problems, in the files main.c parse.c and
file.c,
mainly with memory allocation, that I tried to solve.
Finally, I think that I succeed. The attached patch should bypass a lot of
segfaults.
It should close this bug, bug #491802, and maybe other segmentation fault bugs.
(#447232? #427561?)
Any one to test it ?
--
Laurent Dard
--- apt-spy-3.1.orig/main.c
+++ apt-spy-3.1/main.c
@@ -49,7 +49,7 @@
int main(int argc, char *argv[])
{
int c;
- char *cur_entry; /* Entry we are benchmarking */
+ char *cur_entry = NULL; /* Entry we are benchmarking */
char *distrib = NULL; /* distribution to use. */
char *mirror_list = NULL; /* mirror list file */
@@ -63,35 +63,38 @@
char *update_url = NULL; /* URL to use for updating */
char *country_list = NULL; /* List of countries to b/m */
char *section_list = NULL; /* List of section to include */
- FILE *infile_p, *outfile_p; /* input/output file pointers */
- FILE *config_p; /* config file pointer */
- FILE *mirror_p; /* mirror list pointer */
- FILE *topfile_p;
+ FILE *infile_p = NULL; /* input file pointer */
+ FILE *outfile_p = NULL; /* output file pointer */
+ FILE *config_p = NULL; /* config file pointer */
+ FILE *mirror_p = NULL; /* mirror list pointer */
+ FILE *topfile_p = NULL;
int timeout = 15; /* time to benchmark each server */
- int toplist = 0; /* Whether to write toplist. */
int argsCount = 0; /* persistent counter for args */
char *args[100]; /* persistent args array */
- char str[100] = "";
- int i;
+ char str[4] = "-? "; /* short option string */
/* Number of servers to test. If negative, test them all. */
int test_number = -1;
/* Server information structures. */
- server_t current, *best;
+ server_t current;
+ server_t *best = NULL;
+
+ /* Initialization of the persistent args array */
+ memset(args,0,100);
/* Parse options... */
while((c = getopt(argc, argv, "y:a:c:d:e:f:i:m:o:p:s:t:u:w:n:vh")) != -1)
{
- for(i = 0; i < 100; i++)
- str[i] = '\0';
- strcpy(str,"");
- str[0] = '-'; str[1] = c; str[2] = ' ';
- strcat(str, optarg);
- args[argsCount] = (char *)malloc(strlen(str));
- strcpy(args[argsCount],str);
- argsCount++;
+ if (optarg) { /* not set with -h */
+ str[1] = c;
+ args[argsCount] = malloc(strlen(str)+strlen(optarg)+1);
+ memset(args[argsCount],0,strlen(str)+strlen(optarg)+1);
+ strncpy(args[argsCount],str,strlen(str));
+ strncat(args[argsCount],optarg,strlen(optarg));
+ argsCount++;
+ }
switch(c) {
- char *end;
+ char *end = NULL;
/* Sections to include into apt-source */
case 'y':
section_list = optarg;
@@ -158,7 +161,6 @@
break;
/* Should we write a list of the "top" servers? */
case 'w':
- toplist = 1;
topfile = optarg;
break;
/* Number of servers to write in "top" server list */
@@ -178,6 +180,7 @@
case 'h':
default:
usage(); /* display help */
+ break;
}
}
argc -= optind;
@@ -194,6 +197,9 @@
exit(1);
}
+ /* Zero the "best" structure */
+ memset(best, 0, sizeof(server_t) * (BESTNUMBER + 1));
+
/* We require an area and distribution argument if we are not updating */
if ((argc == 0) && (distrib == NULL))
usage();
@@ -240,7 +246,6 @@
/* argc should be 0. If not, there's something wrong. */
if (argc != 0)
usage();
-
/* We open the infile. Either a temporary file, or a user-specified
one. */
@@ -297,15 +302,10 @@
}
}
}
-
/* Make sure we're at the beginning... */
rewind(infile_p);
- /* Zero the "best" structure */
- for (c = 0; c < BESTNUMBER; c++)
- memset(&best[c], 0, sizeof(server_t));
-
/* This is the main loop. It'll exit when we've exhausted the URL
list or test_number is 0 */
--- apt-spy-3.1.orig/parse.c
+++ apt-spy-3.1/parse.c
@@ -23,10 +23,10 @@
int build_area_file(FILE *config_p, FILE *infile_p, FILE *mirror_list, char *area)
{
- char *line; /* Where we read the lines into */
- char *tmp; /* Temp. pointer */
- char *inputline; /* The line that will be written to config_p */
- char *country_code; /* where we put the country code */
+ char *line = NULL; /* Where we read the lines into */
+ char *tmp = NULL; /* Temp. pointer */
+ char *inputline = NULL; /* The line that will be written to config_p */
+ char *country_code = NULL; /* where we put the country code */
/* Uppercase area */
str_toupper(area);
@@ -77,13 +77,26 @@
/* If country_code points to a '\n', there were no other characters. It was a blank line. */
/* If it points to a '#', there is a comment. We skip it too. */
- if ((*country_code == '\n') || (*country_code == '#'))
+ if ((*country_code == '\n') || (*country_code == '#')) {
+ free(line);
continue;
+ }
- if ((strchr(line, ':')) != NULL)
+ if ((strchr(line, ':')) != NULL) {
+ free(line);
return 0; /* End of list. Return. */
+ }
/* We do a little fiddling to get the country code down to 2 letters and a space */
+ /* but before we must add 1 or 2 bytes to the allocated memory if it's too small.
+ The line can be only one char: users may do silly things sometimes... */
+ if (strlen(country_code) < 3) {
+ line = realloc(line,strlen(line)+4-strlen(country_code));
+ if (line == NULL) {
+ perror("realloc");
+ exit(1);
+ }
+ }
*(country_code + 2) = ' ';
*(country_code + 3) = '\0';
@@ -97,6 +110,8 @@
continue;
}
+ free(line);
+
/* The next read of infile_p will return the first mirror entry. We parse */
/* this and build a line to put into the temporary file. */
@@ -106,13 +121,9 @@
fputs(inputline, infile_p);
free(inputline);
- if ((ferror(infile_p)) != 0) { /* Check for file error */
- free(line);
+ if ((ferror(infile_p)) != 0) /* Check for file error */
return 1;
- }
}
-
- free(line);
}
return 0;
}
@@ -186,7 +197,9 @@
int find_country(FILE *mirror_list, char *country_code)
{
- char *line, *cc;
+ char *line =NULL;
+ char *cc =NULL;
+ char *tmp =NULL; /* Temp. pointer */
/* Make sure we're at beginning of file */
rewind(mirror_list);
@@ -199,27 +212,29 @@
while ((line = next_entry(mirror_list)) != NULL) {
cc = strstr(line, country_code);
- if (cc == NULL)
+ if (cc == NULL) {
+ free(line);
continue;
-
+ }
+
/* Skip white space */
- while (isspace(*line))
- ++line;
-
+ tmp = line;
+ while (isspace(*tmp))
+ ++tmp;
/* Country code should be first two characters on line. */
- if (cc == line)
+ if (cc == tmp)
break;
}
if (line == NULL)
return 1;
+ free(line);
- next_entry(mirror_list); /* Skip a line */
-
- if (ferror(mirror_list)) {
- free(line);
+ line = next_entry(mirror_list); /* Skip a line */
+ if (!line) {
return 1;
}
+ free(line);
return 0; /* We're positioned nicely for the next read */
}
@@ -235,6 +250,11 @@
/* First, we read in a line from the file */
save_line = line = next_entry(mirror_list);
+ if (line == NULL) {
+ perror("malloc");
+ exit(1);
+ }
+
/* Allocate space for creation */
len=5+strlen(line);
save_creation = creation = malloc(len);
@@ -247,12 +267,14 @@
/* test for file error */
if (ferror(mirror_list)) {
perror("fopen");
+ free(line);
free(save_creation);
return NULL;
}
/* If the line begins with a space, we assume it is empty and the list is exhausted. */
if (isspace(*line) != 0) {
+ free(line);
free(save_creation);
return NULL;
}
@@ -301,7 +323,6 @@
void tokenise(server_t *current, char *cur_entry)
{
char *temp; /* We use this for temporary string-pointing :P */
- static char null_string[]="";
/* We initialise the structure to 0 */
memset(current, 0, sizeof(*current));
@@ -341,7 +362,7 @@
perror("malloc");
exit(1);
}
- } else current->path[FTP]=null_string;
+ } else current->path[FTP]=strdup("");
/* And now check for HTTP entry */
if (*(++cur_entry) != ':') {
@@ -360,7 +381,7 @@
perror("realloc");
exit(1);
}
- } else current->path[HTTP]=null_string;
+ } else current->path[HTTP]=strdup("");
/* We're done for now */
}
@@ -426,7 +447,7 @@
int write_top(FILE *infile_p, FILE *outfile_p, server_t *best)
{
int i = 0;
- char *line;
+ char *line = NULL;
while (i < BESTNUMBER) {
@@ -438,8 +459,10 @@
if (best[i].hostname != NULL &&
strstr(line, best[i].hostname) != NULL) { /* Check for hostname */
fputs(line, outfile_p); /* if it's there, write to file */
+ free(line);
break;
}
+ free(line);
}
if ((ferror(infile_p) != 0) || (ferror(outfile_p) != 0))
--- apt-spy-3.1.orig/file.c
+++ apt-spy-3.1/file.c
@@ -77,12 +77,17 @@
char *next_entry(FILE *infile_p)
{
- char *temp;
- unsigned long orig_position, buffsize;
+ char *temp = NULL;
+ long int orig_position = 0;
+ long int buffsize = 0;
int c;
/* Save original file position */
orig_position = ftell(infile_p);
+ if (orig_position < 0) {
+ perror("Input error (ftell)");
+ exit(1);
+ }
/* Fast-forward to new line */
while ((c = getc(infile_p)) != '\n') {
@@ -92,7 +97,13 @@
}
}
- buffsize = ftell(infile_p) - orig_position; /* Calculate buffer size */
+ /* Calculate buffer size */
+ buffsize = ftell(infile_p);
+ if (buffsize < 0) {
+ perror("Input error (ftell)");
+ exit(1);
+ }
+ buffsize -= orig_position;
/* create storage space for line */
temp = malloc(buffsize + 1);
@@ -103,10 +114,16 @@
}
/* Rewind file to original position */
- fseek(infile_p, orig_position, SEEK_SET);
+ if (fseek(infile_p, orig_position, SEEK_SET) != 0) {
+ perror("Input error (fseek)");
+ exit(1);
+ }
/* We now read the line into the newly created buffer */
- fgets(temp, buffsize + 1, infile_p);
+ if (!fgets(temp, buffsize + 1, infile_p)) {
+ perror("Input error (fseek)");
+ exit(1);
+ }
return temp;
}
