close() handler of tty driver called for port that's not open?

close() handler of tty driver called for port that's not open?

[Grant Edwards]

Why does the tty layer call my tty driver's close() handler on a port
that's not open?

The scenario I've run into is that my open() handler is called for a
serial port that's not currently available, so I return -EBUSY.  The
libc open() call returns an error:

  [Errno 16] Device or resource busy: '/dev/ttySI0'

But then after the open has failed, my tty driver sees a call to
close() the device which failed to open.

Why does the tty layer try to close a device that isn't open?

Does the tty consider a device to be open even if the open() handler
returned an error?

The problem is that my driver keeps a usage count on the port so it
can clean up some internal stuff when the last user closes a port.

I only increment the usage count when an open is successful.  That
doesn't work very well when the tty layer calls close() on a device
regardless of whether or not it was able to open it first.

Does the tty layer guarantee that the number of close() calls will
match the number of open() calls regardless of whether the open()
calls succeed or not?

If so, has that always been the policy, or is calling close() on
non-open device a new feature?

-- 
Grant Edwards               grant.b.edwards        Yow! I want to mail a
                                  at               bronzed artichoke to
                              gmail.com            Nicaragua!

Re: close() handler of tty driver called for port that's not open?

[Alan Cox]

On Wed, 14 Sep 2011 16:21:03 +0000 (UTC)
Grant Edwards <grant.b.edwards@gmail.com> wrote:

> Why does the tty layer call my tty driver's close() handler on a port
> that's not open?

Because a lot of the tty drivers it makes sense to structure the cleanup
this way.
 
> Why does the tty layer try to close a device that isn't open?

Because it was originally designed that way

> Does the tty consider a device to be open even if the open() handler
> returned an error?

Define "open". The answer depends upon what you mean by "open"

> The problem is that my driver keeps a usage count on the port so it
> can clean up some internal stuff when the last user closes a port.

Use the tty_port helpers and it'll do that for you including the locking,
blocking, POSIX open blocking/waiting semantics, error handling, callbacks
and hangup. Trying to open code this stuff is a world of pain and the
locking rules are now much more your problem as we strip out the remains
of the big kernel lock.

> Does the tty layer guarantee that the number of close() calls will
> match the number of open() calls regardless of whether the open()
> calls succeed or not?
> 
> If so, has that always been the policy, or is calling close() on
> non-open device a new feature?

The tty layer has always worked that way. It's I think unique in that
behaviour in the kernel.

If you use the tty_port_open/hangup/close boilerplate then you can
provide shutdown and activate port methods. The guarantee they provide is
that they will be called in matching pairs, they will be called in the
correct places, they will not be called in parallel with one another and
they will properly handle hangup() events - where you shutdown on the
hangup but the close occurs afterwards.

In an ideal world eventually everything will be using the tty port
helpers and then we can make that the underlying (sane) tty interface and
clean up the rest.

Alan

Re: close() handler of tty driver called for port that's not open?

[Grant Edwards]

On 2011-09-14, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Grant Edwards <grant.b.edwards@gmail.com> wrote:
>
>> Why does the tty layer call my tty driver's close() handler on a port
>> that's not open?
>
> Because a lot of the tty drivers it makes sense to structure the
> cleanup this way.
>  
>> Why does the tty layer try to close a device that isn't open?
>
> Because it was originally designed that way

Interesting.  I only noticed it recently, but I think that may be
because of other changes made to the kernel.

>> Does the tty consider a device to be open even if the open() handler
>> returned an error?
>
> Define "open". The answer depends upon what you mean by "open"

In my mind, a port was only open after my open() handler returns
success.  I guess that's wrong, and I should increment my use counter
unconditionally each time my open() handler is called.

>> The problem is that my driver keeps a usage count on the port so it
>> can clean up some internal stuff when the last user closes a port.
>
> Use the tty_port helpers and it'll do that for you including the
> locking, blocking, POSIX open blocking/waiting semantics, error
> handling, callbacks and hangup. Trying to open code this stuff is a
> world of pain and the locking rules are now much more your problem as
> we strip out the remains of the big kernel lock.

Thanks for the suggestion.  I've looked at those, but not all my
customers are running kernels recent enough to have those helpers. I
suppose I could look into writing compatibility versions compatible
with the range of kernels I have to support.

>> Does the tty layer guarantee that the number of close() calls will
>> match the number of open() calls regardless of whether the open()
>> calls succeed or not?
>> 
>> If so, has that always been the policy, or is calling close() on
>> non-open device a new feature?
>
> The tty layer has always worked that way. It's I think unique in that
> behaviour in the kernel.

