Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1281564 > unrolled thread

Re: Asterisk deadlocks since Kernel 4.1

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Asterisk deadlocks since Kernel 4.1 Stefan Priebe - Profihost AG <s.priebe@profihost.ag> - 2015-12-02 10:50 +0100
    Re: Asterisk deadlocks since Kernel 4.1 Hannes Frederic Sowa <hannes@stressinduktion.org> - 2015-12-02 12:50 +0100
      Re: Asterisk deadlocks since Kernel 4.1 Philipp Hahn <pmhahn@pmhahn.de> - 2015-12-02 19:00 +0100
      Re: Asterisk deadlocks since Kernel 4.1 Stefan Priebe - Profihost AG <s.priebe@profihost.ag> - 2015-12-03 09:30 +0100
      Re: Asterisk deadlocks since Kernel 4.1 Stefan Priebe <s.priebe@profihost.ag> - 2015-12-04 19:30 +0100
        Re: Asterisk deadlocks since Kernel 4.1 Herbert Xu <herbert@gondor.apana.org.au> - 2015-12-05 02:10 +0100
          Re: Asterisk deadlocks since Kernel 4.1 Stefan Priebe <s.priebe@profihost.ag> - 2015-12-06 22:00 +0100
            Re: Asterisk deadlocks since Kernel 4.1 Herbert Xu <herbert@gondor.apana.org.au> - 2015-12-07 02:30 +0100
              Re: Asterisk deadlocks since Kernel 4.1 Stefan Priebe - Profihost AG <s.priebe@profihost.ag> - 2015-12-07 08:00 +0100
                netlink: Add missing goto statement to netlink_insert Herbert Xu <herbert@gondor.apana.org.au> - 2015-12-08 07:20 +0100
                  Re: netlink: Add missing goto statement to netlink_insert David Miller <davem@davemloft.net> - 2015-12-08 17:30 +0100
                  Re: netlink: Add missing goto statement to netlink_insert Greg KH <greg@kroah.com> - 2015-12-09 04:40 +0100
            Re: Asterisk deadlocks since Kernel 4.1 Philipp Hahn <pmhahn@pmhahn.de> - 2015-12-07 08:50 +0100
      Re: Asterisk deadlocks since Kernel 4.1 Philipp Matthias Hahn <pmhahn@pmhahn.de> - 2015-12-05 15:20 +0100
        Re: Asterisk deadlocks since Kernel 4.1 Stefan Priebe <s.priebe@profihost.ag> - 2015-12-05 16:40 +0100
    Re: Asterisk deadlocks since Kernel 4.1 Philipp Hahn <pmhahn@pmhahn.de> - 2015-12-02 19:00 +0100
      Re: Asterisk deadlocks since Kernel 4.1 Hannes Frederic Sowa <hannes@stressinduktion.org> - 2015-12-02 19:30 +0100

#1281564 — Re: Asterisk deadlocks since Kernel 4.1

Hi,

here are the results.

It works with 4.1.
It works with 4.2.
It does not work with 4.1.13.

git bisect tells me it stopped working after those two commits were applied:

commit d48623677191e0f035d7afd344f92cf880b01f8e
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Tue Sep 22 11:38:56 2015 +0800

    netlink: Replace rhash_portid with bound

commit 4e27762417669cb459971635be550eb7b5598286
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Fri Sep 18 19:16:50 2015 +0800

    netlink: Fix autobind race condition that leads to zero port ID

