fib_rules: Fix dump_rules() not to exit early

PATCH Applied Roland Dreier 7 participants 8 messages Oct 5, 2015 – Oct 19, 2015

Subsystems (4)

Networking Network drivers Stable branch IPv4/IPv6

lore

Share & tools

lore

Roland Dreier

author

2015-10-05 17:29 UTC #0 lore raw

PATCH 1/1 fib_rules: Fix dump_rules() not to exit early

From: Roland Dreier <[email protected]>

Backports of 41fc014332d9 ("fib_rules: fix fib rule dumps across multiple skbs") introduced a regression in "ip rule show" - it ends up dumping the first rule over and over and never exiting, because 3.19 and earlier are missing commit 053c095a82cf ("netlink: make nlmsg_end() and genlmsg_end() void"), so fib_nl_fill_rule() ends up returning skb->len (i.e. > 0) in the success case.

Fix this by checking the return code for < 0 instead of != 0.

Signed-off-by Roland Dreier <[email protected]>

---
Hi, this is needed for all stable trees earlier than 4.0 that have picked up 41fc014332d9; so far looks like at least 3.10.y and 3.14.y have made such releases.

 net/core/fib_rules.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/core/fib_rules.c +1 -1

index 627e517077e4..84340a2605ed 100644
@@ -606,7 +606,7 @@ static int dump_rules(struct sk_buff *skb, struct netlink_callback *cb,
 		err = fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid,
 				       cb->nlh->nlmsg_seq, RTM_NEWRULE,
 				       NLM_F_MULTI, ops);
-		if (err)
+		if (err < 0)
 			break;
 skip:
 		idx++;

Signature · 1 line

2.5.0

Jiri Slaby

2015-10-05 21:50 UTC #1 lore raw

Quoted 25 lines · Roland Dreier

From: Roland Dreier <[email protected]>
Backports of 41fc014332d9 ("fib_rules: fix fib rule dumps across multiple skbs") introduced a regression in "ip rule show" - it ends up dumping the first rule over and over and never exiting, because 3.19 and earlier are missing commit 053c095a82cf ("netlink: make nlmsg_end() and genlmsg_end() void"), so fib_nl_fill_rule() ends up returning skb->len (i.e. > 0) in the success case.
Fix this by checking the return code for < 0 instead of != 0.
Signed-off-by Roland Dreier <[email protected]>
---
Hi, this is needed for all stable trees earlier than 4.0 that have picked up 41fc014332d9; so far looks like at least 3.10.y and 3.14.y have made such releases.
 net/core/fib_rules.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/core/fib_rules.c +0 -0
index 627e517077e4..84340a2605ed 100644

net/core/fib_rules.c static int dump_rules(struct sk_buff *skb, struct netlink_callback *cb, · line 609

Quoted 5 lines

net/core/fib_rules.c +1 -1

@@ -606,7 +606,7 @@ static int dump_rules(struct sk_buff *skb, struct netlink_callback *cb,
 		err = fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid,
 				       cb->nlh->nlmsg_seq, RTM_NEWRULE,
 				       NLM_F_MULTI, ops);
-		if (err)
+		if (err < 0)

I integrated the fix into the backport of 41fc014332d9 in 3.12 and made a note in there. Thanks!

Signature · 2 lines

js
suse labs

Ben Hutchings

2015-10-08 12:50 UTC #2 lore raw

In reply to Roland Dreier

Quoted 9 lines · Roland Dreier

From: Roland Dreier <[email protected]>
Backports of 41fc014332d9 ("fib_rules: fix fib rule dumps across multiple skbs") introduced a regression in "ip rule show" - it ends up dumping the first rule over and over and never exiting, because 3.19 and earlier are missing commit 053c095a82cf ("netlink: make nlmsg_end() and genlmsg_end() void"), so fib_nl_fill_rule() ends up returning skb->len (i.e. > 0) in the success case.

Quoted 7 lines

Fix this by checking the return code for < 0 instead of != 0.
Signed-off-by Roland Dreier <[email protected]>
---
Hi, this is needed for all stable trees earlier than 4.0 that have picked up 41fc014332d9; so far looks like at least 3.10.y and 3.14.y have made such releases.

[...]

Thanks, I'm folding this into the backported patch in 3.2.

Ben.

Signature · 2 lines

Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

Thomas Jarosch

2015-10-09 14:43 UTC #3 lore raw

In reply to Roland Dreier

Hi Roland,

Quoted 10 lines · On Monday, 5. October 2015 10:29:28 Roland Dreier wrote:

From: Roland Dreier <[email protected]>
Backports of 41fc014332d9 ("fib_rules: fix fib rule dumps across multiple skbs") introduced a regression in "ip rule show" - it ends up dumping the first rule over and over and never exiting, because 3.19 and earlier are missing commit 053c095a82cf ("netlink: make nlmsg_end() and genlmsg_end() void"), so fib_nl_fill_rule() ends up returning skb->len (i.e. > 0) in the success case.
Fix this by checking the return code for < 0 instead of != 0.

thanks for this fix. You just saved me an afternoon of bisecting :)

I can confirm that this fixes the mentioned issue introduced in 3.14.54. We have an automated ipsec VPN test that failed after the upgrade:
The "ip rule list" command was hanging forever.

