Venky, I added a patch as attachment which should work (its your patch
with if(shp->shcomp) added).
Could you test it with your scripts and make testshell, please?

David, can you take a look at the patch and see if it makes sense, please?

Olga

2010/3/28 ????? ???????????? <olga.kryzhanovska at gmail.com>:
> Venky, are you sure the patch works?
> I applied it to my work tree (which includes changes for
> http://cr.opensolaris.org/~fleyta/ksh93_update2_bugfix1_20100326_03_webrev/)
> and make testshell in usr/src/cmd/ksh/amd64 immediately crashed in the
> alias.sh module:
> ## Running amd64/ksh test: LANG='C' script='alias.sh', mode='compiled_script'
> #       test shcomp-alias.ksh begins
> #       ../../../lib/libshell/common/tests/shtests: line 130: 21908:
> Memory fault(coredump)
> #       test shcomp-alias.ksh failed to compile with exit code 267 [ 1
> test 1 error ]
> ##--------> test failed
> *** Error code 1
> The stack trace shows that shcomp blew up:
> core 'core' of 21908:
> /home/fleyta/ksh93/venky_setid/p030/proto/root_i386/usr/bin/shcomp
>  fedce25a nv_disc  (80743fc, 80747a0, 1, fedcf6b9) + 18a
>  fedcf6e9 nv_mount (80743fc, 0, 80746c0, fedae2e2) + 101
>  fedae43f inittree (8062238, fee1d940, 8062238, fedadbaa) + 16b
>  fedadbf8 nv_init  (8062238, fedaaee0, a, fedaca05) + 5c
>  fedacc4f sh_init  (2, 8046db4, 0, 0) + 40f
>  08051167 main     (2, 8046db4, 8046dc0, 8050fbf) + bb
>  0805101d _start   (2, 8046fa4, 8046fee, 0, 804701a, 804706d) + 7d
>
> Can you go to usr/src/cmd/ksh/amd64 in your workspace, do a make
> testshell and look if this crashes, please?
>
> Olga
>
> On Wed, Mar 24, 2010 at 2:20 PM, Venky <venkytv at opensolaris.org> wrote:
>> Hi Olga,
>>
>>> > Venky, does this issue occur even if you bypass isaexec, i.e.
>>
>> Yes, tried this with /usr/bin/sparcv9/ksh93 to make sure isaexec
>> does not complicate things.  It does seem to be because of the
>> arguments getting mangled in line 1217 of libshell/common/sh/init.c.
>>
>> A quick hack to restore the mangled arguments before exec (patch
>> attached) seems to fix this issue.  The $0 value remains messed up,
>> though.  It displays /dev/fd/XX as the script name, while a #! line
>> without arguments displays the correct script name.
>>
>> # cat >t1.ksh <<EOF
>> #!/usr/bin/sparcv9/ksh93 -p
>> echo \$0
>> EOF
>>
>> # cat >t2.ksh <<EOF
>> #!/usr/bin/sparcv9/ksh93
>> echo \$0
>> EOF
>>
>> # chmod +xs t[12].ksh
>> # ls -l t*
>> -rwsr-sr-x 1 root root 36 Mar 24 05:51 t1.ksh
>> -rwsr-sr-x 1 root root 33 Mar 24 05:51 t2.ksh
>> # exit
>>
>> $ ./t1.ksh
>> /dev/fd/4
>> $ ./t2.ksh
>> t2.ksh
>>
>> Venky.
>>
>> On Wed, Mar 24, 2010 at 03:13:08AM +0100, ????? ???????????? wrote:
>>> 2010/3/24 ?????????? ???????????????????????? <olga.kryzhanovska at 
>>> gmail.com>:
>>> > Venky, does this issue occur even if you bypass isaexec, i.e.
>>> > #!/usr/bin/i86/ksh -p
>>>
>>> Correction:
>>> #!/usr/bin/i86/ksh93 -p
>>>
>>> > or
>>> > #!/usr/bin/sparcv0/ksh -p
>>>
>>> Correction:
>>> #!/usr/bin/sparcv9/ksh93 -p
>>>
>>> >
>>> > Olga
>>> >
>>> > On Fri, Mar 19, 2010 at 4:06 PM, Venky <venkytv at opensolaris.org> wrote:
>>> >> Have been investigating CR 6934836.
>>> >>
>>> >> 6934836 set-uid script with -p in magic number gets Exec format error
>>> >> http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6934836
>>> >>
>>> >> Have a few questions I'm hoping the ksh93 folks here will be able
>>> >> to help me with.
>>> >>
>>> >> It looks like the bug is due to the fact that set-uid scripts get
>>> >> passed to the shell as a /dev/fd/XX parameter instead of the actual
>>> >> path.  This has problems with ksh93 *only* if there are any options
>>> >> passed on the command line.
>>> >>
>>> >> The test program below demonstrates this:
>>> >>
>>> >> ----------
>>> >>
>>> >> $ cat testexec.c
>>> >> #include <stdio.h>
>>> >> #include <fcntl.h>
>>> >> #include <unistd.h>
>>> >>
>>> >> int
>>> >> main()
>>> >> {
>>> >>        int fd = -1;
>>> >>        char devfd[32];
>>> >>        char *script = "/tmp/ok.ksh";  /* Can be any simple script */
>>> >>
>>> >>        fd = open(script, O_RDONLY);
>>> >>        sprintf(devfd, "/dev/fd/%d", fd);
>>> >>        execl("/usr/bin/sparcv9/ksh93", "ksh", "-v", devfd, NULL);
>>> >> }
>>> >> $ ./testexec
>>> >> /usr/bin/ksh: /usr/bin/ksh: cannot execute [Exec format error]
>>> >>
>>> >> ----------
>>> >>
>>> >> The culprit seems to be the code below:
>>> >>
>>> >> <lib/libshell/common/sh/init.c>
>>> >> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libshell/common/sh/init.c#1216
>>> >>
>>> >> 1216   shp->st.dolv=argv+(argc-1)-shp->st.dolc;
>>> >> 1217   shp->st.dolv[0] = argv[0];
>>> >>
>>> >> Here, we are overwriting one of the arguments of argv (because
>>> >> shp->st.dolv indexes into the argv vector).
>>> >>
>>> >> In this particular case, argv which originally looked like this:
>>> >>
>>> >>    ksh, -v, /dev/fd/3
>>> >>
>>> >> ends up looking like this:
>>> >>
>>> >>    ksh, ksh, /dev/fd/3
>>> >>
>>> >> We then pass the mangled argv to execv():
>>> >>
>>> >> <lib/libshell/common/sh/main.c>
>>> >> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libshell/common/sh/main.c#298
>>> >>
>>> >> 298   /*  exec to change $0 for ps  */
>>> >> 299   execv(pathshell(),av);
>>> >>
>>> >> As a consequence, ksh tries to load the ksh binary as a shell script and
>>> >> fails with an "Exec format" error.
>>> >>
>>> >> Have been digging around trying to figure out what is the right
>>> >> thing to do in this situation.  Figured some of the people more
>>> >> familiar with the ksh93 source might be able to help.
>>> >>
>>> >> Also, the execv() call above uses pathshell() which seems plain wrong.
>>> >> The whole exec hack here seems to be to make sure $0 is set correctly
>>> >> for ps.  But pathshell() looks at the SHELL variable and might end up
>>> >> executing the script with a different shell altogether.
>>> >>
>>> >> Any help appreciated.
>>> >>
>>> >> Thanks,
>>> >> Venky.
>>> >> _______________________________________________
>>> >> ksh93-integration-discuss mailing list
>>> >> ksh93-integration-discuss at opensolaris.org
>>> >> http://mail.opensolaris.org/mailman/listinfo/ksh93-integration-discuss
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> >      ,   _                                    _   ,
>>> >     { \/`o;====-    Olga Kryzhanovska   -====;o`\/ }
>>> > .----'-/`-/     olga.kryzhanovska at gmail.com   \-`\-'----.
>>> >  `'-..-| /     Solaris/BSD//C/C++ programmer   \ |-..-'`
>>> >      /\/\                                     /\/\
>>> >      `--`                                      `--`
>>> >
>>>
>>>
>>>
>>> --
>>>       ,   _                                    _   ,
>>>      { \/`o;====-    Olga Kryzhanovska   -====;o`\/ }
>>> .----'-/`-/     olga.kryzhanovska at gmail.com   \-`\-'----.
>>>  `'-..-| /     Solaris/BSD//C/C++ programmer   \ |-..-'`
>>>       /\/\                                     /\/\
>>>       `--`                                      `--`
>>
>
>
>
> --
>      ,   _                                    _   ,
>     { \/`o;====-    Olga Kryzhanovska   -====;o`\/ }
> .----'-/`-/     olga.kryzhanovska at gmail.com   \-`\-'----.
>  `'-..-| /     Solaris/BSD//C/C++ programmer   \ |-..-'`
>      /\/\                                     /\/\
>      `--`                                      `--`
>