Stefan
Am 23.11.2015 um 13:57 schrieb Hannes Frederic Sowa:
> On Mon, Nov 23, 2015, at 13:44, Stefan Priebe - Profihost AG wrote:
>> Am 19.11.2015 um 20:51 schrieb Stefan Priebe:
>>>
>>> Am 19.11.2015 um 14:19 schrieb Florian Weimer:
>>>> On 11/19/2015 01:46 PM, Stefan Priebe - Profihost AG wrote:
>>>>
>>>>> I can try Kernel 4.4-rc1 next week. Or something else?
>>>>
>>>> I found this bug report which indicates that 4.1.10 works:
>>>>
>>>>    <https://issues.asterisk.org/jira/browse/ASTERISK-25251>
>>>>
>>>> But in your original report, you said that 4.1.13 is broken.
>>>
>>> That's correct i'm running 4.1.13.
>>>
>>>> This backtrace:
>>>>
>>>>    <https://issues.asterisk.org/jira/secure/attachment/52702/bt.txt>
>>>>
>>>> shows a lot of waiting on quite different netlink sockets.  So if this
>>>> is due to a race in Asterisk, it must have happened several times in a
>>>> row.
>>
>> Kernel 4.4-rc2 works fine. How can we grab / get an idea which is
>> causing the isse in 4.1? It's an LTE kernel so it should be fixed!
> 
> Thanks for testing. I was not able to reproduce it at all, with as much
> parallelism and threads as possible on any kernel. Could you try to do a
> git bisect?
> 
> Thanks,
> Hannes
> 
--
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/

[toc] | [next] | [standalone]

#1281696

Hello Stefan,

Stefan Priebe - Profihost AG <s.priebe@profihost.ag> writes:


> here are the results.
>
> It works with 4.1.
> It works with 4.2.
> It does not work with 4.1.13.
>
> git bisect tells me it stopped working after those two commits were applied:
>
> commit d48623677191e0f035d7afd344f92cf880b01f8e
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date:   Tue Sep 22 11:38:56 2015 +0800
>
>     netlink: Replace rhash_portid with bound
>
> commit 4e27762417669cb459971635be550eb7b5598286
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date:   Fri Sep 18 19:16:50 2015 +0800
>
>     netlink: Fix autobind race condition that leads to zero port ID

Cool, thanks a lot. Does this patch make a difference?

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 59651af..278e94c 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1137,7 +1137,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
 
 	/* We need to ensure that the socket is hashed and visible. */
 	smp_wmb();
-	nlk_sk(sk)->bound = portid;
+	nlk_sk(sk)->bound = true;
 
 err:
 	release_sock(sk);
--
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/

[toc] | [prev] | [next] | [standalone]

#1282199

Hi,

Am 02.12.2015 um 12:40 schrieb Hannes Frederic Sowa:
> Cool, thanks a lot. Does this patch make a difference?
> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 59651af..278e94c 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1137,7 +1137,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
>  
>  	/* We need to ensure that the socket is hashed and visible. */
>  	smp_wmb();
> -	nlk_sk(sk)->bound = portid;
> +	nlk_sk(sk)->bound = true;
>  
>  err:
>  	release_sock(sk);

Will test that tomorrow.

What confuss me is that it works with 4.3. I compared
net/netlink/af_netlink.[ch] for differences between v4.1.10 and v4.3 and
didn't find anything which would explain only 4,1 getting stuck.

Philipp
--
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/

[toc] | [prev] | [next] | [standalone]

#1282776

> Am 02.12.2015 um 12:40 schrieb Hannes Frederic Sowa <hannes@stressinduktion.org>:
> 
> Hello Stefan,
> 
> Stefan Priebe - Profihost AG <s.priebe@profihost.ag> writes:
> 
> 
>> here are the results.
>> 
>> It works with 4.1.
>> It works with 4.2.
>> It does not work with 4.1.13.
>> 
>> git bisect tells me it stopped working after those two commits were applied:
>> 
>> commit d48623677191e0f035d7afd344f92cf880b01f8e
>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>> Date:   Tue Sep 22 11:38:56 2015 +0800
>> 
>>    netlink: Replace rhash_portid with bound
>> 
>> commit 4e27762417669cb459971635be550eb7b5598286
>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>> Date:   Fri Sep 18 19:16:50 2015 +0800
>> 
>>    netlink: Fix autobind race condition that leads to zero port ID
> 
> Cool, thanks a lot. Does this patch make a difference?
> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 59651af..278e94c 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1137,7 +1137,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
> 
>    /* We need to ensure that the socket is hashed and visible. */
>    smp_wmb();
> -    nlk_sk(sk)->bound = portid;
> +    nlk_sk(sk)->bound = true;
> 
> err:
>    release_sock(sk);

