Re: [PATCH 2/5] linux-uesr: make exec_path realpath

[Takashi Yamamoto]

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