lkml.org 
[lkml]   [2010]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCHv5 05/17] tty: don't allow ldisc dcd_change() after ldisc halt
    В Wed, 24 Nov 2010 16:43:29 +0000
    Alan Cox <alan@lxorguk.ukuu.org.uk> пишет:

    > On Wed, 24 Nov 2010 19:15:43 +0300
    > Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:
    >
    > > There was a possibility that uart_handle_dcd_change() could obtain a
    > > reference to ldisc while running in parallel with tty_set_ldisc() on
    > > different CPU but call dcd_change() operation after
    > > tty_ldisc_close() which is incorrect.
    >
    > How can this occur ?
    >
    >
    > > + spin_lock_irqsave(&tty->dcd_change_lock, flags);
    > > +
    > > + ld = tty_ldisc_ref(tty);
    >
    > What is the expecting lock ordering rule here ?
    >
    >
    >
    > I don't see why this patch is needed. You've got an ldisc ref from
    > tty_ldisc_ref, until you drop that ldisc ref you are fine. If for some
    > reason that is not the case then there is a bug in the ldisc code.

    Yes, indeed, it's a bug. Please consider the following example:

    CPU1 CPU2
    =========================================================
    uart_handle_dcd_change() { tty_set_ldisc() {
    ld = tty_ldisc_ref(...) ...
    ... tty_ldisc_halt(...)
    ... ...
    ... tty_ldisc_close(...)
    if (ld && ld->ops->dcd_change) ...
    ld->ops->dcd_change(...); ...
    ... tty_ldisc_open(...)
    } }

    I think that semantically ldisc ops should never be called before open
    or after close. This situation is possible because tty_ldisc_halt() only
    ensures that no more references are taken. This is ok for everything
    except dcd_change() because it cleans up workqueue and doesn't accept
    any more data. dcd_change() is a different story because it doesn't use
    workqueues.

    I think tty code is exactly the right place to fix this bug; this is
    what my patch is for.

    --
    Alexander
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2010-11-24 23:41    [W:8.978 / U:0.120 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site