Will try that soon but already started another test. I reverted all netlink patches from 4.1.13 back to 4.1 and applied all netlink patches from 4.1 up to 4.3.

Greets,
Stefan--
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/

[toc] | [prev] | [next] | [standalone]

#1284147

Hi,

I got it fixed / at least not live / deadlocking by doing applying the 
following patch - which is the diff of the commits below on top of 4.1.13.

patch:
http://pastebin.com/raw.php?i=hiuq4bsW

all commits / changes in reverse order:

* 0ceb380 - (6 weeks ago) netlink: fix locking around 
NETLINK_LIST_MEMBERSHIPS - David Herrmann (HEAD)
* c3f272b - (7 weeks ago) netlink: Trim skb to alloc size to avoid 
MSG_TRUNC - Arad, Ronen
* 9f87e0c - (2 months ago) netlink: Replace rhash_portid with bound - 
Herbert Xu
* 35e9890 - (3 months ago) netlink: Fix autobind race condition that 
leads to zero port ID - Herbert Xu
* f1d1215 - (3 months ago) netlink, mmap: transform mmap skb into full 
skb on taps - Daniel Borkmann
* faad871 - (3 months ago) netlink, mmap: fix edge-case leakages in nf 
queue zero-copy - Daniel Borkmann
* fb18c94 - (3 months ago) netlink, mmap: don't walk rx ring on poll if 
receive queue non-empty - Daniel Borkmann
* da13789 - (3 months ago) netlink: rx mmap: fix POLLIN condition - 
Ken-ichirou MATSUZAWA
* 808071f - (3 months ago) netlink: mmap: fix lookup frame position - 
Ken-ichirou MATSUZAWA
* 589bfd5 - (3 months ago) netlink: add NETLINK_CAP_ACK socket option - 
Christophe Ricard
* d23c4eb - (4 months ago) netlink: mmap: fix tx type check - 
Ken-ichirou MATSUZAWA
* 5dcc50a - (4 months ago) netlink: make sure -EBUSY won't escape from 
netlink_insert - Daniel Borkmann
* ada2b3e - (5 months ago) netlink: don't hold mutex in rcu callback 
when releasing mmapd ring - Florian Westphal
* e0f54a3 - (5 months ago) netlink: Delete an unnecessary check before 
the function call "module_put" - Markus Elfring
* 0a5bdaf - (6 months ago) netlink: add API to retrieve all group 
memberships - David Herrmann
* 30c6472 - (7 months ago) netlink: Use random autobind rover - Herbert Xu
* 021a670 - (7 months ago) netlink: Create kernel netlink sockets in the 
proper network namespace - Eric W. Biederman
* e1b01b4 - (7 months ago) net: Pass kern from net_proto_family.create 
to sk_alloc - Eric W. Biederman
* dd4b3c9 - (7 months ago) netlink: rename private flags and states - 
Nicolas Dichtel
* 0356126 - (2 days ago) Revert "netlink: don't hold mutex in rcu 
callback when releasing mmapd ring" - Stefan Priebe
* 231d0da - (2 days ago) Revert "netlink: make sure -EBUSY won't escape 
from netlink_insert" - Stefan Priebe
* e0f56af1 - (2 days ago) Revert "netlink, mmap: transform mmap skb into 
full skb on taps" - Stefan Priebe
* 23a0326 - (2 days ago) Revert "netlink: Fix autobind race condition 
that leads to zero port ID" - Stefan Priebe
* 97f4677 - (2 days ago) Revert "netlink: Replace rhash_portid with 
bound" - Stefan Priebe
* 40c851fe - (2 days ago) Revert "netlink: Trim skb to alloc size to 
avoid MSG_TRUNC" - Stefan Priebe
* 1f2ce4a - (4 weeks ago) Linux 4.1.13 - Greg Kroah-Hartman (v4.1.13, 
origin/linux-4.1.y)