-- 
      ,   _                                    _   ,
     { \/`o;====-    Olga Kryzhanovska   -====;o`\/ }
.----'-/`-/     olga.kryzhanovska at gmail.com   \-`\-'----.
 `'-..-| /     Solaris/BSD//C/C++ programmer   \ |-..-'`
      /\/\                                     /\/\
      `--`                                      `--`
-------------- next part --------------
Index: usr/src/lib/libshell/common/include/defs.h
===================================================================
--- usr/src/lib/libshell/common/include/defs.h  (revision 1854)
+++ usr/src/lib/libshell/common/include/defs.h  (working copy)
@@ -97,6 +97,8 @@
        char            **otrapcom;
        void            *timetrap;
        struct Ufunction *real_fun;     /* current 'function name' function */
+       int             repl_index;
+       char            *repl_arg;
 };
 
 struct limits
Index: usr/src/lib/libshell/common/sh/init.c
===================================================================
--- usr/src/lib/libshell/common/sh/init.c       (revision 1854)
+++ usr/src/lib/libshell/common/sh/init.c       (working copy)
@@ -1192,6 +1192,7 @@
        job_clear();
        if(argc>0)
        {
+               int dolv_index = -1;
                /* check for restricted shell */
                if(type&SH_TYPE_RESTRICTED)
                        sh_onoption(SH_RESTRICTED);
@@ -1219,7 +1220,17 @@
                        sh_done(shp,0);
                }
                opt_info.disc = 0;
