[PATCH] rose_node_list_lock was not released before returning to user space - Kernel

This is a discussion on [PATCH] rose_node_list_lock was not released before returning to user space - Kernel ; From 74859daa5a1ef4d793c03c77b52affa0f95c609d Mon Sep 17 00:00:00 2001 From: Bernard Pidoux Date: Sat, 19 Apr 2008 20:13:55 +0200 Subject: [PATCH] rose_node_list_lock was not released before returning to user space I have already submited this patch on January 11, 2008. As the ...

+ Reply to Thread
Results 1 to 6 of 6

Thread: [PATCH] rose_node_list_lock was not released before returning to user space

  1. [PATCH] rose_node_list_lock was not released before returning to user space

    From 74859daa5a1ef4d793c03c77b52affa0f95c609d Mon Sep 17 00:00:00 2001
    From: Bernard Pidoux
    Date: Sat, 19 Apr 2008 20:13:55 +0200
    Subject: [PATCH] rose_node_list_lock was not released before returning to user space

    I have already submited this patch on January 11, 2008.
    As the bug is still present, I resend it.

    ================================================
    [ BUG: lock held when returning to user space! ]
    ------------------------------------------------
    xfbbd/3683 is leaving the kernel with locks still held!
    1 lock held by xfbbd/3683:
    #0: (sk_lock-AF_ROSE){--..}, at: [] rose_connect+0x73/0x420 [rose]

    INFO: task xfbbd:3683 blocked for more than 120 seconds.
    "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
    xfbbd D 00000246 0 3683 3669
    c6965ee0 00000092 c02c5c40 00000246 c0f6b5f0 c0f6b5c0 c0f6b5f0 c0f6b5c0
    c0f6b614 c6965f18 c024b74b ffffffff c06ba070 00000000 00000000 00000001
    c6ab07c0 c012d450 c0f6b634 c0f6b634 c7b5bf10 c0d6004c c7b5bf10 c6965f40
    Call Trace:
    [] lock_sock_nested+0x6b/0xd0
    [] ? autoremove_wake_function+0x0/0x40
    [] sock_fasync+0x41/0x150
    [] sock_close+0x19/0x40
    [] __fput+0xb4/0x170
    [] fput+0x18/0x20
    [] filp_close+0x3e/0x70
    [] sys_close+0x69/0xb0
    [] sysenter_past_esp+0x5f/0xa5
    =======================
    INFO: lockdep is turned off.


    Signed-off-by: Bernard Pidoux
    ---
    net/rose/af_rose.c | 6 ++++--
    1 files changed, 4 insertions(+), 2 deletions(-)

    diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
    index d1ff3f8..1ebf652 100644
    --- a/net/rose/af_rose.c
    +++ b/net/rose/af_rose.c
    @@ -760,8 +760,10 @@ static int rose_connect(struct socket *sock, struct sockaddr *uaddr, int addr_le

    rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause,
    &diagnostic);
    - if (!rose->neighbour)
    - return -ENETUNREACH;
    + if (!rose->neighbour) {
    + err = -ENETUNREACH;
    + goto out_release;
    + }

    rose->lci = rose_new_lci(rose->neighbour);
    if (!rose->lci) {
    --
    1.5.5


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] rose_node_list_lock was not released before returning to user space

    From: Bernard Pidoux
    Date: Sat, 19 Apr 2008 23:12:20 +0200

    > I have already submited this patch on January 11, 2008.
    > As the bug is still present, I resend it.


    I'll apply this fix, thanks a lot Bernard.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] rose_node_list_lock was not released before returning to user space

    From: Bernard Pidoux
    Date: Sat, 19 Apr 2008 23:12:20 +0200

    > Subject: [PATCH] rose_node_list_lock was not released before returning to user space


    BTW, I had to fix the commit message because it's not the
    rose_node_list_lock that isn't being released, it's the socket lock.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. [PATCH] soft lockup rose_node_list_lock

    From f40c15d0ea5a22178e6cbb0331486d2297abeeb7 Mon Sep 17 00:00:00 2001
    From: Bernard Pidoux
    Date: Sun, 20 Apr 2008 18:19:06 +0200
    Subject: [PATCH] soft lockup rose_node_list_lock

    [ INFO: possible recursive locking detected ]
    2.6.25 #3
    ---------------------------------------------
    ax25ipd/3811 is trying to acquire lock:
    (rose_node_list_lock){-+..}, at: [] rose_get_neigh+0x1a/0xa0
    [rose]

    but task is already holding lock:
    (rose_node_list_lock){-+..}, at: []
    rose_route_frame+0x4d/0x620 [rose]

    other info that might help us debug this:
    6 locks held by ax25ipd/3811:
    #0: (&tty->atomic_write_lock){--..}, at: []
    tty_write_lock+0x1c/0x50
    #1: (rcu_read_lock){..--}, at: [] net_rx_action+0x96/0x230
    #2: (rcu_read_lock){..--}, at: [] netif_receive_skb+0x100/0x2f0
    #3: (rose_node_list_lock){-+..}, at: []
    rose_route_frame+0x4d/0x620 [rose]
    #4: (rose_neigh_list_lock){-+..}, at: []
    rose_route_frame+0x57/0x620 [rose]
    #5: (rose_route_list_lock){-+..}, at: []
    rose_route_frame+0x61/0x620 [rose]

    stack backtrace:
    Pid: 3811, comm: ax25ipd Not tainted 2.6.25 #3
    [] print_deadlock_bug+0xc7/0xd0
    [] check_deadlock+0x9a/0xb0
    [] validate_chain+0x1e2/0x310
    [] ? validate_chain+0xa5/0x310
    [] ? native_sched_clock+0x88/0xc0
    [] __lock_acquire+0x1a1/0x750
    [] lock_acquire+0x81/0xa0
    [] ? rose_get_neigh+0x1a/0xa0 [rose]
    [] _spin_lock_bh+0x33/0x60
    [] ? rose_get_neigh+0x1a/0xa0 [rose]
    [] rose_get_neigh+0x1a/0xa0 [rose]
    [] rose_route_frame+0x464/0x620 [rose]
    [] ? _read_unlock+0x1d/0x20
    [] ? rose_route_frame+0x0/0x620 [rose]
    [] ax25_rx_iframe+0x66/0x3b0 [ax25]
    [] ? ax25_start_t3timer+0x1f/0x40 [ax25]
    [] ax25_std_frame_in+0x7fb/0x890 [ax25]
    [] ? _spin_unlock_bh+0x25/0x30
    [] ax25_kiss_rcv+0x2c6/0x800 [ax25]
    [] ? sock_def_readable+0x59/0x80
    [] ? __lock_release+0x47/0x70
    [] ? sock_def_readable+0x59/0x80
    [] ? _read_unlock+0x1d/0x20
    [] ? sock_def_readable+0x59/0x80
    [] ? sock_queue_rcv_skb+0x13a/0x1d0
    [] ? sock_queue_rcv_skb+0x45/0x1d0
    [] ? ax25_kiss_rcv+0x0/0x800 [ax25]
    [] netif_receive_skb+0x255/0x2f0
    [] ? netif_receive_skb+0x100/0x2f0
    [] process_backlog+0x7c/0xf0
    [] net_rx_action+0x16c/0x230
    [] ? net_rx_action+0x96/0x230
    [] __do_softirq+0x93/0x120
    [] ? mkiss_receive_buf+0x33a/0x3f0 [mkiss]
    [] do_softirq+0x57/0x60
    [] local_bh_enable_ip+0xa5/0xe0
    [] _spin_unlock_bh+0x25/0x30
    [] mkiss_receive_buf+0x33a/0x3f0 [mkiss]
    [] pty_write+0x47/0x60
    [] write_chan+0x1b0/0x220
    [] ? tty_write_lock+0x1c/0x50
    [] ? default_wake_function+0x0/0x10
    [] tty_write+0x12a/0x1c0
    [] ? write_chan+0x0/0x220
    [] vfs_write+0x96/0x130
    [] ? tty_write+0x0/0x1c0
    [] sys_write+0x3d/0x70
    [] sysenter_past_esp+0x5f/0xa5
    =======================
    BUG: soft lockup - CPU#0 stuck for 61s! [ax25ipd:3811]

    Pid: 3811, comm: ax25ipd Not tainted (2.6.25 #3)
    EIP: 0060:[] EFLAGS: 00000246 CPU: 0
    EIP is at native_read_tsc+0xb/0x20
    EAX: b404aa2c EBX: b404a9c9 ECX: 017f1000 EDX: 0000076b
    ESI: 00000001 EDI: 00000000 EBP: ecc83afc ESP: ecc83afc
    DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
    CR0: 8005003b CR2: b7f5f000 CR3: 2cd8e000 CR4: 000006f0
    DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
    DR6: ffff0ff0 DR7: 00000400
    [] delay_tsc+0x17/0x30
    [] __delay+0x9/0x10
    [] __spin_lock_debug+0x76/0xf0
    [] ? spin_bug+0x18/0x100
    [] ? __lock_contended+0xa3/0x110
    [] _raw_spin_lock+0x68/0x90
    [] _spin_lock_bh+0x4f/0x60
    [] ? rose_get_neigh+0x1a/0xa0 [rose]
    [] rose_get_neigh+0x1a/0xa0 [rose]
    [] rose_route_frame+0x464/0x620 [rose]
    [] ? _read_unlock+0x1d/0x20
    [] ? rose_route_frame+0x0/0x620 [rose]
    [] ax25_rx_iframe+0x66/0x3b0 [ax25]
    [] ? ax25_start_t3timer+0x1f/0x40 [ax25]
    [] ax25_std_frame_in+0x7fb/0x890 [ax25]
    [] ? _spin_unlock_bh+0x25/0x30
    [] ax25_kiss_rcv+0x2c6/0x800 [ax25]
    [] ? sock_def_readable+0x59/0x80
    [] ? __lock_release+0x47/0x70
    [] ? sock_def_readable+0x59/0x80
    [] ? _read_unlock+0x1d/0x20
    [] ? sock_def_readable+0x59/0x80
    [] ? sock_queue_rcv_skb+0x13a/0x1d0
    [] ? sock_queue_rcv_skb+0x45/0x1d0
    [] ? ax25_kiss_rcv+0x0/0x800 [ax25]
    [] netif_receive_skb+0x255/0x2f0
    [] ? netif_receive_skb+0x100/0x2f0
    [] process_backlog+0x7c/0xf0
    [] net_rx_action+0x16c/0x230
    [] ? net_rx_action+0x96/0x230
    [] __do_softirq+0x93/0x120
    [] ? mkiss_receive_buf+0x33a/0x3f0 [mkiss]
    [] do_softirq+0x57/0x60
    [] local_bh_enable_ip+0xa5/0xe0
    [] _spin_unlock_bh+0x25/0x30
    [] mkiss_receive_buf+0x33a/0x3f0 [mkiss]
    [] pty_write+0x47/0x60
    [] write_chan+0x1b0/0x220
    [] ? tty_write_lock+0x1c/0x50
    [] ? default_wake_function+0x0/0x10
    [] tty_write+0x12a/0x1c0
    [] ? write_chan+0x0/0x220
    [] vfs_write+0x96/0x130
    [] ? tty_write+0x0/0x1c0
    [] sys_write+0x3d/0x70
    [] sysenter_past_esp+0x5f/0xa5
    =======================

    Since rose_route_frame() does not use rose_node_list we can safely
    remove rose_node_list_lock spin lock here and let it be free for
    rose_get_neigh().

    Signed-off-by: Bernard Pidoux
    ---
    net/rose/rose_route.c | 2 --
    1 files changed, 0 insertions(+), 2 deletions(-)

    diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
    index fb9359f..5053a53 100644
    --- a/net/rose/rose_route.c
    +++ b/net/rose/rose_route.c
    @@ -857,7 +857,6 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
    src_addr = (rose_address *)(skb->data + 9);
    dest_addr = (rose_address *)(skb->data + 4);

    - spin_lock_bh(&rose_node_list_lock);
    spin_lock_bh(&rose_neigh_list_lock);
    spin_lock_bh(&rose_route_list_lock);

    @@ -1060,7 +1059,6 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb
    *ax25)
    out:
    spin_unlock_bh(&rose_route_list_lock);
    spin_unlock_bh(&rose_neigh_list_lock);
    - spin_unlock_bh(&rose_node_list_lock);

    return res;
    }
    --
    1.5.5
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] soft lockup rose_node_list_lock

    From: Bernard Pidoux
    Date: Sun, 20 Apr 2008 19:09:23 +0200

    > Since rose_route_frame() does not use rose_node_list we can safely
    > remove rose_node_list_lock spin lock here and let it be free for
    > rose_get_neigh().
    >
    > Signed-off-by: Bernard Pidoux


    Indeed, I went over this code several times and I can't
    see any reason for rose_route_frame() to take the node
    list lock.

    Patch applied, thanks Bernard. But one thing...

    > diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
    > index fb9359f..5053a53 100644
    > --- a/net/rose/rose_route.c
    > +++ b/net/rose/rose_route.c
    > @@ -857,7 +857,6 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
    > src_addr = (rose_address *)(skb->data + 9);
    > dest_addr = (rose_address *)(skb->data + 4);
    >
    > - spin_lock_bh(&rose_node_list_lock);
    > spin_lock_bh(&rose_neigh_list_lock);
    > spin_lock_bh(&rose_route_list_lock);
    >


    Could you please fix your email client so it doesn't corrupt
    patches like this? I've had to apply all of your patches by
    hand because the tabs have been converted into spaces. Use
    MIME attachments if you have to.

    Thanks again.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH] soft lockup rose_node_list_lock

    Hi David,

    I also spent a lot of time to understand how rose behaved and I agree
    that it is difficult to decifer a code especially dealing
    with socket programming and when it was written by someone else.
    But as a radioamateur, Linux is a hobby for me and I like to learn.

    Actually, rose_get_neigh() is called when two different events are
    occuring :

    - first, it is called by rose_connect() in order to find if an adjacent
    node is ready to route to a specific ROSE address.
    - second, rose_route_frame() calls rose_get_neigh() every time an
    incoming frame must be routed to an appropriate AX25 connection.

    By the way, rose_get_neigh() function is not optimized for it does not
    check if an adjacent node is already connected before a new connect is
    requested.
    For this purpose I have derived a new function, I named
    rose_get_route(), that is called by rose_route_frame() to find a route
    via an adjacent node.
    This function has been tested for months now and it works fine.
    It adds the automatic frames routing that rose needed desperately.
    I will send next a patch with this new rose_get_route().

    Bernard Pidoux

    p.s. my email client is set for MIME attachements, but it seems corrupted.
    I will fix that. Sorry for the unvoluntary increase of workload it
    gave you.


    David Miller a écrit :
    > From: Bernard Pidoux
    > Date: Sun, 20 Apr 2008 19:09:23 +0200
    >
    >
    >> Since rose_route_frame() does not use rose_node_list we can safely
    >> remove rose_node_list_lock spin lock here and let it be free for
    >> rose_get_neigh().
    >>
    >> Signed-off-by: Bernard Pidoux
    >>

    >
    > Indeed, I went over this code several times and I can't
    > see any reason for rose_route_frame() to take the node
    > list lock.
    >
    > Patch applied, thanks Bernard. But one thing...
    >
    >
    >> diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
    >> index fb9359f..5053a53 100644
    >> --- a/net/rose/rose_route.c
    >> +++ b/net/rose/rose_route.c
    >> @@ -857,7 +857,6 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
    >> src_addr = (rose_address *)(skb->data + 9);
    >> dest_addr = (rose_address *)(skb->data + 4);
    >>
    >> - spin_lock_bh(&rose_node_list_lock);
    >> spin_lock_bh(&rose_neigh_list_lock);
    >> spin_lock_bh(&rose_route_list_lock);
    >>
    >>

    >
    > Could you please fix your email client so it doesn't corrupt
    > patches like this? I've had to apply all of your patches by
    > hand because the tabs have been converted into spaces. Use
    > MIME attachments if you have to.
    >
    > Thanks again.
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-hams" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    >
    >
    >


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread