[rsync@b] Re: rsync 2.5.6 globbing bug

Alan Burlison Alan.Burlison at sun.com
Tue Aug 5 00:57:31 EST 2003


Bert wrote:

> While I don't know about the ins-and-outs of globbing libraries (and, 
> therefore, can't judge ease, etc)...
> 
> The whole "it's rare [and an obscure (imo) work around exists] so let's 
> not worry about it" is a reason to reject a feature, NOT a bug fix.
> 
> As Alan already has done much and is prepared to do more of the work, I 
> hope his patch is accepted.

Patch attached.  It's pretty trivial - consisting mainly of allocating argv 
on the heap instead of allocating it on the stack, and growing it as 
appropriate.  All the necessary checks for detecting when it argv was about 
to overflow were already in the code, so all was that was necessary was to 
realloc argv & remove the error return.  I free up argc at the end, but I 
notice that the contents of argv, which are allocated with strdup, aren't 
freed in the existing implementation.  I guess this is because the child 
exits soon afterwards.  In this case the calls to free(argv) could be 
removed, but won't do any harm if they are left in.

For testing purposes I reduced the size of MAX_ARGS to 10 and rsynced just 
over 4000 dirs using the modified executable as the server (the whole of 
CPAN), directory at a time.  The largest directory contained 1645 entries. 
There were no problems.

-- 
Alan Burlison
--
-------------- next part --------------
*** proto.h.orig	Sun Aug  3 16:20:03 2003
--- proto.h	Sun Aug  3 23:34:36 2003
***************
*** 252,258 ****
  int name_to_uid(char *name, uid_t *uid);
  int name_to_gid(char *name, gid_t *gid);
  int lock_range(int fd, int offset, int len);
! void glob_expand(char *base1, char **argv, int *argc, int maxargs);
  void strlower(char *s);
  void *Realloc(void *p, int size);
  void clean_fname(char *name);
--- 252,258 ----
  int name_to_uid(char *name, uid_t *uid);
  int name_to_gid(char *name, gid_t *gid);
  int lock_range(int fd, int offset, int len);
! void glob_expand(char *base1, char ***argv, int *argc, int *maxargs);
  void strlower(char *s);
  void *Realloc(void *p, int size);
  void clean_fname(char *name);
*** clientserver.c.orig	Sun Jan 26 20:08:14 2003
--- clientserver.c	Sun Aug  3 23:34:00 2003
***************
*** 198,204 ****
  static int rsync_module(int f_in, int f_out, int i)
  {
  	int argc=0;
! 	char *argv[MAX_ARGS];
  	char **argp;
  	char line[MAXPATHLEN];
  	uid_t uid = (uid_t)-2;  /* canonically "nobody" */
--- 198,205 ----
  static int rsync_module(int f_in, int f_out, int i)
  {
  	int argc=0;
! 	int maxargs=0;
! 	char **argv=0;
  	char **argp;
  	char line[MAXPATHLEN];
  	uid_t uid = (uid_t)-2;  /* canonically "nobody" */
***************
*** 243,249 ****
  		}
  		return -1;
  	}
- 
  	
  	auth_user = auth_server(f_in, f_out, i, addr, "@RSYNCD: AUTHREQD ");
  
--- 244,249 ----
***************
*** 373,378 ****
--- 373,382 ----
  
  	io_printf(f_out, "@RSYNCD: OK\n");
  
+ 	maxargs = MAX_ARGS;
+ 	if (! (argv = (char **) malloc(maxargs * sizeof(char *)))) {
+ 		out_of_memory("rsync_module");
+ 	}
  	argv[argc++] = "rsyncd";
  
  	while (1) {
***************
*** 386,391 ****
--- 390,396 ----
  
  		argv[argc] = strdup(p);
  		if (!argv[argc]) {
+ 			free(argv);
  			return -1;
  		}
  
***************
*** 394,400 ****
  				request = strdup(p);
  				start_glob++;
  			}
! 			glob_expand(name, argv, &argc, MAX_ARGS);
  		} else {
  			argc++;
  		}
--- 399,405 ----
  				request = strdup(p);
  				start_glob++;
  			}
! 			glob_expand(name, &argv, &argc, &maxargs);
  		} else {
  			argc++;
  		}
***************
*** 403,410 ****
  			start_glob = 1;
  		}
  
! 		if (argc == MAX_ARGS) {
! 			return -1;
  		}
  	}
  
--- 408,419 ----
  			start_glob = 1;
  		}
  
! 		if (argc == maxargs) {
! 			maxargs *= 2;
! 			argv = (char **) realloc((void *) argc, maxargs);
! 			if (! argv) {
! 				out_of_memory("rsync_module");
! 			}
  		}
  	}
  
***************
*** 457,462 ****
--- 466,472 ----
           * message back to them. */
  	if (!ret) {
                  option_error();
+ 		free(argv);
                  exit_cleanup(RERR_UNSUPPORTED);
  	}
  
