* [PATCH] Fix `Bad address` when running `cmd /c [...]`
@ 2022-10-21 21:37 Johannes Schindelin
2022-10-22 1:36 ` Takashi Yano
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Schindelin @ 2022-10-21 21:37 UTC (permalink / raw)
To: cygwin-patches
In 2b4f986e49 (Cygwin: pty: Treat *.bat and *.cmd as a non-cygwin
console app., 2022-07-31), we introduced a bug fix that specifically
looks for a suffix of the command's file name.
However, that file name might be set to `NULL`, namely when
`null_app_name == true`, which is the case when we detected a
command-line `cmd /c [...]`.
But the commit mentioned above did not account for that possibility,
instead assuming that it always has to check the file name for a `.bat`
or `.cmd` suffix. As a consequence, `cmd /c [...]` invocations are
completely broken in v3.3.6, resulting in a `Bad address` error.
Let's guard the code properly so that it does not try to look at the
file name suffix of `(WCHAR *)NULL`.
This fixes https://github.com/msys2/msys2-runtime/issues/108
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/msys2-runtime/releases/tag/fix-bad-address-in-cmd-invocation-v1
Fetch-It-Via: git fetch https://github.com/dscho/msys2-runtime fix-bad-address-in-cmd-invocation-v1
winsup/cygwin/spawn.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index d9d7716519..d4e7f10e2b 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -209,7 +209,7 @@ is_console_app (WCHAR *filename)
char *p = (char *) memmem (buf, n, "PE\0\0", 4);
if (p && p + id_offset < buf + n)
return p[id_offset] == '\003'; /* 02: GUI, 03: console */
- else
+ else if (filename)
{
wchar_t *e = wcsrchr (filename, L'.');
if (e && (wcscasecmp (e, L".bat") == 0 || wcscasecmp (e, L".cmd") == 0))
base-commit: 1ca46b22d6edb3d469b51475e8b096d86deaac67
--
2.38.1.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix `Bad address` when running `cmd /c [...]`
2022-10-21 21:37 [PATCH] Fix `Bad address` when running `cmd /c [...]` Johannes Schindelin
@ 2022-10-22 1:36 ` Takashi Yano
2022-10-22 1:54 ` Takashi Yano
0 siblings, 1 reply; 7+ messages in thread
From: Takashi Yano @ 2022-10-22 1:36 UTC (permalink / raw)
To: cygwin-patches; +Cc: Johannes Schindelin
On Fri, 21 Oct 2022 23:37:35 +0200 (CEST)
Johannes Schindelin wrote:
> In 2b4f986e49 (Cygwin: pty: Treat *.bat and *.cmd as a non-cygwin
> console app., 2022-07-31), we introduced a bug fix that specifically
> looks for a suffix of the command's file name.
>
> However, that file name might be set to `NULL`, namely when
> `null_app_name == true`, which is the case when we detected a
> command-line `cmd /c [...]`.
It seems that this is msys2 specific issue.
I also noticed that
cmd //c 'echo AAA' instead of cmd /c 'echo AAA' works in msys2.
In cygwin, filename is
C:\WINDOWS\system32\cmd.exe
for cmd /c 'echo AAA'.
Why the filename can be NULL in msys2 in the case of cmd /c 'echo AAA'?
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix `Bad address` when running `cmd /c [...]`
2022-10-22 1:36 ` Takashi Yano
@ 2022-10-22 1:54 ` Takashi Yano
2022-10-22 5:37 ` Takashi Yano
0 siblings, 1 reply; 7+ messages in thread
From: Takashi Yano @ 2022-10-22 1:54 UTC (permalink / raw)
To: cygwin-patches; +Cc: Johannes Schindelin
On Sat, 22 Oct 2022 10:36:39 +0900
Takashi Yano wrote:
> On Fri, 21 Oct 2022 23:37:35 +0200 (CEST)
> Johannes Schindelin wrote:
> > In 2b4f986e49 (Cygwin: pty: Treat *.bat and *.cmd as a non-cygwin
> > console app., 2022-07-31), we introduced a bug fix that specifically
> > looks for a suffix of the command's file name.
> >
> > However, that file name might be set to `NULL`, namely when
> > `null_app_name == true`, which is the case when we detected a
> > command-line `cmd /c [...]`.
>
> It seems that this is msys2 specific issue.
> I also noticed that
> cmd //c 'echo AAA' instead of cmd /c 'echo AAA' works in msys2.
>
> In cygwin, filename is
> C:\WINDOWS\system32\cmd.exe
> for cmd /c 'echo AAA'.
>
> Why the filename can be NULL in msys2 in the case of cmd /c 'echo AAA'?
I can reproduce the issue in cygwin with cmd.exe /c 'echo AAA'
instead of cmd /c 'echo AAA'.
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix `Bad address` when running `cmd /c [...]`
2022-10-22 1:54 ` Takashi Yano
@ 2022-10-22 5:37 ` Takashi Yano
2022-10-22 5:55 ` Johannes Schindelin
0 siblings, 1 reply; 7+ messages in thread
From: Takashi Yano @ 2022-10-22 5:37 UTC (permalink / raw)
To: cygwin-patches; +Cc: Johannes Schindelin
On Sat, 22 Oct 2022 10:54:06 +0900
Takashi Yano wrote:
> On Sat, 22 Oct 2022 10:36:39 +0900
> Takashi Yano wrote:
> > On Fri, 21 Oct 2022 23:37:35 +0200 (CEST)
> > Johannes Schindelin wrote:
> > > In 2b4f986e49 (Cygwin: pty: Treat *.bat and *.cmd as a non-cygwin
> > > console app., 2022-07-31), we introduced a bug fix that specifically
> > > looks for a suffix of the command's file name.
> > >
> > > However, that file name might be set to `NULL`, namely when
> > > `null_app_name == true`, which is the case when we detected a
> > > command-line `cmd /c [...]`.
> >
> > It seems that this is msys2 specific issue.
> > I also noticed that
> > cmd //c 'echo AAA' instead of cmd /c 'echo AAA' works in msys2.
> >
> > In cygwin, filename is
> > C:\WINDOWS\system32\cmd.exe
> > for cmd /c 'echo AAA'.
> >
> > Why the filename can be NULL in msys2 in the case of cmd /c 'echo AAA'?
>
> I can reproduce the issue in cygwin with cmd.exe /c 'echo AAA'
> instead of cmd /c 'echo AAA'.
I have just pushed a counter patch. Thanks for the report.
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix `Bad address` when running `cmd /c [...]`
2022-10-22 5:37 ` Takashi Yano
@ 2022-10-22 5:55 ` Johannes Schindelin
2022-10-22 6:07 ` Takashi Yano
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Schindelin @ 2022-10-22 5:55 UTC (permalink / raw)
To: Takashi Yano, cygwin-patches
On October 22, 2022 7:37:09 AM GMT+02:00, Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>On Sat, 22 Oct 2022 10:54:06 +0900
>Takashi Yano wrote:
>> On Sat, 22 Oct 2022 10:36:39 +0900
>> Takashi Yano wrote:
>> > On Fri, 21 Oct 2022 23:37:35 +0200 (CEST)
>> > Johannes Schindelin wrote:
>> > > In 2b4f986e49 (Cygwin: pty: Treat *.bat and *.cmd as a non-cygwin
>> > > console app., 2022-07-31), we introduced a bug fix that specifically
>> > > looks for a suffix of the command's file name.
>> > >
>> > > However, that file name might be set to `NULL`, namely when
>> > > `null_app_name == true`, which is the case when we detected a
>> > > command-line `cmd /c [...]`.
>> >
>> > It seems that this is msys2 specific issue.
>> > I also noticed that
>> > cmd //c 'echo AAA' instead of cmd /c 'echo AAA' works in msys2.
>> >
>> > In cygwin, filename is
>> > C:\WINDOWS\system32\cmd.exe
>> > for cmd /c 'echo AAA'.
>> >
>> > Why the filename can be NULL in msys2 in the case of cmd /c 'echo AAA'?
>>
>> I can reproduce the issue in cygwin with cmd.exe /c 'echo AAA'
>> instead of cmd /c 'echo AAA'.
>
>I have just pushed a counter patch. Thanks for the report.
>
Thanks, I guess?
It's not very nice to simply drop my work, and then not even link to your "counter".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix `Bad address` when running `cmd /c [...]`
2022-10-22 5:55 ` Johannes Schindelin
@ 2022-10-22 6:07 ` Takashi Yano
2022-10-23 20:48 ` Johannes Schindelin
0 siblings, 1 reply; 7+ messages in thread
From: Takashi Yano @ 2022-10-22 6:07 UTC (permalink / raw)
To: cygwin-patches; +Cc: Johannes Schindelin
On Sat, 22 Oct 2022 07:55:53 +0200
Johannes Schindelin wrote:
> It's not very nice to simply drop my work, and then not even link to your "counter".
I am sorry, however, your patch can be indirectly reached
via link https://github.com/msys2/msys2-runtime/issues/108
in my counter patch.
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix `Bad address` when running `cmd /c [...]`
2022-10-22 6:07 ` Takashi Yano
@ 2022-10-23 20:48 ` Johannes Schindelin
0 siblings, 0 replies; 7+ messages in thread
From: Johannes Schindelin @ 2022-10-23 20:48 UTC (permalink / raw)
To: Takashi Yano; +Cc: cygwin-patches
Hi Takashi,
On Sat, 22 Oct 2022, Takashi Yano wrote:
> On Sat, 22 Oct 2022 07:55:53 +0200
> Johannes Schindelin wrote:
> > It's not very nice to simply drop my work, and then not even link to your "counter".
>
> I am sorry, however, your patch can be indirectly reached
> via link https://github.com/msys2/msys2-runtime/issues/108
> in my counter patch.
You misunderstand what I wrote.
You answered the question "where is the link _from_ your 'counter' to the
original patch?"
But the concern was "you did not link _to_ your 'counter'".
For posterity, here is that link:
https://github.com/cygwin/cygwin/commit/08281cf4cca9593adcc3d30184322dc60fa1cd61
Ciao,
Johannes
P.S.: I quote "counter" because what you meant is actually a counter
proposal, not a counter (which is typically a variable that is used to
count things). It's one more irritating thing in a thread that was not
exactly short on those things. Quite frustrating.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-23 20:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-21 21:37 [PATCH] Fix `Bad address` when running `cmd /c [...]` Johannes Schindelin
2022-10-22 1:36 ` Takashi Yano
2022-10-22 1:54 ` Takashi Yano
2022-10-22 5:37 ` Takashi Yano
2022-10-22 5:55 ` Johannes Schindelin
2022-10-22 6:07 ` Takashi Yano
2022-10-23 20:48 ` Johannes Schindelin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).