So the netlink code is in line with 4.3.

Stefan

Am 02.12.2015 um 12:40 schrieb Hannes Frederic Sowa:
> Hello Stefan,
>
> Stefan Priebe - Profihost AG <s.priebe@profihost.ag> writes:
>
>
>> here are the results.
>>
>> It works with 4.1.
>> It works with 4.2.
>> It does not work with 4.1.13.
>>
>> git bisect tells me it stopped working after those two commits were applied:
>>
>> commit d48623677191e0f035d7afd344f92cf880b01f8e
>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>> Date:   Tue Sep 22 11:38:56 2015 +0800
>>
>>      netlink: Replace rhash_portid with bound
>>
>> commit 4e27762417669cb459971635be550eb7b5598286
>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>> Date:   Fri Sep 18 19:16:50 2015 +0800
>>
>>      netlink: Fix autobind race condition that leads to zero port ID
>
> Cool, thanks a lot. Does this patch make a difference?
>
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 59651af..278e94c 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1137,7 +1137,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
>
>   	/* We need to ensure that the socket is hashed and visible. */
>   	smp_wmb();
> -	nlk_sk(sk)->bound = portid;
> +	nlk_sk(sk)->bound = true;
>
>   err:
>   	release_sock(sk);
>
--
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/

[toc] | [prev] | [next] | [standalone]

#1284421

On Fri, Dec 04, 2015 at 07:26:12PM +0100, Stefan Priebe wrote:
>
> * 9f87e0c - (2 months ago) netlink: Replace rhash_portid with bound
> - Herbert Xu
> * 35e9890 - (3 months ago) netlink: Fix autobind race condition that
> leads to zero port ID - Herbert Xu
> * 30c6472 - (7 months ago) netlink: Use random autobind rover - Herbert Xu

These three patches are absolutely required in any kernel where the
netlink insertion is lockless.  So yes they should be applied to
4.1.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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/

[toc] | [prev] | [next] | [standalone]

#1284915

Hi Herbert,

i think i found the issue in 4.1 with netlink. Somebody made a mistake 
while backporting or cherry-picking your patch "netlink: Fix autobind 
race condition that leads to zero port ID" to 4.1.

It misses a goto in 4.1.

This goto is missing in 4.1:

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 4017e12..f15c001 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1132,7 +1132,6 @@ static int netlink_insert(struct sock *sk, u32 portid)
                 if (err == -EEXIST)
                         err = -EADDRINUSE;
                 sock_put(sk);
-               goto err;
         }

         /* We need to ensure that the socket is hashed and visible. */

Can you please confirm, that this is not correct and might cause those 
issues.

Stefan

Am 05.12.2015 um 02:08 schrieb Herbert Xu:
> On Fri, Dec 04, 2015 at 07:26:12PM +0100, Stefan Priebe wrote:
>>
>> * 9f87e0c - (2 months ago) netlink: Replace rhash_portid with bound
>> - Herbert Xu
>> * 35e9890 - (3 months ago) netlink: Fix autobind race condition that
>> leads to zero port ID - Herbert Xu
>> * 30c6472 - (7 months ago) netlink: Use random autobind rover - Herbert Xu
>
> These three patches are absolutely required in any kernel where the
> netlink insertion is lockless.  So yes they should be applied to
> 4.1.
>
> Thanks,
>
--
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/

[toc] | [prev] | [next] | [standalone]

#1284954

On Sun, Dec 06, 2015 at 09:56:34PM +0100, Stefan Priebe wrote:
> Hi Herbert,
> 
> i think i found the issue in 4.1 with netlink. Somebody made a
> mistake while backporting or cherry-picking your patch "netlink: Fix
> autobind race condition that leads to zero port ID" to 4.1.
> 
> It misses a goto in 4.1.
> 
> This goto is missing in 4.1:
> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 4017e12..f15c001 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1132,7 +1132,6 @@ static int netlink_insert(struct sock *sk, u32 portid)
>                 if (err == -EEXIST)
>                         err = -EADDRINUSE;
>                 sock_put(sk);
> -               goto err;
>         }
> 
>         /* We need to ensure that the socket is hashed and visible. */
> 
> Can you please confirm, that this is not correct and might cause
> those issues.

