[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MiNT] XaAES: bug(s) in shel_write()



> What is SHD_ENVIRON? I've found

Its the same as SW_ENVIRON. I took the name from the XaAES sources (the ones that are part of MiNT).

> What exactly did you write in ARGV?

The arguments that should be passed to the program of course, including the command name. That part works, i even verified it again when that bug first appeared.

> longtail is not used in ARGV, it rather a flag in this case (at least this 
> is how I understand this code).

Yes, i understand that code in the same manner. But it is only set when the cmd tail contains 127 as its length, indicating that ARGV method should be used.

> It is:
>
>         if (tailsize > 126)
>        {
>            longtail = tailsize;
>            tail[0] = 0x7f;

But that code is only used, when the new command line (the constructed from prepending the command name) exceeds 126.

> Looks like that is not supported by XaAES.

The code look like it tries to do so, but fails to do it correctly. If the original arguments are too long to be passed in the basepage, and this fact is mentioned as 127 being used as length, the function should either fail with an error indicating that this is not supported, or support it correctly. It does not make sense to pass wrong, truncated arguments to the program instead, this can be rather disastrous. BTW XaAES does indicate in appl_getinfo(10) that ARGV *is* supported.

> Only if SW_ENVIRON is set.

But this is the only way to pass the environment in the call that contains the ARGV element. The application itself does not create a new process where this could be done, nor does XaAES has access to the process' environment.

> Do you have a simple test-case?

Not really. It has to be a complete GEM program after all. I can try to make one if it is really needed. But i hope my explanations made clear that something is going wrong there. Not using the environment that was constructed by make_argv() is at least definitely a bug. And that code is used also when the original parameters would fit, but the new ones don't. That is a case that is rather common i guess, given that you normally pass at least one path of a file to a TOS program to do something with it, and the code prepends a 2nd path to the argument. It is more than likely that this will exceed the limit of 126 characters.



Helmut Karlowski <helmut.karlowski@ish.de> schrieb am 23:40 Donnerstag, 4.Februar 2016:


Am 04.02.2016, 19:31 Uhr, schrieb Thorsten Otto:

> I recently tried to run a TOS application via shel_write(), using  an 
> extended to pass the command line parameters via ARGV. After some 
> results it looks like there are at least 2 bugs in the current 
> implementation.
> What i did wasa) construct a new enviroment block including the ARGV 
> command line parameters.b) put a pointer to that block, and a pointer to 
> the program name into a x_shell-structurec) call 
> shel_write(SHW_EXEC|SHD_ENVIRON, 0, 1, &x_shell, cmd) where cmd[0] 
> contains 0x7f

What is SHD_ENVIRON? I've found

#define SWM_ENVIRON  8
#define SW_ENVIRON    0x0800

in mt_gem.h in the xaaes-source-dir.

>  What happened was, that tw-call tried to execute a program named "-p" 
> (the first parameter that was intended to be passed to my program).After

What exactly did you write in ARGV?

> some investigation, i think found out the reason for this. When running 
> a TOS program, shel_write executes tw-call instead (or whatever happens 
> to be in the environment variable TOSRUN), constructing a new command 
> line to included the original program name. There are however a few 
> things that  look strange:a) in function launch(), there is code like:
>        if (wiscr == 1 || p_tail[0] == 0x7f || (unsigned char)p_tail[0] 
> == 0xff)
>        {
>            /* In this case the string CAN ONLY BE null terminated. */
>            longtail = strlen(p_tail + 1);
>            if (longtail > 124)
>                longtail = 124;
>            DIAG((D_shel, NULL, "ARGV!  longtail = %ld", longtail));
>        }

longtail is not used in ARGV, it rather a flag in this case (at least this 
is how I understand this code).

The real tail is made up from tail in #584 (my branch):
          strncpy(new_tail + new_tailsize + 1, tail + 1, tailsize);

tailsize is the length used in make_argv().

>    Why is the commandline restricted to 124, when it was expclicity 
> specified that ARGV method should be used? The variable longtail is not 
> adjusted later,

It is:

          if (tailsize > 126)
          {
            longtail = tailsize;
            tail[0] = 0x7f;
...

> nor does the function try to get the actial parameters from the ARGV 
> environment variable.

Looks like that is not supported by XaAES.

> b) if the variable longtail is set, a new enviroment is created using 
> make_argv(). However, make_argv() stashes the build environment block 
> into C.env, while create_process() passes my Environment block to 
> tw-call.

Only if SW_ENVIRON is set.


> The outcome of this is that tw-call parses the original command line 
> parameters, instead of the ones including its own path, ie. it only 
> sees"myprogram -p" instead of "tw-call myprogram -p".


Do you have a simple test-case?

-Helmut



--