On Mon, Jul 9, 2012 at 6:16 PM, Roland Mainz <[email protected]> wrote:
> On Mon, Jul 9, 2012 at 12:59 PM, Lionel Cons
> <[email protected]> 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) [email protected]
\__\/\/__/ 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
[email protected]
https://mailman.research.att.com/mailman/listinfo/ast-developers