-               shp->st.dolv=argv+(argc-1)-shp->st.dolc;
+               if (shp->shcomp)
+               {
+                       shp->st.dolv=argv+(argc-1)-shp->st.dolc;
+               }
+               else
+               {
+                       dolv_index = (argc-1)-shp->st.dolc;
+                       shp->st.dolv=argv+dolv_index;
+                       shp->st.repl_index = dolv_index;
+                       shp->st.repl_arg = argv[dolv_index];
+               }
                shp->st.dolv[0] = argv[0];
                if(shp->st.dolc < 1)
                        sh_onoption(SH_SFLAG);
Index: usr/src/lib/libshell/common/sh/main.c
===================================================================
--- usr/src/lib/libshell/common/sh/main.c       (revision 1854)
+++ usr/src/lib/libshell/common/sh/main.c       (working copy)
@@ -292,6 +292,8 @@
                                         * try to undo effect of solaris 2.5+
                                         * change for argv for setuid scripts
                                         */
+                                       if (!shp->shcomp && shp->st.repl_index 
> 0)
+                                               av[shp->st.repl_index] = 
shp->st.repl_arg;
                                        if(((type = sh_type(cp = av[0])) & 
SH_TYPE_SH) && (!(name = nv_getval(L_ARGNOD)) || !((type = sh_type(cp = name)) 
& SH_TYPE_SH)))
                                        {
                                                av[0] = (type & SH_TYPE_LOGIN) 
? cp : path_basename(cp);

Reply via email to