Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 28 Feb 2016 08:08:05 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Changes needed for cygwin (-fork tty problems)

Hi Jim,

Thank you for bringing this in here.

On Sat, Feb 27, 2016 at 10:44:36PM -0600, jfoug wrote:
> This has been a problem for me, since we have gotten cygwin working with 
> -fork.  When complete (with abort with 'q' or the session completes), 
> the cygwin shell is screwed up (tty is in non-echo mode).   I have lived 
> with this, but recently there are other user posts, where I see them 
> doing 'stty sane' calls after a -fork run (cygwin).
> 
> So I made a change to john.c 
> https://github.com/magnumripper/JohnTheRipper/commit/81e0cce6303cbea87420ccd70151f0779857b564 
> ,  so that when john exited, it did a system 'stty sane' call to clean 
> things up..   This seemed to work fine.  However, magnum question why this 
> was done, and we had some discussion.  Well to put things simply, we found 
> the 'real' bug on why cygwin was left in a tty non-echo mode.   It was 
> fixed here: 
> https://github.com/magnumripper/JohnTheRipper/commit/5aebb0bc4f94e778be5f95599cd47313604a64a9 
> reverting the other 'hack',, and fixing the real problem.  This fix should 
> be pushed down to john core, since this was code code in tty.   It may be 
> that at some time, the getpid() or some other call in cygwin was not 
> working. Well, if this code IS done in cygwin, then when -fork=x runs exit, 
> the tty is left in a sane state.   This code should really be looked at, 
> and likely the #ifndef __CYGWIN__ define should be removed.

This is indeed a better fix, but not necessarily the right fix.  I think
the problem isn't even limited to Cygwin, it's just hidden by that code.

I think the real problem is that we call tty_init() after fork()'ing,
and thus per process, rather than just once in the parent process.
After all, they're supposed to share the same tty, which we only need to
initialize once.

The fork()'ing is at the end of john_load(), which is the last thing
called by john_init().  The tty_init() is in john_run(), which is called
after john_init().  Maybe we need to move the fork()'ing to right after
where tty_init() is.  However, there's some other initialization code
right above it, some of which we might (or might not?) want to run per
process - e.g., the self-test.  This needs some thinking and testing.

For now, Jim's tty.c fix is fine.

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.