diff mbox

ext-toolchain-wrapper: fix paths if executable was resolved by PATH

Message ID 51A5CD3F.1070100@fh-kl.de
State Accepted
Headers show

Commit Message

Patrick Ziegler May 29, 2013, 9:41 a.m. UTC 
If ext-toolchain-wrapper or any symbolic link to it was resolved by PATH,
the wrapper takes the working directory to calculate the relative paths.

Now '/proc/self/exe' is used to resolve the absolute path to the toolchain
directory if the wrapper was called neither with a relative nor an absolute
path.

Signed-off-by: Patrick Ziegler <patrick.ziegler@fh-kl.de>
---
 toolchain/toolchain-external/ext-toolchain-wrapper.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Lionel Orry May 30, 2013, 9:47 a.m. UTC | #1 
On Wed, May 29, 2013 at 11:41 AM, Patrick Ziegler
<patrick.ziegler@fh-kl.de> wrote:
> If ext-toolchain-wrapper or any symbolic link to it was resolved by PATH,
> the wrapper takes the working directory to calculate the relative paths.
>
> Now '/proc/self/exe' is used to resolve the absolute path to the toolchain
> directory if the wrapper was called neither with a relative nor an absolute
> path.

This patch saved my day and I could remove my own weird customizations
to ext-toolchain-wrapper. Works like a charm, I'd like to see it
integrated.

BR,
Lionel

