Re: Comments on the pax patch?

[Guillem Jover <guillem@debian.org>]

On Wed, 2011-10-12 at 21:03:13 +0200, Svante Signell wrote:
> Attached is a patch to make pax-20090728 build under GNU/Hurd. Function
> calls changed are readlink()->xreadlink() (new, same as for psmisc) and

The same comment here, I've mentioned before the lstat/malloc/readlink
combo before. I even sent a patch to the man-pages upstream to
document it, now merged, please see:

  <http://man7.org/linux/man-pages/online/pages/man2/readlink.2.html>

> realpath()->canonicalize_file_name().

Does the code only support GNU based systems? Otherwise this cannot be
changed unconditionally.

> diff -ur pax-20090728/file_subs.c pax-20090728.modified/file_subs.c
> --- pax-20090728/file_subs.c	2009-07-28 17:38:28.000000000 +0000
> +++ pax-20090728.modified/file_subs.c	2011-10-12 19:03:24.000000000 +0000
> @@ -374,17 +396,16 @@
[...]
> -					target[len] = '\0';
>  					nm = target;
> +					free(target);
>  				}
>  			}

If you assign and immediately free then nm will point to undefined
memory...

>  			res = mkdir(nm, file_mode);

... here.

> diff -ur pax-20090728/tables.c pax-20090728.modified/tables.c
> --- pax-20090728/tables.c	2009-07-28 17:38:28.000000000 +0000
> +++ pax-20090728.modified/tables.c	2011-10-12 18:34:21.000000000 +0000
> @@ -1126,17 +1126,18 @@
>  add_dir(char *name, struct stat *psb, int frc_mode)
>  {
>  	DIRDATA *dblk;
> -	char realname[MAXPATHLEN], *rp;
> +	char *rp;
>  
>  	if (dirp == NULL)
>  		return;
>  
>  	if (havechd && *name != '/') {
> -		if ((rp = realpath(name, realname)) == NULL) {
> +	  if ((rp = canonicalize_file_name(name)) == NULL) {
>  			paxwarn(1, "Cannot canonicalize %s", name);
>  			return;
>  		}
>  		name = rp;
> +		free(rp);

Same undefined memory access after here.

thanks,
guillem