***************
*** 466,471 ****
--- 476,482 ----
  	}
  
  	start_server(f_in, f_out, argc, argp);
+ 	free(argv);
  
  	return 0;
  }
*** util.c.orig	Sun Jan 19 21:37:11 2003
--- util.c	Sun Aug  3 23:33:40 2003
***************
*** 458,469 ****
  	return fcntl(fd,F_SETLK,&lock) == 0;
  }
  
! 
! static void glob_expand_one(char *s, char **argv, int *argc, int maxargs)
  {
  #if !(defined(HAVE_GLOB) && defined(HAVE_GLOB_H))
  	if (!*s) s = ".";
! 	argv[*argc] = strdup(s);
  	(*argc)++;
  	return;
  #else
--- 458,468 ----
  	return fcntl(fd,F_SETLK,&lock) == 0;
  }
  
! static void glob_expand_one(char *s, char ***argv, int *argc, int *maxargs)
  {
  #if !(defined(HAVE_GLOB) && defined(HAVE_GLOB_H))
  	if (!*s) s = ".";
! 	(*argv)[*argc] = strdup(s);
  	(*argc)++;
  	return;
  #else
***************
*** 473,503 ****
  
  	if (!*s) s = ".";
  
! 	argv[*argc] = strdup(s);
  	if (sanitize_paths) {
! 		sanitize_path(argv[*argc], NULL);
  	}
  
  	memset(&globbuf, 0, sizeof(globbuf));
! 	glob(argv[*argc], 0, NULL, &globbuf);
  	if (globbuf.gl_pathc == 0) {
  		(*argc)++;
  		globfree(&globbuf);
  		return;
  	}
! 	for (i=0; i<(maxargs - (*argc)) && i < (int) globbuf.gl_pathc;i++) {
! 		if (i == 0) free(argv[*argc]);
! 		argv[(*argc) + i] = strdup(globbuf.gl_pathv[i]);
! 		if (!argv[(*argc) + i]) out_of_memory("glob_expand");
  	}
  	globfree(&globbuf);
  	(*argc) += i;
  #endif
  }
  
! void glob_expand(char *base1, char **argv, int *argc, int maxargs)
  {
! 	char *s = argv[*argc];
  	char *p, *q;
  	char *base = base1;
  
--- 472,510 ----
  
  	if (!*s) s = ".";
  
! 	(*argv)[*argc] = strdup(s);
  	if (sanitize_paths) {
! 		sanitize_path((*argv)[*argc], NULL);
  	}
  
  	memset(&globbuf, 0, sizeof(globbuf));
! 	glob((*argv)[*argc], 0, NULL, &globbuf);
  	if (globbuf.gl_pathc == 0) {
  		(*argc)++;
  		globfree(&globbuf);
  		return;
  	}
! 	if ((int) globbuf.gl_pathc > *maxargs - *argc) {
! 		*maxargs += globbuf.gl_pathc + MAX_ARGS;
! 		*argv = (char **) realloc((void *) *argv,
! 		    *maxargs * sizeof (char *));
! 		if (! *argv) {
! 			out_of_memory("glob_expand");
! 		}
! 	}
! 	for (i=0; i < (int) globbuf.gl_pathc; i++) {
! 		if (i == 0) free((*argv)[*argc]);
! 		(*argv)[(*argc) + i] = strdup(globbuf.gl_pathv[i]);
! 		if (! (*argv)[(*argc) + i]) out_of_memory("glob_expand");
  	}
  	globfree(&globbuf);
  	(*argc) += i;
  #endif
  }
  
! void glob_expand(char *base1, char ***argv, int *argc, int *maxargs)
  {
! 	char *s = (*argv)[*argc];
  	char *p, *q;
  	char *base = base1;
  
***************
*** 513,526 ****
  	if (asprintf(&base," %s/", base1) <= 0) out_of_memory("glob_expand");
  
  	q = s;
! 	while ((p = strstr(q,base)) && ((*argc) < maxargs)) {
  		/* split it at this point */
  		*p = 0;
  		glob_expand_one(q, argv, argc, maxargs);
  		q = p+strlen(base);
  	}
  
! 	if (*q && (*argc < maxargs)) glob_expand_one(q, argv, argc, maxargs);
  
  	free(s);
  	free(base);
--- 520,543 ----
  	if (asprintf(&base," %s/", base1) <= 0) out_of_memory("glob_expand");
  
  	q = s;
! 	while ((p = strstr(q,base))) {
! 		if (*argc == *maxargs) {
! 			*maxargs *= 2;
! 			*argv = (char **) realloc((void *) *argv,
! 			    *maxargs * sizeof (char *));
! 			if (! *argv) {
! 				out_of_memory("glob_expand");
! 			}
! 		}
  		/* split it at this point */
  		*p = 0;
  		glob_expand_one(q, argv, argc, maxargs);
  		q = p+strlen(base);
  	}
  
! 	if (*q) {
! 		glob_expand_one(q, argv, argc, maxargs);
! 	}
  
  	free(s);
  	free(base);


More information about the rsync mailing list