Well spotted! Yes this would be a fatal error and can cause the
problems you guys are seeing.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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/

[toc] | [prev] | [next] | [standalone]

#1285044

Hi Herbert,

Am 07.12.2015 um 02:20 schrieb Herbert Xu:
> On Sun, Dec 06, 2015 at 09:56:34PM +0100, Stefan Priebe wrote:
>> Hi Herbert,
>>
>> i think i found the issue in 4.1 with netlink. Somebody made a
>> mistake while backporting or cherry-picking your patch "netlink: Fix
>> autobind race condition that leads to zero port ID" to 4.1.
>>
>> It misses a goto in 4.1.
>>
>> This goto is missing in 4.1:
>>
>> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
>> index 4017e12..f15c001 100644
>> --- a/net/netlink/af_netlink.c
>> +++ b/net/netlink/af_netlink.c
>> @@ -1132,7 +1132,6 @@ static int netlink_insert(struct sock *sk, u32 portid)
>>                 if (err == -EEXIST)
>>                         err = -EADDRINUSE;
>>                 sock_put(sk);
>> -               goto err;
>>         }
>>
>>         /* We need to ensure that the socket is hashed and visible. */
>>
>> Can you please confirm, that this is not correct and might cause
>> those issues.
> 
> Well spotted! Yes this would be a fatal error and can cause the
> problems you guys are seeing.

Thanks, good. Can you help me to get this fix upstream into the stable
lines?

Stefan

> 
> Thanks,
> 
--
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/

[toc] | [prev] | [next] | [standalone]

#1286259 — netlink: Add missing goto statement to netlink_insert

On Mon, Dec 07, 2015 at 07:58:25AM +0100, Stefan Priebe - Profihost AG wrote:
>
> Thanks, good. Can you help me to get this fix upstream into the stable
> lines?

Sure.  Greg, please apply this patch to fix up the backport for 4.1.

---8<---
The backport of 1f770c0a09da855a2b51af6d19de97fb955eca85 ("netlink:
Fix autobind race condition that leads to zero port ID") missed a
goto statement, which causes netlink to break subtly.

This was discovered by Stefan Priebe <s.priebe@profihost.ag>.

Fixes: 4e2776241766 ("netlink: Fix autobind race condition that...")
Reported-by: Stefan Priebe <s.priebe@profihost.ag>
Reported-by: Philipp Hahn <pmhahn@pmhahn.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index d139c43..0d6038c 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1118,6 +1118,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
 		if (err == -EEXIST)
 			err = -EADDRINUSE;
 		sock_put(sk);
+		goto err;
 	}
 
 	/* We need to ensure that the socket is hashed and visible. */

-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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/

[toc] | [prev] | [next] | [standalone]

#1286655 — Re: netlink: Add missing goto statement to netlink_insert

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 8 Dec 2015 14:13:19 +0800

> On Mon, Dec 07, 2015 at 07:58:25AM +0100, Stefan Priebe - Profihost AG wrote:
>>
>> Thanks, good. Can you help me to get this fix upstream into the stable
>> lines?
> 
> Sure.  Greg, please apply this patch to fix up the backport for 4.1.

Herbert, thanks so much for taking care of this.

> ---8<---
> The backport of 1f770c0a09da855a2b51af6d19de97fb955eca85 ("netlink:
> Fix autobind race condition that leads to zero port ID") missed a
> goto statement, which causes netlink to break subtly.
> 
> This was discovered by Stefan Priebe <s.priebe@profihost.ag>.
> 
> Fixes: 4e2776241766 ("netlink: Fix autobind race condition that...")
> Reported-by: Stefan Priebe <s.priebe@profihost.ag>
> Reported-by: Philipp Hahn <pmhahn@pmhahn.de>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Acked-by: David S. Miller <davem@davemloft.net>

> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index d139c43..0d6038c 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1118,6 +1118,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
>  		if (err == -EEXIST)
>  			err = -EADDRINUSE;
>  		sock_put(sk);
> +		goto err;
>  	}
>  
>  	/* We need to ensure that the socket is hashed and visible. */
> 
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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/

