| Message ID | 20210524045412.15152-3-yamamoto@midokura.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | linux-user changes to run docker | expand |
YAMAMOTO Takashi <yamamoto@midokura.com> writes: > Otherwise, it can be easily fooled by the user app using chdir(). > > Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com> > --- > linux-user/main.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/linux-user/main.c b/linux-user/main.c > index 4dfc47ad3b..1f9f4e3820 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -55,6 +55,7 @@ > #endif > > char *exec_path; > +char exec_path_store[PATH_MAX]; Is there any point in keeping this as a static path rather than just allocating it off the heap? > > int singlestep; > static const char *argv0; > @@ -610,7 +611,10 @@ static int parse_args(int argc, char **argv) > exit(EXIT_FAILURE); > } > > - exec_path = argv[optind]; > + exec_path = realpath(argv[optind], exec_path_store); > + if (exec_path == NULL) { > + exec_path = argv[optind]; > + } exec_path = realpath(argv[optind], NULL) exec_path = exec_path ? exec_path : argv[optind]; what situations would we expect realpath to fail and in those cases is sticking to argv[optind] safe? > > return optind; > }
On Mon, May 24, 2021 at 7:59 PM Alex Bennée <alex.bennee@linaro.org> wrote: > > > YAMAMOTO Takashi <yamamoto@midokura.com> writes: > > > Otherwise, it can be easily fooled by the user app using chdir(). > > > > Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com> > > --- > > linux-user/main.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/linux-user/main.c b/linux-user/main.c > > index 4dfc47ad3b..1f9f4e3820 100644 > > --- a/linux-user/main.c > > +++ b/linux-user/main.c > > @@ -55,6 +55,7 @@ > > #endif > > > > char *exec_path; > > +char exec_path_store[PATH_MAX]; > > Is there any point in keeping this as a static path rather than just > allocating it off the heap? it's just the simplest given the api of realpath(). do you mean it's better to use malloc? why? > > > > > int singlestep; > > static const char *argv0; > > @@ -610,7 +611,10 @@ static int parse_args(int argc, char **argv) > > exit(EXIT_FAILURE); > > } > > > > - exec_path = argv[optind]; > > + exec_path = realpath(argv[optind], exec_path_store); > > + if (exec_path == NULL) { > > + exec_path = argv[optind]; > > + } > > exec_path = realpath(argv[optind], NULL) > exec_path = exec_path ? exec_path : argv[optind]; > > what situations would we expect realpath to fail and in those cases is > sticking to argv[optind] safe? i don't have any particular case in my mind. i guess it rarely fails and it might be simpler to just bail out on the failure. > > > > > return optind; > > } > > > -- > Alex Bennée
On Tue, May 25, 2021 at 7:59 AM Takashi Yamamoto <yamamoto@midokura.com> wrote: > > On Mon, May 24, 2021 at 7:59 PM Alex Bennée <alex.bennee@linaro.org> wrote: > > > > > > YAMAMOTO Takashi <yamamoto@midokura.com> writes: > > > > > Otherwise, it can be easily fooled by the user app using chdir(). > > > > > > Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com> > > > --- > > > linux-user/main.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/linux-user/main.c b/linux-user/main.c > > > index 4dfc47ad3b..1f9f4e3820 100644 > > > --- a/linux-user/main.c > > > +++ b/linux-user/main.c > > > @@ -55,6 +55,7 @@ > > > #endif > > > > > > char *exec_path; > > > +char exec_path_store[PATH_MAX]; > > > > Is there any point in keeping this as a static path rather than just > > allocating it off the heap? > > it's just the simplest given the api of realpath(). > do you mean it's better to use malloc? why? > > > > > > > > > int singlestep; > > > static const char *argv0; > > > @@ -610,7 +611,10 @@ static int parse_args(int argc, char **argv) > > > exit(EXIT_FAILURE); > > > } > > > > > > - exec_path = argv[optind]; > > > + exec_path = realpath(argv[optind], exec_path_store); > > > + if (exec_path == NULL) { > > > + exec_path = argv[optind]; > > > + } > > > > exec_path = realpath(argv[optind], NULL) > > exec_path = exec_path ? exec_path : argv[optind]; > > > > what situations would we expect realpath to fail and in those cases is > > sticking to argv[optind] safe? > > i don't have any particular case in my mind. > i guess it rarely fails and it might be simpler to just bail out on the failure. i recalled why i did this way. it was actually necessary for some apps. eg. runc it executes an unlinked binary via /proc/self/fd. i'll clean it up a bit and add comments. > > > > > > > > > return optind; > > > } > > > > > > -- > > Alex Bennée
diff --git a/linux-user/main.c b/linux-user/main.c index 4dfc47ad3b..1f9f4e3820 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -55,6 +55,7 @@ #endif char *exec_path; +char exec_path_store[PATH_MAX]; int singlestep; static const char *argv0; @@ -610,7 +611,10 @@ static int parse_args(int argc, char **argv) exit(EXIT_FAILURE); } - exec_path = argv[optind]; + exec_path = realpath(argv[optind], exec_path_store); + if (exec_path == NULL) { + exec_path = argv[optind]; + } return optind; }
Otherwise, it can be easily fooled by the user app using chdir(). Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com> --- linux-user/main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)