linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Slaby <jirislaby@gmail.com>
To: Jiri Slaby <jslaby@suse.cz>
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
	Sukadev Bhattiprolu <sukadev@us.ibm.com>,
	Alan Cox <alan@redhat.com>
Subject: Re: [PATCH 4/4] TTY: call tty_driver_lookup_tty unconditionally
Date: Wed, 12 Oct 2011 22:59:54 +0200	[thread overview]
Message-ID: <4E95FFCA.2090604@gmail.com> (raw)
In-Reply-To: <1318411965-23917-4-git-send-email-jslaby@suse.cz>

On 10/12/2011 11:32 AM, Jiri Slaby wrote:
> Commit 4a2b5fddd5 (Move tty lookup/reopen to caller) made the call to
> tty_driver_lookup_tty conditional in tty_open. It doesn't look like it
> was an intention. Or if it was, it was not documented in the changelog
> and the code now looks weird. For example there would be no need to
> remember the tty driver and tty index. Further the condition depends
> on a tty which we drop a reference of already.
> 
> If I'm looking correctly, this should not matter thanks to the locking
> currently done there. Thus, tty_driver->ttys[idx] cannot change under
> our hands. But anyway, it makes sense to change that to the old
> behaviour.

Well, this doesn't work for ptys. devpts lookup code expects an inode to
be one of devpts filesystem (/dev/pts/*), not something on tmpfs like
/dev/tty. So it looks like the change was intentional, but very
undocumented and leaving there some unused code.

Now, I don't know what to do with the tty being around with a dropped
reference. Maybe nothing in the presence of the BTM which I *think*
prevents tty to go now.

Also I have just found that having ptm_unix98_lookup is pointless. For
master, we have ptmx_open which doesn't call lookup. And if it did, it
would trigger BUG_ON in devpts_get_tty called from ptm_unix98_lookup.

thanks,
-- 
js

  reply	other threads:[~2011-10-12 21:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-04 20:05 NULL dereference in tty_open() Dan Carpenter
2011-10-05 14:22 ` NULL dereference in tty_open() [and other bugs there] Jiri Slaby
2011-10-12  9:32   ` [PATCH 1/4] TTY: drop driver reference in tty_open fail path Jiri Slaby
2011-10-12  9:32     ` [PATCH 2/4] TTY: make tty_add_file non-failing Jiri Slaby
2011-10-12  9:32     ` [PATCH 3/4] TTY: pty, release tty in all ptmx_open fail paths Jiri Slaby
2011-10-12 13:23       ` Arnd Bergmann
2011-10-12  9:32     ` [PATCH 4/4] TTY: call tty_driver_lookup_tty unconditionally Jiri Slaby
2011-10-12 20:59       ` Jiri Slaby [this message]
2011-10-16 19:20         ` Sukadev Bhattiprolu
2011-10-16 19:37           ` Jiri Slaby
2011-10-16 18:28     ` [PATCH 1/4] TTY: drop driver reference in tty_open fail path Sukadev Bhattiprolu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E95FFCA.2090604@gmail.com \
    --to=jirislaby@gmail.com \
    --cc=alan@redhat.com \
    --cc=gregkh@suse.de \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sukadev@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).