[toc] | [prev] | [next] | [standalone]

#1287063 — Re: netlink: Add missing goto statement to netlink_insert

On Tue, Dec 08, 2015 at 02:13:19PM +0800, Herbert Xu wrote:
> On Mon, Dec 07, 2015 at 07:58:25AM +0100, Stefan Priebe - Profihost AG wrote:
> >
> > Thanks, good. Can you help me to get this fix upstream into the stable
> > lines?
> 
> Sure.  Greg, please apply this patch to fix up the backport for 4.1.

Now queued up, thanks for the fix.

greg k-h
--
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/

[toc] | [prev] | [next] | [standalone]

#1285059

Hello Stefan,

Am 06.12.2015 um 21:56 schrieb Stefan Priebe:
> i think i found the issue in 4.1 with netlink. Somebody made a mistake
> while backporting or cherry-picking your patch "netlink: Fix autobind
> race condition that leads to zero port ID" to 4.1.
> 
> It misses a goto in 4.1.
> 
> This goto is missing in 4.1:
> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 4017e12..f15c001 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1132,7 +1132,6 @@ static int netlink_insert(struct sock *sk, u32
> portid)
>                 if (err == -EEXIST)
>                         err = -EADDRINUSE;
>                 sock_put(sk);
> -               goto err;
>         }
> 
>         /* We need to ensure that the socket is hashed and visible. */
> 
> Can you please confirm, that this is not correct and might cause those
> issues.

I just tested that patch and it seems to fix our hang. Thank you for
your good work.

Philipp

PS: I guess I can skip testing your other test request as this simple
patch is part of your other hiuq4bsW patch. If I should still test it,
just send a note.
--
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/

[toc] | [prev] | [next] | [standalone]

#1284561

Hello Hannes,

On Wed, Dec 02, 2015 at 12:40:32PM +0100, Hannes Frederic Sowa wrote:
> > git bisect tells me it stopped working after those two commits were applied:
> >
> > commit d48623677191e0f035d7afd344f92cf880b01f8e
> > Author: Herbert Xu <herbert@gondor.apana.org.au>
> > Date:   Tue Sep 22 11:38:56 2015 +0800
> >
> >     netlink: Replace rhash_portid with bound
> >
> > commit 4e27762417669cb459971635be550eb7b5598286
> > Author: Herbert Xu <herbert@gondor.apana.org.au>
> > Date:   Fri Sep 18 19:16:50 2015 +0800
> >
> >     netlink: Fix autobind race condition that leads to zero port ID
> 
> Cool, thanks a lot. Does this patch make a difference?
> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 59651af..278e94c 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1137,7 +1137,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
>  
>  	/* We need to ensure that the socket is hashed and visible. */
>  	smp_wmb();
> -	nlk_sk(sk)->bound = portid;
> +	nlk_sk(sk)->bound = true;
>  
>  err:
>  	release_sock(sk);

Didn't help, test program still gets stuck; sorry :-(

Philipp

-- 
  / /  (_)__  __ ____  __ Philipp Hahn
 / /__/ / _ \/ // /\ \/ /
/____/_/_//_/\_,_/ /_/\_\ pmhahn@pmhahn.de
--
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/

[toc] | [prev] | [next] | [standalone]

#1284564

Hello Philipp,

