Re: [PATCH] Cygwin: exec: check execute bit prior to evaluating script
On Aug 6 12:09, Ken Brown wrote:
> On 8/6/2019 4:53 AM, Corinna Vinschen wrote:
> > From: Corinna Vinschen
> >
> > When the exec family of functions is called for a script-like
> > file, the av::setup function handles the exec[vl]p case as
> > well. The execve case for files not starting with a she-bang
> > is handled first by returning ENOEXEC. Only after that, the
> > file's executability is checked.
> >
> > This leads to the problem that ENOEXEC is returned for non-executable
> > files as well. A calling shell interprets this as a file it should try
> > to run as script. This is not desired for non-executable files.
> >
> > Fix this problem by checking the file for executability first. Only
> > after that, follow the other potential code paths.
> > ---
> > winsup/cygwin/spawn.cc | 12 ++--
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> > index 7f7af4449da1..d95772802f8f 100644
> > --- a/winsup/cygwin/spawn.cc
> > +++ b/winsup/cygwin/spawn.cc
> > @@ -1172,6 +1172,12 @@ av::setup (const char *prog_arg, path_conv&
> > real_path, const char *ext,
> > }
> > UnmapViewOfFile (buf);
> > just_shell:
> > + /* Check if script is executable. Otherwise we start non-executable
> > + scripts successfully, which is incorrect behaviour. */
> > + if (real_path.has_acls ()
> > + && check_file_access (real_path, X_OK, true) < 0)
> > + return -1;/* errno is already set. */
> > +
> > if (!pgm)
> > {
> > if (!p_type_exec)
> > @@ -1188,12 +1194,6 @@ av::setup (const char *prog_arg, path_conv&
> > real_path, const char *ext,
> > arg1 = NULL;
> > }
> >
> > - /* Check if script is executable. Otherwise we start non-executable
> > - scripts successfully, which is incorrect behaviour. */
> > - if (real_path.has_acls ()
> > - && check_file_access (real_path, X_OK, true) < 0)
> > - return -1;/* errno is already set. */
> > -
> > /* Replace argv[0] with the full path to the script if this is the
> >first time through the loop. */
> > replace0_maybe (prog_arg);
>
> LGTM, and I've confirmed that it fixes the problem reported in
> http://www.cygwin.org/ml/cygwin/2019-08/msg00054.html.
>
> Ken
Thanks! Pushed.
Corinna
--
Corinna Vinschen
Cygwin Maintainer
signature.asc
Description: PGP signature
Re: [PATCH] Cygwin: exec: check execute bit prior to evaluating script
On 8/6/2019 4:53 AM, Corinna Vinschen wrote:
> From: Corinna Vinschen
>
> When the exec family of functions is called for a script-like
> file, the av::setup function handles the exec[vl]p case as
> well. The execve case for files not starting with a she-bang
> is handled first by returning ENOEXEC. Only after that, the
> file's executability is checked.
>
> This leads to the problem that ENOEXEC is returned for non-executable
> files as well. A calling shell interprets this as a file it should try
> to run as script. This is not desired for non-executable files.
>
> Fix this problem by checking the file for executability first. Only
> after that, follow the other potential code paths.
> ---
> winsup/cygwin/spawn.cc | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> index 7f7af4449da1..d95772802f8f 100644
> --- a/winsup/cygwin/spawn.cc
> +++ b/winsup/cygwin/spawn.cc
> @@ -1172,6 +1172,12 @@ av::setup (const char *prog_arg, path_conv& real_path,
> const char *ext,
> }
> UnmapViewOfFile (buf);
> just_shell:
> + /* Check if script is executable. Otherwise we start non-executable
> +scripts successfully, which is incorrect behaviour. */
> + if (real_path.has_acls ()
> + && check_file_access (real_path, X_OK, true) < 0)
> + return -1;/* errno is already set. */
> +
> if (!pgm)
> {
> if (!p_type_exec)
> @@ -1188,12 +1194,6 @@ av::setup (const char *prog_arg, path_conv& real_path,
> const char *ext,
> arg1 = NULL;
> }
>
> - /* Check if script is executable. Otherwise we start non-executable
> -scripts successfully, which is incorrect behaviour. */
> - if (real_path.has_acls ()
> - && check_file_access (real_path, X_OK, true) < 0)
> - return -1;/* errno is already set. */
> -
> /* Replace argv[0] with the full path to the script if this is the
> first time through the loop. */
> replace0_maybe (prog_arg);
LGTM, and I've confirmed that it fixes the problem reported in
http://www.cygwin.org/ml/cygwin/2019-08/msg00054.html.
Ken
[PATCH] Cygwin: exec: check execute bit prior to evaluating script
From: Corinna Vinschen
When the exec family of functions is called for a script-like
file, the av::setup function handles the exec[vl]p case as
well. The execve case for files not starting with a she-bang
is handled first by returning ENOEXEC. Only after that, the
file's executability is checked.
This leads to the problem that ENOEXEC is returned for non-executable
files as well. A calling shell interprets this as a file it should try
to run as script. This is not desired for non-executable files.
Fix this problem by checking the file for executability first. Only
after that, follow the other potential code paths.
---
winsup/cygwin/spawn.cc | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 7f7af4449da1..d95772802f8f 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -1172,6 +1172,12 @@ av::setup (const char *prog_arg, path_conv& real_path,
const char *ext,
}
UnmapViewOfFile (buf);
just_shell:
+ /* Check if script is executable. Otherwise we start non-executable
+ scripts successfully, which is incorrect behaviour. */
+ if (real_path.has_acls ()
+ && check_file_access (real_path, X_OK, true) < 0)
+ return -1;/* errno is already set. */
+
if (!pgm)
{
if (!p_type_exec)
@@ -1188,12 +1194,6 @@ av::setup (const char *prog_arg, path_conv& real_path,
const char *ext,
arg1 = NULL;
}
- /* Check if script is executable. Otherwise we start non-executable
- scripts successfully, which is incorrect behaviour. */
- if (real_path.has_acls ()
- && check_file_access (real_path, X_OK, true) < 0)
- return -1;/* errno is already set. */
-
/* Replace argv[0] with the full path to the script if this is the
first time through the loop. */
replace0_maybe (prog_arg);
--
2.20.1