Cheers,
Thomas

Greg KH

2015-10-18 00:12 UTC #4 lore raw

In reply to Roland Dreier

Quoted 9 lines · Roland Dreier

From: Roland Dreier <[email protected]>
Backports of 41fc014332d9 ("fib_rules: fix fib rule dumps across multiple skbs") introduced a regression in "ip rule show" - it ends up dumping the first rule over and over and never exiting, because 3.19 and earlier are missing commit 053c095a82cf ("netlink: make nlmsg_end() and genlmsg_end() void"), so fib_nl_fill_rule() ends up returning skb->len (i.e. > 0) in the success case.

Quoted 7 lines

Fix this by checking the return code for < 0 instead of != 0.
Signed-off-by Roland Dreier <[email protected]>
---
Hi, this is needed for all stable trees earlier than 4.0 that have picked up 41fc014332d9; so far looks like at least 3.10.y and 3.14.y have made such releases.

Thanks for this, now applied to 3.10 and 3.14-stable trees.

greg k-h

[email protected]

maintainer Applied

2015-10-18 00:34 UTC #5 lore raw

Patch "fib_rules: Fix dump_rules() not to exit early" has been added to the 3.14-stable tree

In reply to Roland Dreier

This is a note to let you know that I've just added the patch titled

fib_rules: Fix dump_rules() not to exit early

to the 3.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
fib_rules-fix-dump_rules-not-to-exit-early.patch
and it can be found in the queue-3.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From [email protected] Sat Oct 17 17:11:23 2015

From: Roland Dreier <[email protected]>
Date: Mon, 5 Oct 2015 10:29:28 -0700
Subject: fib_rules: Fix dump_rules() not to exit early
To: [email protected], [email protected]
Cc: Roland Dreier <[email protected]>
Message-ID: <[email protected]>

From: Roland Dreier <[email protected]>

Fix this by checking the return code for < 0 instead of != 0.

Signed-off-by Roland Dreier <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 net/core/fib_rules.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/core/fib_rules.c +1 -1

@@ -631,7 +631,7 @@ static int dump_rules(struct sk_buff *sk
 		err = fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid,
 				       cb->nlh->nlmsg_seq, RTM_NEWRULE,
 				       NLM_F_MULTI, ops);
-		if (err)
+		if (err < 0)
 			break;
 skip:
 		idx++;

Patches currently in stable-queue which might be from [email protected] are

queue-3.14/fib_rules-fix-dump_rules-not-to-exit-early.patch

[email protected]

maintainer Applied

2015-10-18 00:51 UTC #6 lore raw

Patch "fib_rules: Fix dump_rules() not to exit early" has been added to the 3.10-stable tree

In reply to Roland Dreier

This is a note to let you know that I've just added the patch titled

fib_rules: Fix dump_rules() not to exit early

to the 3.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
fib_rules-fix-dump_rules-not-to-exit-early.patch
and it can be found in the queue-3.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From [email protected] Sat Oct 17 17:11:23 2015

From: Roland Dreier <[email protected]>

Fix this by checking the return code for < 0 instead of != 0.

Signed-off-by Roland Dreier <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 net/core/fib_rules.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/core/fib_rules.c +1 -1

@@ -606,7 +606,7 @@ static int dump_rules(struct sk_buff *sk
 		err = fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid,
 				       cb->nlh->nlmsg_seq, RTM_NEWRULE,
 				       NLM_F_MULTI, ops);
-		if (err)
+		if (err < 0)
 			break;
 skip:
 		idx++;

Patches currently in stable-queue which might be from [email protected] are

queue-3.10/fib_rules-fix-dump_rules-not-to-exit-early.patch

Luis Henriques

2015-10-19 10:24 UTC #7 lore raw

In reply to Roland Dreier

Quoted 9 lines · Roland Dreier

From: Roland Dreier <[email protected]>
Backports of 41fc014332d9 ("fib_rules: fix fib rule dumps across multiple skbs") introduced a regression in "ip rule show" - it ends up dumping the first rule over and over and never exiting, because 3.19 and earlier are missing commit 053c095a82cf ("netlink: make nlmsg_end() and genlmsg_end() void"), so fib_nl_fill_rule() ends up returning skb->len (i.e. > 0) in the success case.

Quoted 8 lines

Fix this by checking the return code for < 0 instead of != 0.
Signed-off-by Roland Dreier <[email protected]>
---
Hi, this is needed for all stable trees earlier than 4.0 that have picked up 41fc014332d9; so far looks like at least 3.10.y and 3.14.y have made such releases.

Thanks Roland, I'm queuing this fix for the 3.16 kernel as well.

Cheers,
--
Luís

Quoted 23 lines

 net/core/fib_rules.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/core/fib_rules.c +1 -1

index 627e517077e4..84340a2605ed 100644
@@ -606,7 +606,7 @@ static int dump_rules(struct sk_buff *skb, struct netlink_callback *cb,
 		err = fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid,
 				       cb->nlh->nlmsg_seq, RTM_NEWRULE,
 				       NLM_F_MULTI, ops);
-		if (err)
+		if (err < 0)
 			break;
 skip:
 		idx++;

Signature · 6 lines

2.5.0

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html