Am 05.12.2015 um 15:19 schrieb Philipp Matthias Hahn:
> Hello Hannes,
>
> On Wed, Dec 02, 2015 at 12:40:32PM +0100, Hannes Frederic Sowa wrote:
>>> git bisect tells me it stopped working after those two commits were applied:
>>>
>>> commit d48623677191e0f035d7afd344f92cf880b01f8e
>>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>>> Date:   Tue Sep 22 11:38:56 2015 +0800
>>>
>>>      netlink: Replace rhash_portid with bound
>>>
>>> commit 4e27762417669cb459971635be550eb7b5598286
>>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>>> Date:   Fri Sep 18 19:16:50 2015 +0800
>>>
>>>      netlink: Fix autobind race condition that leads to zero port ID
>>
>> Cool, thanks a lot. Does this patch make a difference?
>>
>> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
>> index 59651af..278e94c 100644
>> --- a/net/netlink/af_netlink.c
>> +++ b/net/netlink/af_netlink.c
>> @@ -1137,7 +1137,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
>>
>>   	/* We need to ensure that the socket is hashed and visible. */
>>   	smp_wmb();
>> -	nlk_sk(sk)->bound = portid;
>> +	nlk_sk(sk)->bound = true;
>>
>>   err:
>>   	release_sock(sk);
>
> Didn't help, test program still gets stuck; sorry :-(

Can you please try this patch on top of 4.1.13 (which helps me):
http://pastebin.com/raw.php?i=hiuq4bsW

Stefan

>
> Philipp
>
--
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/

[toc] | [prev] | [next] | [standalone]

#1282219

Hi,

Am 02.12.2015 um 10:45 schrieb Stefan Priebe - Profihost AG:
> here are the results.
> 
> It works with 4.1.
> It works with 4.2.
> It does not work with 4.1.13.

the patches were first commitet in v4.3-rc3 and appear as backports only
since v4.2.3 and v4.1.10

> git bisect tells me it stopped working after those two commits were applied:
> 
> commit d48623677191e0f035d7afd344f92cf880b01f8e
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date:   Tue Sep 22 11:38:56 2015 +0800
> 
>     netlink: Replace rhash_portid with bound
> 
> commit 4e27762417669cb459971635be550eb7b5598286
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date:   Fri Sep 18 19:16:50 2015 +0800
> 
>     netlink: Fix autobind race condition that leads to zero port ID

I identified the same two patches for our prpblem; see mail from
2015-11-17 16:53 with subject "Strange PF_NETLINK NETLINK_ROUTE stall:
netlink: Fix autobind race condition that leads to zero port ID"

<http://marc.info/?l=linux-kernel&m=144863837825887&w=2>

Philipp
--
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/

[toc] | [prev] | [next] | [standalone]

#1282249

Hello,

On Wed, Dec 2, 2015, at 18:15, Philipp Hahn wrote:
> Hi,
> 
> Am 02.12.2015 um 10:45 schrieb Stefan Priebe - Profihost AG:
> > here are the results.
> > 
> > It works with 4.1.
> > It works with 4.2.
> > It does not work with 4.1.13.
> 
> the patches were first commitet in v4.3-rc3 and appear as backports only
> since v4.2.3 and v4.1.10
> 
> > git bisect tells me it stopped working after those two commits were applied:
> > 
> > commit d48623677191e0f035d7afd344f92cf880b01f8e
> > Author: Herbert Xu <herbert@gondor.apana.org.au>
> > Date:   Tue Sep 22 11:38:56 2015 +0800
> > 
> >     netlink: Replace rhash_portid with bound
> > 
> > commit 4e27762417669cb459971635be550eb7b5598286
> > Author: Herbert Xu <herbert@gondor.apana.org.au>
> > Date:   Fri Sep 18 19:16:50 2015 +0800
> > 
> >     netlink: Fix autobind race condition that leads to zero port ID
> 
> I identified the same two patches for our prpblem; see mail from
> 2015-11-17 16:53 with subject "Strange PF_NETLINK NETLINK_ROUTE stall:
> netlink: Fix autobind race condition that leads to zero port ID"
> 
> <http://marc.info/?l=linux-kernel&m=144863837825887&w=2>

Thanks, I missed this mail. I have a further look. I couldn't reproduce
it myself but will check if your test program does it.

Thanks a lot,
Hannes
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web