On Mon, Jul 9, 2012 at 6:16 PM, Roland Mainz <roland.ma...@nrubsig.org> wrote: > On Mon, Jul 9, 2012 at 12:59 PM, Lionel Cons > <lionelcons1...@googlemail.com> wrote: > [snip] >> Don't have much time so only a few comments: > [snip] >> 4. Has anyone tested Linus's patch for the "unfortunate engineering mistake"? > > We didn't test it (Olga and I aren't familiar with the Linux kernel) > ... but I'll ask around... > >> 5. Non-AIX/Solaris platforms, like Linux: Facing the "unfortunate >> engineering mistake", is there a way around this, like doing a >> open(".", ...) after each chdir() in b_cd() to get an handle to >> current working directory? I'll make the fix available on all >> platforms, right? > > Uhm... that will only work if we have the "read" permission for that > directory... |O_SEARCH| and Linux's |O_PATH| were added to address the > issue and allow an application to obtain a file handle to a directory > for which the user only has the "execute" permission. > But I can modify the patch to do that... in that case all platforms > would use the |fchdir()| codepath except for the case that a user has > only "execute" but no "read" permission... then we have to fall-back > to |chdir()|. > >> 6. Portability: O_SEARCH/O_PATH symbols are a precondition for O_XATTR >> but not the other way around? Otherwise your #ifdef logic will break. > > Yes... a platform can only have |O_XATTR| if it also has |O_SEARCH|. > Only Linux has |O_PATH| but no |O_XATTR|. > >> 7. Please ask David whether you can pass the cwd dir handle to >> builtins using the builtin api. It'll make our own builtin code much >> easier since we already mandate the ...at() api everywhere. > > I'll ask him... > > I'll craft a new patch this evening...
Attached (as "astksh_subshell_restore_cwd_xattr20120710_001.diff.txt") is a new version of the patch... ... there is one major problem: ksh93 with this patch passes all tests on Solaris 11 and AIX but it does not pass them on Linux... and I have no clue why this happens... ;-( ... David: Can you have a look, please ? ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) roland.ma...@nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 3992797 (;O/ \/ \O;)
diff -r -u original/src/cmd/ksh93/bltins/cd_pwd.c build_xattr/src/cmd/ksh93/bltins/cd_pwd.c --- src/cmd/ksh93/bltins/cd_pwd.c 2012-05-29 15:08:17.000000000 +0200 +++ src/cmd/ksh93/bltins/cd_pwd.c 2012-07-11 03:24:38.323051129 +0200 @@ -19,8 +19,8 @@ ***********************************************************************/ #pragma prototyped /* - * cd [-LP] [dirname] - * cd [-LP] [old] [new] + * cd [-LP@] [dirname] + * cd [-LP@] [old] [new] * pwd [-LP] * * David Korn @@ -38,6 +38,11 @@ #include "builtins.h" #include <ls.h> +/* Repeat syscall in expr each time it gets hit with EINTR */ +#ifndef EINTR_REPEAT +#define EINTR_REPEAT(expr) while((expr) && (errno == EINTR)) errno=0; +#endif + /* * Invalidate path name bindings to relative paths */ @@ -49,6 +54,89 @@ _nv_unset(np,0); } +int sh_diropenat(Shell_t *shp, int dir, const char *path, int xattr) +{ + int fd; + int shfd; + int savederrno; + +#ifdef O_XATTR + if(xattr) + { + int apfd; /* attribute parent fd */ + + /* open parent node... */ + EINTR_REPEAT((apfd = openat(dir, path, O_RDONLY|O_NONBLOCK|O_cloexec)) < 0); + if(apfd < 0) + return -1; + + /* ... and then open a fd to the attribute directory */ + EINTR_REPEAT((fd = openat(apfd, e_dot, O_XATTR|O_cloexec)) < 0); + + savederrno = errno; + EINTR_REPEAT(close(apfd) < 0); + errno = savederrno; + } + else +#endif + { +#ifdef AT_FDCWD + /* + * Open directory. First we try without |O_SEARCH| and + * if this fails with EACCESS we try with |O_SEARCH| + * again. + * This is required ... + * - ... because some platforms may require that it can + * only be used for directories while some filesystems + * (e.g. Reiser4 or HSM systems) allow a |fchdir()| into + * files, too) + * - ... to preserve the semantics of "cd", e.g. + * otherwise "cd" would return [No access] instead of + * [Not a directory] for files on filesystems which do + * not allow a "cd" into files. + * - ... to allow that a + * $ redirect {n}</etc ; cd /dev/fd/$n # works on most + * platforms. + */ + EINTR_REPEAT((fd = openat(dir, path, O_cloexec)) < 0); +#ifdef O_SEARCH + if((fd < 0) && (errno == EACCES)) + { + EINTR_REPEAT((fd = openat(dir, path, O_SEARCH|O_cloexec)) < 0) + } +#endif +#else + /* + * Version of openat() call above for systems without + * openat API. This only works because we basically + * gurantee that |dir| is always the same place as + * |cwd| on such machines (but this won't be the case + * in the future). + */ + EINTR_REPEAT((fd = open(path, O_cloexec)) < 0); +#endif + } + + if(fd < 0) + return fd; + + /* Move fd to a number > 10 and *register* the fd number with the shell */ + shfd = sh_fcntl(fd, F_DUPFD, 10); + savederrno=errno; + + EINTR_REPEAT(close(fd) < 0); + + /* + * FIXME: |sh_fcntl()| should implement F_DUPFD_CLOEXEC + * and F_DUP2FD_CLOEXEC to reduce the number of system calls + */ + if(shfd >=0) + sh_fcntl(shfd, F_SETFD, FD_CLOEXEC); + errno=savederrno; + + return shfd; +} + int b_cd(int argc, char *argv[],Shbltin_t *context) { register char *dir; @@ -56,8 +144,10 @@ register const char *dp; register Shell_t *shp = context->shp; int saverrno=0; - int rval,flag=0; + int rval,flag=0,xattr=0; char *oldpwd; + int newdirfd; + Namval_t *opwdnod, *pwdnod; if(sh_isoption(SH_RESTRICTED)) errormsg(SH_DICT,ERROR_exit(1),e_restricted+4); @@ -69,6 +159,11 @@ case 'P': flag = 1; break; +#ifdef O_XATTR + case '@': + xattr = 1; + break; +#endif case ':': errormsg(SH_DICT,2, "%s", opt_info.arg); break; @@ -131,6 +226,15 @@ cdpath = 0; } } +#ifdef O_XATTR + if (xattr) +#ifdef PATH_BFPATH + cdpath = NULL +#else + cdpath = "" +#endif + ; +#endif rval = -1; do { @@ -171,15 +275,77 @@ continue; #endif /* SHOPT_FS_3D */ } - if((rval=chdir(path_relative(shp,stakptr(PATH_OFFSET)))) >= 0) - goto success; - if(errno!=ENOENT && saverrno==0) + + rval = newdirfd = sh_diropenat(shp, + ((shp->pwdfd >= 0)?shp->pwdfd:AT_FDCWD), + path_relative(shp,stakptr(PATH_OFFSET)), + xattr); + if(newdirfd >=0) + { + /* chdir for directories on HSM/tapeworms may take minutes */ + EINTR_REPEAT((rval=fchdir(newdirfd)) < 0); + if(rval >= 0) + { + if(shp->pwdfd >= 0) + sh_close(shp->pwdfd); + shp->pwdfd=newdirfd; + goto success; + } + } +#ifndef O_SEARCH + else + { + rval=chdir(path_relative(shp,stakptr(PATH_OFFSET))); + if(rval >=0) + { + if(shp->pwdfd >= 0) + { + sh_close(shp->pwdfd); + shp->pwdfd=-1; + } + } + } +#endif + if(saverrno==0) saverrno=errno; + if(newdirfd >=0) + sh_close(newdirfd); } while(cdpath); - if(rval<0 && *dir=='/' && *(path_relative(shp,stakptr(PATH_OFFSET)))!='/') - rval = chdir(dir); /* use absolute chdir() if relative chdir() fails */ + if(rval<0 && *dir=='/' && *(path_relative(shp,stakptr(PATH_OFFSET)))!='/') + { + rval = newdirfd = sh_diropenat(shp, + ((shp->pwdfd >= 0)?shp->pwdfd:AT_FDCWD), + dir, + xattr); + if(newdirfd >=0) + { + /* chdir for directories on HSM/tapeworms may take minutes */ + EINTR_REPEAT((rval=fchdir(newdirfd)) < 0); + if(rval >= 0) + { + if(shp->pwdfd >= 0) + sh_close(shp->pwdfd); + shp->pwdfd=newdirfd; + goto success; + } + } +#ifndef O_SEARCH + else + { + rval=chdir(dir); + if(rval >=0) + { + if(shp->pwdfd >= 0) + { + sh_close(shp->pwdfd); + shp->pwdfd=-1; + } + } + } +#endif + } if(rval<0) { if(saverrno) diff -r -u original/src/cmd/ksh93/data/builtins.c build_xattr/src/cmd/ksh93/data/builtins.c --- src/cmd/ksh93/data/builtins.c 2012-06-19 10:02:12.000000000 +0200 +++ src/cmd/ksh93/data/builtins.c 2012-07-11 03:24:38.326384665 +0200 @@ -440,7 +440,7 @@ ; const char sh_optcd[] = -"[-1c?\n@(#)$Id: cd (AT&T Research) 1999-06-05 $\n]" +"[-1c?\n@(#)$Id: cd (AT&T Research) 2012-06-18 $\n]" USAGE_LICENSE "[+NAME?cd - change working directory ]" "[+DESCRIPTION?\bcd\b changes the current working directory of the " @@ -481,6 +481,10 @@ "[P?The present working directory is first converted to an absolute pathname " "that does not contain symbolic link components and symbolic name " "components are expanded in the resulting directory name.]" +#ifdef O_XATTR +"[@?Change into the hidden attribute directory of a file or directory. The " + "\bCDPATH\b environment variable is being ignored.]" +#endif "\n" "\n[directory]\n" "old new\n" diff -r -u original/src/cmd/ksh93/include/defs.h build_xattr/src/cmd/ksh93/include/defs.h --- src/cmd/ksh93/include/defs.h 2012-06-25 20:47:47.000000000 +0200 +++ src/cmd/ksh93/include/defs.h 2012-07-11 03:24:38.327090868 +0200 @@ -245,6 +245,7 @@ Sfio_t **sftable; \ unsigned char *fdstatus; \ const char *pwd; \ + int pwdfd; \ void *jmpbuffer; \ void *mktype; \ Sfio_t *strbuf; \ diff -r -u original/src/cmd/ksh93/sh/init.c build_xattr/src/cmd/ksh93/sh/init.c --- src/cmd/ksh93/sh/init.c 2012-05-11 19:19:10.000000000 +0200 +++ src/cmd/ksh93/sh/init.c 2012-07-11 03:24:38.328286488 +0200 @@ -1365,6 +1365,12 @@ } } sh_ioinit(shp); + shp->pwdfd = sh_diropenat(shp, AT_FDCWD, ".", 0); +#ifdef O_SEARCH + /* This should _never_ happen, guranteed by design and goat sacrifice */ + if(shp->pwdfd < 0) + errormsg(SH_DICT,ERROR_exit(1), "Can't obtain directory fd."); +#endif /* initialize signal handling */ sh_siginit(shp); stakinstall(NIL(Stak_t*),nospace); diff -r -u original/src/cmd/ksh93/sh/subshell.c build_xattr/src/cmd/ksh93/sh/subshell.c --- src/cmd/ksh93/sh/subshell.c 2012-05-30 22:52:44.000000000 +0200 +++ src/cmd/ksh93/sh/subshell.c 2012-07-11 03:43:47.751158180 +0200 @@ -75,6 +75,7 @@ Sfio_t* saveout;/*saved standard output */ char *pwd; /* present working directory */ const char *shpwd; /* saved pointer to sh.pwd */ + int shpwdfd; void *jobs; /* save job info */ mode_t mask; /* saved umask */ short tmpfd; /* saved tmp file descriptor */ @@ -520,9 +521,22 @@ shp->subshare = comsub==2 || (comsub==1 && sh_isoption(SH_SUBSHARE)); if(comsub) shp->comsub = comsub; + + sp->shpwdfd=-1; if(!comsub || !shp->subshare) { sp->shpwd = shp->pwd; + sp->shpwdfd=((shp->pwdfd >= 0) +#ifdef AT_FDCWD + &&(shp->pwdfd!=AT_FDCWD) +#endif + )?sh_fcntl(shp->pwdfd, F_DUPFD, 10):-1; + if(sp->shpwdfd>=0) + sh_fcntl(sp->shpwdfd, F_SETFD, FD_CLOEXEC); +#ifdef O_SEARCH + else + errormsg(SH_DICT,ERROR_exit(1), "Can't obtain directory fd."); +#endif sp->pwd = (shp->pwd?strdup(shp->pwd):0); sp->mask = shp->mask; sh_stats(STAT_SUBSHELL); @@ -699,7 +713,11 @@ Namval_t *pwdnod = sh_scoped(shp,PWDNOD); if(shp->pwd) { - chdir(shp->pwd=sp->pwd); + shp->pwd=sp->pwd; +#ifndef O_SEARCH + if (sp->shpwdfd < 0) + chdir(shp->pwd); +#endif path_newdir(shp,shp->pathlist); } if(nv_isattr(pwdnod,NV_NOFREE)) @@ -724,6 +742,15 @@ shp->cpipe[1] = sp->cpipe; shp->coutpipe = sp->coutpipe; } + if(sp->shpwdfd >=0) + { + if(shp->pwdfd >=0) + sh_close(shp->pwdfd); + shp->pwdfd=sp->shpwdfd; + /* chdir for directories on HSM/tapeworms may take minutes */ + while((fchdir(shp->pwdfd) < 0) && (errno == EINTR)) + errno=0; + } shp->subshare = sp->subshare; shp->comsub = sp->comsub; shp->subdup = sp->subdup; diff -r -u original/src/cmd/ksh93/sh/xec.c build_xattr/src/cmd/ksh93/sh/xec.c --- src/cmd/ksh93/sh/xec.c 2012-06-26 18:03:05.000000000 +0200 +++ src/cmd/ksh93/sh/xec.c 2012-07-11 03:24:38.331870027 +0200 @@ -1346,7 +1346,7 @@ { if(!shp->pwd) path_pwd(shp,0); - if(shp->pwd) + else if(shp->pwd || (shp->pwdfd >= 0)) stat(".",&statb); sfsync(NULL); share = sfset(sfstdin,SF_SHARE,0); @@ -1426,13 +1426,26 @@ sh_offstate(SH_NOFORK); if(!(nv_isattr(np,BLT_ENV))) { - if(shp->pwd) + if(shp->pwd || (shp->pwdfd >= 0)) { struct stat stata; stat(".",&stata); /* restore directory changed */ if(statb.st_ino!=stata.st_ino || statb.st_dev!=stata.st_dev) - chdir(shp->pwd); + { + if(shp->pwdfd >= 0) + { + /* chdir for directories on HSM/tapeworms may take minutes */ + while((fchdir(shp->pwdfd) < 0) && (errno == EINTR)) + errno=0; + } + else + { + /* chdir for directories on HSM/tapeworms may take minutes */ + while((chdir(shp->pwd) < 0) && (errno == EINTR)) + errno=0; + } + } } sh_offstate(SH_STOPOK); if(share&SF_SHARE) Only in original/src/cmd/ksh93/tests: arrays.sh
_______________________________________________ ast-developers mailing list ast-developers@research.att.com https://mailman.research.att.com/mailman/listinfo/ast-developers