Date: Mon, 10 Jul 2017 12:55:44 +0100
From: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
To: Okash Khawaja <okash.khawaja@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Samuel Thibault <samuel.thibault@ens-lyon.org>,
        linux-kernel@vger.kernel.org, William Hubbs <w.d.hubbs@gmail.com>,
        Chris Brannon <chris@the-brannons.com>, Kirk Reiser <kirk@reisers.ca>,
        speakup@linux-speakup.org, devel@driverdev.osuosl.org
Subject: Re: [patch] staging: speakup: safely close tty
Message-ID: <20170710125544.2aa4d12a@alans-desktop>
In-Reply-To: <20170707191301.GA3798@sanghar>
References: <20170707191301.GA3798@sanghar>
Organization: Intel Corporation
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 955
Lines: 23

On Fri, 7 Jul 2017 20:13:01 +0100
Okash Khawaja <okash.khawaja@gmail.com> wrote:

> Speakup opens tty using tty_open_by_driver. When closing, it calls
> tty_ldisc_release but doesn't close and remove the tty itself. As a
> result, that tty cannot then be opened from user space. This patch calls
> tty_release_struct which ensures that tty is safely removed and freed
> up. It also calls tty_ldisc_release, so speakup doesn't need to call it.
> 
> This patch also unregisters N_SPEAKUP. It is registered when a speakup
> module is loaded.

What happens if after you register it someone assigns that ldisc to
another tty as well ?

You should register the ldisc when the relevant module is initialized and
release it only when the module is unloaded. That way the module ref
counts will handle cases where someone uses the ldisc with something else.

I'd also btw strongly recommend putting the ldisc and the speakup tty
driver as different modules.

Alan