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);