>
> Signed-off-by: Patrick Ziegler <patrick.ziegler@fh-kl.de>
> ---
>  toolchain/toolchain-external/ext-toolchain-wrapper.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/toolchain/toolchain-external/ext-toolchain-wrapper.c b/toolchain/toolchain-external/ext-toolchain-wrapper.c
> index 9a2fc70..e71a90a 100644
> --- a/toolchain/toolchain-external/ext-toolchain-wrapper.c
> +++ b/toolchain/toolchain-external/ext-toolchain-wrapper.c
> @@ -61,7 +61,7 @@ int main(int argc, char **argv)
>         char *relbasedir, *absbasedir;
>         char *progpath = argv[0];
>         char *basename;
> -       int ret;
> +       int ret, i, count = 0;
>
>         /* Calculate the relative paths */
>         basename = strrchr(progpath, '/');
> @@ -77,7 +77,19 @@ int main(int argc, char **argv)
>                 absbasedir = realpath(relbasedir, NULL);
>         } else {
>                 basename = progpath;
> -               absbasedir = realpath("../..", NULL);
> +               absbasedir = malloc(PATH_MAX + 1);
> +               ret = readlink("/proc/self/exe", absbasedir, PATH_MAX);
> +               if (ret < 0) {
> +                       perror(__FILE__ ": readlink");
> +                       return 2;
> +               }
> +               for (i = ret; i > 0; i--) {
> +                       if ('/' == absbasedir[i]) {
> +                               absbasedir[i] = '\0';
> +                               if (3 == ++count)
> +                                       break;
> +                       }
> +               }
>         }
>         if (absbasedir == NULL) {
>                 perror(__FILE__ ": realpath");
> --
> 1.8.1.2
>
>
> --
> Dipl.-Inf. (FH) Patrick Ziegler
>
> University Of Applied Sciences
> Kaiserslautern
>
> Amerikastrasse 1
> D-66482 Zweibruecken
> Germany
>
> Phone:  +49 631 3724 5526
> Mail:   patrick.ziegler@fh-kl.de
> PGP KeyID 0xB4796B8C
>
> http://www.fh-kl.de
> http://www.fh-kl.de/fachbereiche/imst/iuk-knowhow.html
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni May 31, 2013, 3:25 p.m. UTC | #2 
Dear Patrick Ziegler,

On Wed, 29 May 2013 11:41:19 +0200, Patrick Ziegler wrote:
> If ext-toolchain-wrapper or any symbolic link to it was resolved by PATH,
> the wrapper takes the working directory to calculate the relative paths.
> 
> Now '/proc/self/exe' is used to resolve the absolute path to the toolchain
> directory if the wrapper was called neither with a relative nor an absolute
> path.
> 
> Signed-off-by: Patrick Ziegler <patrick.ziegler@fh-kl.de>

Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

This should be part of 2013.05 if it's not too late, since it fixes a
real problem. My colleague Gregory (Cc'ed) was hit by the problem, and
both of us confirmed that this patch is fixing the problem.

Best regards,

Thomas
Peter Korsgaard May 31, 2013, 8:05 p.m. UTC | #3 
>>>>> "Patrick" == Patrick Ziegler <patrick.ziegler@fh-kl.de> writes:

 Patrick> If ext-toolchain-wrapper or any symbolic link to it was
 Patrick> resolved by PATH, the wrapper takes the working directory to
 Patrick> calculate the relative paths.

 Patrick> Now '/proc/self/exe' is used to resolve the absolute path to
 Patrick> the toolchain directory if the wrapper was called neither with
 Patrick> a relative nor an absolute path.

 Patrick> Signed-off-by: Patrick Ziegler <patrick.ziegler@fh-kl.de>
 Patrick> ---
 Patrick>  toolchain/toolchain-external/ext-toolchain-wrapper.c | 16 ++++++++++++++--
 Patrick>  1 file changed, 14 insertions(+), 2 deletions(-)

 Patrick> diff --git a/toolchain/toolchain-external/ext-toolchain-wrapper.c b/toolchain/toolchain-external/ext-toolchain-wrapper.c
 Patrick> index 9a2fc70..e71a90a 100644
 Patrick> --- a/toolchain/toolchain-external/ext-toolchain-wrapper.c
 Patrick> +++ b/toolchain/toolchain-external/ext-toolchain-wrapper.c
 Patrick> @@ -61,7 +61,7 @@ int main(int argc, char **argv)
 Patrick>  	char *relbasedir, *absbasedir;
 Patrick>  	char *progpath = argv[0];
 Patrick>  	char *basename;
 Patrick> -	int ret;
 Patrick> +	int ret, i, count = 0;
 
 Patrick>  	/* Calculate the relative paths */
 Patrick>  	basename = strrchr(progpath, '/');
 Patrick> @@ -77,7 +77,19 @@ int main(int argc, char **argv)
 Patrick>  		absbasedir = realpath(relbasedir, NULL);
 Patrick>  	} else {
 Patrick>  		basename = progpath;
 Patrick> -		absbasedir = realpath("../..", NULL);
 Patrick> +		absbasedir = malloc(PATH_MAX + 1);
 Patrick> +		ret = readlink("/proc/self/exe", absbasedir, PATH_MAX);
 Patrick> +		if (ret < 0) {
 Patrick> +			perror(__FILE__ ": readlink");
 Patrick> +			return 2;
 Patrick> +		}

 Patrick> +		for (i = ret; i > 0; i--) {
 Patrick> +			if ('/' == absbasedir[i]) {

The output of readlink is NOT null terminated, so you end up accessing
uninitialized data on the first iteration of this loop. I've fixed it by
an explicit:

absbasedir[ret] = '\0';

In the rest of the file we don't do the reverse order checks (value ==
var), so I've swapped those.

The toolchain wrapper has unfortunately grown quite complicated since
I've added it. I THINK we can simply always resolve /proc/self/exe and
get rid of the conditional here, but as we're this close to the release
I decided to apply your patch as it seems the safest option.

Committed, thanks.
diff mbox

Patch

diff --git a/toolchain/toolchain-external/ext-toolchain-wrapper.c b/toolchain/toolchain-external/ext-toolchain-wrapper.c
index 9a2fc70..e71a90a 100644
--- a/toolchain/toolchain-external/ext-toolchain-wrapper.c
+++ b/toolchain/toolchain-external/ext-toolchain-wrapper.c
@@ -61,7 +61,7 @@  int main(int argc, char **argv)
 	char *relbasedir, *absbasedir;
 	char *progpath = argv[0];
 	char *basename;
-	int ret;
+	int ret, i, count = 0;
 
 	/* Calculate the relative paths */
 	basename = strrchr(progpath, '/');
@@ -77,7 +77,19 @@  int main(int argc, char **argv)
 		absbasedir = realpath(relbasedir, NULL);
 	} else {
 		basename = progpath;
-		absbasedir = realpath("../..", NULL);
+		absbasedir = malloc(PATH_MAX + 1);
+		ret = readlink("/proc/self/exe", absbasedir, PATH_MAX);
+		if (ret < 0) {
+			perror(__FILE__ ": readlink");
+			return 2;
+		}
+		for (i = ret; i > 0; i--) {
+			if ('/' == absbasedir[i]) {
+				absbasedir[i] = '\0';
+				if (3 == ++count)
+					break;
+			}
+		}
 	}
 	if (absbasedir == NULL) {
 		perror(__FILE__ ": realpath");