Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 3 Dec 2015 01:30:30 +0800
From:      Marcelo Araujo <araujobsdport@gmail.com>
To:        Fabien Thomas <fabient@freebsd.org>
Cc:        svn-src-stable-10@freebsd.org, svn-src-stable@freebsd.org,  src-committers@freebsd.org, svn-src-all@freebsd.org
Subject:   Re: svn commit: r291652 - stable/10/sys/netinet
Message-ID:  <CAOfEmZh2zTfN66Sj5h-pDxEXsj9%2Bp0DY-u9wScF2kuunR%2BTWbw@mail.gmail.com>
In-Reply-To: <201512021726.tB2HQb5p056013@repo.freebsd.org>
References:  <201512021726.tB2HQb5p056013@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hello,

I'm sorry, but that 'even' smell buggy for me! Could you explain what that
'even' really mean?

Best,
On Dec 3, 2015 1:26 AM, "Fabien Thomas" <fabient@freebsd.org> wrote:

> Author: fabient
> Date: Wed Dec  2 17:26:37 2015
> New Revision: 291652
> URL: https://svnweb.freebsd.org/changeset/base/291652
>
> Log:
>   MFC r291301:
>
>   The r241129 description was wrong that the scenario is possible
>    only for read locks on pcbs. The same race can happen with write
>    lock semantics as well.
>
>    The race scenario:
>
>    - Two threads (1 and 2) locate pcb with writer semantics
> (INPLOOKUP_WLOCKPCB)
>     and do in_pcbref() on it.
>    - 1 and 2 both drop the inp hash lock.
>    - Another thread (3) grabs the inp hash lock. Then it runs in_pcbfree(),
>     which wlocks the pcb. They must happen faster than 1 or 2 come
> INP_WLOCK()!
>    - 1 and 2 congest in INP_WLOCK().
>    - 3 does in_pcbremlists(), drops hash lock, and runs
> in_pcbrele_wlocked(),
>     which doesn't free the pcb due to two references on it.
>     Then it unlocks the pcb.
>    - 1 (or 2) gets wlock on the pcb, runs in_pcbrele_wlocked(), which
> doesn't
>     report inp as freed, due to 2 (or 1) still helding extra reference on
> it.
>     The thread tries to do smth with a disconnected pcb and crashes.
>
>    Submitted by:        emeric.poupon@stormshield.eu
>    Reviewed by: glebius@
>    Sponsored by: Stormshield
>    Tested by: Cassiano Peixoto, Stormshield
>
> Modified:
>   stable/10/sys/netinet/in_pcb.c
> Directory Properties:
>   stable/10/   (props changed)
>
> Modified: stable/10/sys/netinet/in_pcb.c
>
> ==============================================================================
> --- stable/10/sys/netinet/in_pcb.c      Wed Dec  2 16:29:36 2015
> (r291651)
> +++ stable/10/sys/netinet/in_pcb.c      Wed Dec  2 17:26:37 2015
> (r291652)
> @@ -1148,8 +1148,17 @@ in_pcbrele_wlocked(struct inpcb *inp)
>
>         INP_WLOCK_ASSERT(inp);
>
> -       if (refcount_release(&inp->inp_refcount) == 0)
> +       if (refcount_release(&inp->inp_refcount) == 0) {
> +               /*
> +                * If the inpcb has been freed, let the caller know, even
> if
> +                * this isn't the last reference.
> +                */
> +               if (inp->inp_flags2 & INP_FREED) {
> +                       INP_WUNLOCK(inp);
> +                       return (1);
> +               }
>                 return (0);
> +       }
>
>         KASSERT(inp->inp_socket == NULL, ("%s: inp_socket != NULL",
> __func__));
>
>
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOfEmZh2zTfN66Sj5h-pDxEXsj9%2Bp0DY-u9wScF2kuunR%2BTWbw>