It seems that recent improvements to the kenel's pre-emptibility are
showing up all sorts of decade-old holes in this driver.  :/

> If you use the tty_port_open/hangup/close boilerplate then you can
> provide shutdown and activate port methods. The guarantee they
> provide is that they will be called in matching pairs, they will be
> called in the correct places, they will not be called in parallel
> with one another and they will properly handle hangup() events -
> where you shutdown on the hangup but the close occurs afterwards.

The problem is how to make a driver written that way work for somebody
running a 2.4 kernel (or even a not-very-recent 2.6 kernel).  Yes, I
have a few customers running 2.4 kernels, and lots of customers
running older 2.6 kernels.  I _might_ be able to convince management
it's time to drop support for 2.4, but there's no way I can abandon
customers using 2.6 kernels.

> In an ideal world eventually everything will be using the tty port
> helpers and then we can make that the underlying (sane) tty interface
> and clean up the rest.

-- 
Grant Edwards               grant.b.edwards        Yow! ... I have read the
                                  at               INSTRUCTIONS ...
                              gmail.com            

Re: close() handler of tty driver called for port that's not open?

[Alan Cox]

> In my mind, a port was only open after my open() handler returns
> success.  I guess that's wrong, and I should increment my use counter
> unconditionally each time my open() handler is called.

Probably - and remember to allow for hangup.

> customers are running kernels recent enough to have those helpers. I
> suppose I could look into writing compatibility versions compatible
> with the range of kernels I have to support.

It's actually pretty independant of the underlying tty code so may well
backport quite easily although it does touch on the refcounting stuff a
bit I guess.

> The problem is how to make a driver written that way work for somebody
> running a 2.4 kernel (or even a not-very-recent 2.6 kernel).  Yes, I
> have a few customers running 2.4 kernels, and lots of customers
> running older 2.6 kernels.  I _might_ be able to convince management
> it's time to drop support for 2.4, but there's no way I can abandon
> customers using 2.6 kernels.

For 2.4 I think you are on your own, but there are also no 2.4 kernels
without security holes so I tend to think of 2.4 as mostly dead.

In the 2.6 case the tty_port code depends upon the tty layer refcounting,
and the tty layer refcounting fixes exploitable races in tty hangup.

At a certainly point you may actually be best supporting both setups as
two driver variants. One for the kernels with old style lock_kernel
assumptions and the like, one for those where tty_port appears and we
relax the rest of the rules dramatically.

Alan

Re: close() handler of tty driver called for port that's not open?

[Grant Edwards]

On 2011-09-14, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> In my mind, a port was only open after my open() handler returns
>> success.  I guess that's wrong, and I should increment my use counter
>> unconditionally each time my open() handler is called.
>
> Probably - and remember to allow for hangup.

Thanks.  I was just looking at the way that is handled in the tty_port
helpers.

>> customers are running kernels recent enough to have those helpers. I
>> suppose I could look into writing compatibility versions compatible
>> with the range of kernels I have to support.
>
> It's actually pretty independant of the underlying tty code so may well
> backport quite easily although it does touch on the refcounting stuff a
> bit I guess.
>
>> The problem is how to make a driver written that way work for
>> somebody running a 2.4 kernel (or even a not-very-recent 2.6 kernel).
>> Yes, I have a few customers running 2.4 kernels, and lots of
>> customers running older 2.6 kernels.  I _might_ be able to convince
>> management it's time to drop support for 2.4, but there's no way I
>> can abandon customers using 2.6 kernels.
>
> For 2.4 I think you are on your own, but there are also no 2.4
> kernels without security holes so I tend to think of 2.4 as mostly
> dead.

I think a good argument can be made for that, but I AFAIK my 2.4
customers are running isolated, semi-embedded boxes where security
isn't a concern.  (I know... famous last words.)

> In the 2.6 case the tty_port code depends upon the tty layer
> refcounting, and the tty layer refcounting fixes exploitable races in
> tty hangup.
>
> At a certainly point you may actually be best supporting both setups
> as two driver variants. One for the kernels with old style
> lock_kernel assumptions and the like, one for those where tty_port
> appears and we relax the rest of the rules dramatically.

That's certainly worth considering.  Once done, it looks like it would
reduce future maintenance effort.

-- 
Grant Edwards               grant.b.edwards        Yow! One FISHWICH coming
                                  at               up!!
                              gmail.com