Roland Mainz
2013-07-22 18:12:45 UTC
Hi!
----
Attached (as "astksh20130719_cd_fix001.diff.txt") is a patch to fix
the following issues with the cd(1) buildin:
1. error handling in cd(1) is fixed
2. cd(1) no longer uses |stat()| to validate whether the opened file
name is a directory if |O_DIRECTORY| is set. This saves a syscall.
3. cd(1) now opens directories first without |O_SEARCH| and if this
fails tries again with |O_SEARCH|. This is mainly to ensure the error
messages are the same as for other shells. This could should not be
moved into libast since this backwards-compatibility support is only
for the cd(1) builtin. Needed for Solaris/Illumos to boot correctly...
;-/
4. ksh93 now closes the directory fd on exit... a) for correctness and
b) to circumvent an annoying issue which triggers a crash in
"valgrind" ... which disrupts testing
5. dead code removal in src/cmd/ksh93/sh/init.c
The patch should be pretty much safe to use... it was previously part
of the "ast_ksh_20130628/astksh20130628_solaris_fixes005.diff.txt"
patch cluster for ast-ksh.2013-06-28 ...
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 3992797
(;O/ \/ \O;)
-------------- next part --------------
diff -r -u original/src/cmd/ksh93/bltins/cd_pwd.c build_i386_64bit_debug_cdfixes/src/cmd/ksh93/bltins/cd_pwd.c
--- src/cmd/ksh93/bltins/cd_pwd.c 2013-07-09 23:19:09.000000000 +0200
+++ src/cmd/ksh93/bltins/cd_pwd.c 2013-07-22 19:46:49.515699373 +0200
@@ -55,9 +55,17 @@
*/
int sh_diropenat(Shell_t *shp, int dir, const char *path, bool xattr)
{
+#ifdef O_DIRECTORY
+#define O_directory (O_DIRECTORY)
+#else
+#define O_directory (0)
+#endif
+
int fd,shfd;
- int savederrno=errno;
+ int savederrno;
+#ifndef O_DIRECTORY
struct stat fs;
+#endif
#ifndef O_XATTR
NOT_USED(xattr);
#endif
@@ -70,21 +78,56 @@
if((apfd = openat(dir, path, O_RDONLY|O_NONBLOCK|O_cloexec))>=0)
{
fd = openat(apfd, e_dot, O_XATTR|O_cloexec);
+ savederrno=errno;
close(apfd);
+ errno=savederrno;
+ }
+ else
+ {
+ return(-1);
}
}
else
#endif
- fd = openat(dir, path, O_SEARCH|O_NONBLOCK|O_cloexec);
+ {
+ /*
+ * 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.
+ */
+
+ fd = openat(dir, path, O_directory|O_NONBLOCK|O_cloexec);
+ if ((fd < 0) && (errno == EACCES))
+ {
+ fd = openat(dir, path, O_SEARCH|O_directory|O_NONBLOCK|O_cloexec);
+ }
+ }
if(fd < 0)
- return fd;
+ {
+ return(fd);
+ }
+
+#ifndef O_DIRECTORY
if (!fstat(fd, &fs) && !S_ISDIR(fs.st_mode))
{
close(fd);
errno = ENOTDIR;
- return -1;
+ return(-1);
}
+#endif
/* Move fd to a number > 10 and register the fd number with the shell */
shfd = sh_fcntl(fd, F_dupfd_cloexec, 10);
@@ -329,11 +372,7 @@
if(shp->pwdfd >= 0)
{
sh_close(shp->pwdfd);
-#ifdef AT_FDCWD
shp->pwdfd = AT_FDCWD;
-#else
- shp->pwdfd = -1;
-#endif
}
}
}
diff -r -u original/src/cmd/ksh93/include/shell.h build_i386_64bit_debug_cdfixes/src/cmd/ksh93/include/shell.h
--- src/cmd/ksh93/include/shell.h 2013-06-27 23:43:06.000000000 +0200
+++ src/cmd/ksh93/include/shell.h 2013-07-22 19:40:37.403110669 +0200
@@ -155,7 +155,7 @@
char shcomp; /* set when runing shcomp */
short subshell; /* set for virtual subshell */
Stk_t *stk; /* stack poiter */
- int pwdfd; /* file descriptor for pwd */
+ int pwdfd; /* file descriptor for pwd (must be from sh_diropenat()/sh_fcntl()!) */
#ifdef _SH_PRIVATE
_SH_PRIVATE
#endif /* _SH_PRIVATE */
diff -r -u original/src/cmd/ksh93/sh/fault.c build_i386_64bit_debug_cdfixes/src/cmd/ksh93/sh/fault.c
--- src/cmd/ksh93/sh/fault.c 2013-07-11 17:44:57.000000000 +0200
+++ src/cmd/ksh93/sh/fault.c 2013-07-22 19:42:41.605655853 +0200
@@ -688,6 +688,10 @@
if(sh_isoption(shp,SH_NOEXEC))
kiaclose((Lex_t*)shp->lex_context);
#endif /* SHOPT_KIA */
+
+ if (shp->pwdfd >= 0)
+ sh_close(shp->pwdfd);
+
exit(savxit&SH_EXITMASK);
}
diff -r -u original/src/cmd/ksh93/sh/init.c build_i386_64bit_debug_cdfixes/src/cmd/ksh93/sh/init.c
--- src/cmd/ksh93/sh/init.c 2013-07-18 19:22:10.000000000 +0200
+++ src/cmd/ksh93/sh/init.c 2013-07-22 19:41:38.112265548 +0200
@@ -1550,16 +1550,11 @@
}
}
sh_ioinit(shp);
-#ifdef AT_FDCWD
- shp->pwdfd = sh_diropenat(shp, AT_FDCWD, e_dot, false);
-#else
- /* Systems without AT_FDCWD/openat() do not use the |dir| argument */
- shp->pwdfd = sh_diropenat(shp, -1, e_dot, false);
-#endif
+ shp->pwdfd = sh_diropenat(shp, AT_FDCWD, e_dot, false);
#ifdef O_SEARCH
- /* This should _never_ happen, guranteed by design and goat sacrifice */
- if(shp->pwdfd < 0)
- errormsg(SH_DICT,ERROR_system(1), "Can't obtain directory fd.");
+ /* This should _never_ happen, guranteed by design and goat sacrifice */
+ if(shp->pwdfd < 0)
+ errormsg(SH_DICT,ERROR_system(1), "Can't obtain directory fd.");
#endif
/* initialize signal handling */
----
Attached (as "astksh20130719_cd_fix001.diff.txt") is a patch to fix
the following issues with the cd(1) buildin:
1. error handling in cd(1) is fixed
2. cd(1) no longer uses |stat()| to validate whether the opened file
name is a directory if |O_DIRECTORY| is set. This saves a syscall.
3. cd(1) now opens directories first without |O_SEARCH| and if this
fails tries again with |O_SEARCH|. This is mainly to ensure the error
messages are the same as for other shells. This could should not be
moved into libast since this backwards-compatibility support is only
for the cd(1) builtin. Needed for Solaris/Illumos to boot correctly...
;-/
4. ksh93 now closes the directory fd on exit... a) for correctness and
b) to circumvent an annoying issue which triggers a crash in
"valgrind" ... which disrupts testing
5. dead code removal in src/cmd/ksh93/sh/init.c
The patch should be pretty much safe to use... it was previously part
of the "ast_ksh_20130628/astksh20130628_solaris_fixes005.diff.txt"
patch cluster for ast-ksh.2013-06-28 ...
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 3992797
(;O/ \/ \O;)
-------------- next part --------------
diff -r -u original/src/cmd/ksh93/bltins/cd_pwd.c build_i386_64bit_debug_cdfixes/src/cmd/ksh93/bltins/cd_pwd.c
--- src/cmd/ksh93/bltins/cd_pwd.c 2013-07-09 23:19:09.000000000 +0200
+++ src/cmd/ksh93/bltins/cd_pwd.c 2013-07-22 19:46:49.515699373 +0200
@@ -55,9 +55,17 @@
*/
int sh_diropenat(Shell_t *shp, int dir, const char *path, bool xattr)
{
+#ifdef O_DIRECTORY
+#define O_directory (O_DIRECTORY)
+#else
+#define O_directory (0)
+#endif
+
int fd,shfd;
- int savederrno=errno;
+ int savederrno;
+#ifndef O_DIRECTORY
struct stat fs;
+#endif
#ifndef O_XATTR
NOT_USED(xattr);
#endif
@@ -70,21 +78,56 @@
if((apfd = openat(dir, path, O_RDONLY|O_NONBLOCK|O_cloexec))>=0)
{
fd = openat(apfd, e_dot, O_XATTR|O_cloexec);
+ savederrno=errno;
close(apfd);
+ errno=savederrno;
+ }
+ else
+ {
+ return(-1);
}
}
else
#endif
- fd = openat(dir, path, O_SEARCH|O_NONBLOCK|O_cloexec);
+ {
+ /*
+ * 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.
+ */
+
+ fd = openat(dir, path, O_directory|O_NONBLOCK|O_cloexec);
+ if ((fd < 0) && (errno == EACCES))
+ {
+ fd = openat(dir, path, O_SEARCH|O_directory|O_NONBLOCK|O_cloexec);
+ }
+ }
if(fd < 0)
- return fd;
+ {
+ return(fd);
+ }
+
+#ifndef O_DIRECTORY
if (!fstat(fd, &fs) && !S_ISDIR(fs.st_mode))
{
close(fd);
errno = ENOTDIR;
- return -1;
+ return(-1);
}
+#endif
/* Move fd to a number > 10 and register the fd number with the shell */
shfd = sh_fcntl(fd, F_dupfd_cloexec, 10);
@@ -329,11 +372,7 @@
if(shp->pwdfd >= 0)
{
sh_close(shp->pwdfd);
-#ifdef AT_FDCWD
shp->pwdfd = AT_FDCWD;
-#else
- shp->pwdfd = -1;
-#endif
}
}
}
diff -r -u original/src/cmd/ksh93/include/shell.h build_i386_64bit_debug_cdfixes/src/cmd/ksh93/include/shell.h
--- src/cmd/ksh93/include/shell.h 2013-06-27 23:43:06.000000000 +0200
+++ src/cmd/ksh93/include/shell.h 2013-07-22 19:40:37.403110669 +0200
@@ -155,7 +155,7 @@
char shcomp; /* set when runing shcomp */
short subshell; /* set for virtual subshell */
Stk_t *stk; /* stack poiter */
- int pwdfd; /* file descriptor for pwd */
+ int pwdfd; /* file descriptor for pwd (must be from sh_diropenat()/sh_fcntl()!) */
#ifdef _SH_PRIVATE
_SH_PRIVATE
#endif /* _SH_PRIVATE */
diff -r -u original/src/cmd/ksh93/sh/fault.c build_i386_64bit_debug_cdfixes/src/cmd/ksh93/sh/fault.c
--- src/cmd/ksh93/sh/fault.c 2013-07-11 17:44:57.000000000 +0200
+++ src/cmd/ksh93/sh/fault.c 2013-07-22 19:42:41.605655853 +0200
@@ -688,6 +688,10 @@
if(sh_isoption(shp,SH_NOEXEC))
kiaclose((Lex_t*)shp->lex_context);
#endif /* SHOPT_KIA */
+
+ if (shp->pwdfd >= 0)
+ sh_close(shp->pwdfd);
+
exit(savxit&SH_EXITMASK);
}
diff -r -u original/src/cmd/ksh93/sh/init.c build_i386_64bit_debug_cdfixes/src/cmd/ksh93/sh/init.c
--- src/cmd/ksh93/sh/init.c 2013-07-18 19:22:10.000000000 +0200
+++ src/cmd/ksh93/sh/init.c 2013-07-22 19:41:38.112265548 +0200
@@ -1550,16 +1550,11 @@
}
}
sh_ioinit(shp);
-#ifdef AT_FDCWD
- shp->pwdfd = sh_diropenat(shp, AT_FDCWD, e_dot, false);
-#else
- /* Systems without AT_FDCWD/openat() do not use the |dir| argument */
- shp->pwdfd = sh_diropenat(shp, -1, e_dot, false);
-#endif
+ shp->pwdfd = sh_diropenat(shp, AT_FDCWD, e_dot, false);
#ifdef O_SEARCH
- /* This should _never_ happen, guranteed by design and goat sacrifice */
- if(shp->pwdfd < 0)
- errormsg(SH_DICT,ERROR_system(1), "Can't obtain directory fd.");
+ /* This should _never_ happen, guranteed by design and goat sacrifice */
+ if(shp->pwdfd < 0)
+ errormsg(SH_DICT,ERROR_system(1), "Can't obtain directory fd.");
#endif
/* initialize signal handling */