Re: [PATCH net] netlink: make sure -EBUSY won't escape from netlink_insert

[David Miller]

From: Christoph Paasch 
Date: Wed, 16 Sep 2015 22:41:14 -0700

> Hello,
> 
> On 10/08/15 - 11:00:15, David Miller wrote:
>> From: Daniel Borkmann 
>> Date: Fri,  7 Aug 2015 00:26:41 +0200
>> > Linus reports the following deadlock on rtnl_mutex; triggered only
>> > once so far (extract):
>>  ...
>> > It seems so far plausible that the recursive call into rtnetlink_rcv()
>> > looks suspicious. One way, where this could trigger is that the senders
>> > NETLINK_CB(skb).portid was wrongly 0 (which is rtnetlink socket), so
>> > the rtnl_getlink() request's answer would be sent to the kernel instead
>> > to the actual user process, thus grabbing rtnl_mutex() twice.
>> > 
>> > One theory would be that netlink_autobind() triggered via netlink_sendmsg()
>> > internally overwrites the -EBUSY error to 0, but where it is wrongly
>> > originating from __netlink_insert() instead. That would reset the
>> > socket's portid to 0, which is then filled into NETLINK_CB(skb).portid
>> > later on. As commit d470e3b483dc ("[NETLINK]: Fix two socket hashing 
>> > bugs.")
>> > also puts it, -EBUSY should not be propagated from netlink_insert().
>> > 
>> > It looks like it's very unlikely to reproduce. We need to trigger the
>> > rhashtable_insert_rehash() handler under a situation where rehashing
>> > currently occurs (one /rare/ way would be to hit ht->elasticity limits
>> > while not filled enough to expand the hashtable, but that would rather
>> > require a specifically crafted bind() sequence with knowledge about
>> > destination slots, seems unlikely). It probably makes sense to guard
>> > __netlink_insert() in any case and remap that error. It was suggested
>> > that EOVERFLOW might be better than an already overloaded ENOMEM.
>> > 
>> > Reference: http://thread.gmane.org/gmane.linux.network/372676
>> > Reported-by: Linus Torvalds 
>> > Signed-off-by: Daniel Borkmann 
>> 
>> Applied and queued up for -stable, thanks.
> 
> can this patch get queued up for 4.1 as well?
> It seems to fix a similar issue in 4.1.6.

Done.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] netlink: make sure -EBUSY won't escape from netlink_insert

[Daniel Borkmann]

On 09/18/2015 04:09 AM, Herbert Xu wrote:

On Thu, Sep 17, 2015 at 11:47:12AM -0700, Linus Torvalds wrote:

On Wed, Sep 16, 2015 at 10:41 PM, Christoph Paasch
 wrote:


can this patch get queued up for 4.1 as well?
It seems to fix a similar issue in 4.1.6.


I think Herbert has an additional patch for this issue. But yes, I
think should be scheduled for stable. Herbert?


Yes this should be safe for stable, even though the real cause of
the problem is probably the one that Tejun spotted.


Yes, agreed.

Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] netlink: make sure -EBUSY won't escape from netlink_insert

[Linus Torvalds]

On Wed, Sep 16, 2015 at 10:41 PM, Christoph Paasch
 wrote:
>
> can this patch get queued up for 4.1 as well?
> It seems to fix a similar issue in 4.1.6.

I think Herbert has an additional patch for this issue. But yes, I
think should be scheduled for stable. Herbert?

Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] netlink: make sure -EBUSY won't escape from netlink_insert

[Herbert Xu]

On Thu, Sep 17, 2015 at 11:47:12AM -0700, Linus Torvalds wrote:
> On Wed, Sep 16, 2015 at 10:41 PM, Christoph Paasch
>  wrote:
> >
> > can this patch get queued up for 4.1 as well?
> > It seems to fix a similar issue in 4.1.6.
> 
> I think Herbert has an additional patch for this issue. But yes, I
> think should be scheduled for stable. Herbert?

Yes this should be safe for stable, even though the real cause of
the problem is probably the one that Tejun spotted.

Cheers,
-- 
Email: Herbert Xu 
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 netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] netlink: make sure -EBUSY won't escape from netlink_insert

[David Miller]

From: Daniel Borkmann dan...@iogearbox.net
Date: Fri,  7 Aug 2015 00:26:41 +0200

 Linus reports the following deadlock on rtnl_mutex; triggered only
 once so far (extract):
 ...
 It seems so far plausible that the recursive call into rtnetlink_rcv()
 looks suspicious. One way, where this could trigger is that the senders
 NETLINK_CB(skb).portid was wrongly 0 (which is rtnetlink socket), so
 the rtnl_getlink() request's answer would be sent to the kernel instead
 to the actual user process, thus grabbing rtnl_mutex() twice.
 
 One theory would be that netlink_autobind() triggered via netlink_sendmsg()
 internally overwrites the -EBUSY error to 0, but where it is wrongly
 originating from __netlink_insert() instead. That would reset the
 socket's portid to 0, which is then filled into NETLINK_CB(skb).portid
 later on. As commit d470e3b483dc ([NETLINK]: Fix two socket hashing bugs.)
 also puts it, -EBUSY should not be propagated from netlink_insert().
 
 It looks like it's very unlikely to reproduce. We need to trigger the
 rhashtable_insert_rehash() handler under a situation where rehashing
 currently occurs (one /rare/ way would be to hit ht-elasticity limits
 while not filled enough to expand the hashtable, but that would rather
 require a specifically crafted bind() sequence with knowledge about
 destination slots, seems unlikely). It probably makes sense to guard
 __netlink_insert() in any case and remap that error. It was suggested
 that EOVERFLOW might be better than an already overloaded ENOMEM.
 
 Reference: http://thread.gmane.org/gmane.linux.network/372676
 Reported-by: Linus Torvalds torva...@linux-foundation.org
 Signed-off-by: Daniel Borkmann dan...@iogearbox.net

Applied and queued up for -stable, thanks.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] netlink: make sure -EBUSY won't escape from netlink_insert

[Thomas Graf]

On 08/07/15 at 12:26am, Daniel Borkmann wrote:
 Linus reports the following deadlock on rtnl_mutex; triggered only
 once so far (extract):
 
[...] 
 Reference: http://thread.gmane.org/gmane.linux.network/372676
 Reported-by: Linus Torvalds torva...@linux-foundation.org
 Signed-off-by: Daniel Borkmann dan...@iogearbox.net

Acked-by: Thomas Graf tg...@suug.ch
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] netlink: make sure -EBUSY won't escape from netlink_insert

[Herbert Xu]

On Fri, Aug 07, 2015 at 12:26:41AM +0200, Daniel Borkmann wrote:

 Reference: http://thread.gmane.org/gmane.linux.network/372676
 Reported-by: Linus Torvalds torva...@linux-foundation.org
 Signed-off-by: Daniel Borkmann dan...@iogearbox.net

Acked-by: Herbert Xu herb...@gondor.apana.org.au

But as I said earlier, this should not happen and if it does,
then our rhashtable is setup is broken.

Cheers,
-- 
Email: Herbert Xu herb...@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 netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html