[PATCH net 1/1] net sched actions: fix dumping which requires several messages to user space

Subsystems: networking [general], tc subsystem, the rest

STALE3129d

3 messages, 3 authors, 2018-03-27 · open the first message on its own page

[PATCH net 1/1] net sched actions: fix dumping which requires several messages to user space

From: Craig Dillabaugh <hidden>
Date: 2018-03-26 18:58:41

    Fixes a bug in the tcf_dump_walker function that can cause some actions
    to not be reported when dumping a large number of actions. This issue
    became more aggrevated when cookies feature was added. In particular
    this issue is manifest when large cookie values are assigned to the
    actions and when enough actions are created that the resulting table
    must be dumped in multiple batches.

    The number of actions returned in each batch is limited by the total
    number of actions and the memory buffer size.  With small cookies
    the numeric limit is reached before the buffer size limit, which avoids
    the code path triggering this bug. When large cookies are used buffer
    fills before the numeric limit, and the erroneous code path is hit.

    For example after creating 32 csum actions with the cookie
    aaaabbbbccccdddd

    $ tc actions ls action csum
    total acts 26

        action order 0: csum (tcp) action continue
        index 1 ref 1 bind 0
        cookie aaaabbbbccccdddd

        .....

        action order 25: csum (tcp) action continue
        index 26 ref 1 bind 0
        cookie aaaabbbbccccdddd
    total acts 6

        action order 0: csum (tcp) action continue
        index 28 ref 1 bind 0
        cookie aaaabbbbccccdddd

        ......

        action order 5: csum (tcp) action continue
        index 32 ref 1 bind 0
        cookie aaaabbbbccccdddd

    Note that the action with index 27 is omitted from the report.

Fixes: 4b3550ef530c ("[NET_SCHED]: Use nla_nest_start/nla_nest_end")"
Signed-off-by: Craig Dillabaugh <redacted>
---
 net/sched/act_api.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index eba6682..efc6bfb 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -135,8 +135,10 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
 			continue;
 
 		nest = nla_nest_start(skb, n_i);
-		if (!nest)
+		if (!nest) {
+			index--;
 			goto nla_put_failure;
+		}
 		err = tcf_action_dump_1(skb, p, 0, 0);
 		if (err < 0) {
 			index--;
-- 
1.9.1

Re: [PATCH net 1/1] net sched actions: fix dumping which requires several messages to user space

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2018-03-27 11:26:19

On 18-03-26 02:58 PM, Craig Dillabaugh wrote:
     Fixes a bug in the tcf_dump_walker function that can cause some actions
     to not be reported when dumping a large number of actions. This issue
     became more aggrevated when cookies feature was added. In particular
     this issue is manifest when large cookie values are assigned to the
     actions and when enough actions are created that the resulting table
     must be dumped in multiple batches.

     The number of actions returned in each batch is limited by the total
     number of actions and the memory buffer size.  With small cookies
     the numeric limit is reached before the buffer size limit, which avoids
     the code path triggering this bug. When large cookies are used buffer
     fills before the numeric limit, and the erroneous code path is hit.

     For example after creating 32 csum actions with the cookie
     aaaabbbbccccdddd

     $ tc actions ls action csum
     total acts 26

         action order 0: csum (tcp) action continue
         index 1 ref 1 bind 0
         cookie aaaabbbbccccdddd

         .....

         action order 25: csum (tcp) action continue
         index 26 ref 1 bind 0
         cookie aaaabbbbccccdddd
     total acts 6

         action order 0: csum (tcp) action continue
         index 28 ref 1 bind 0
         cookie aaaabbbbccccdddd

         ......

         action order 5: csum (tcp) action continue
         index 32 ref 1 bind 0
         cookie aaaabbbbccccdddd

     Note that the action with index 27 is omitted from the report.

Fixes: 4b3550ef530c ("[NET_SCHED]: Use nla_nest_start/nla_nest_end")"
Signed-off-by: Craig Dillabaugh <redacted>
Good catch.
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [PATCH net 1/1] net sched actions: fix dumping which requires several messages to user space

From: David Miller <davem@davemloft.net>
Date: 2018-03-27 14:59:47

From: Craig Dillabaugh <redacted>
Date: Mon, 26 Mar 2018 14:58:32 -0400
    Fixes a bug in the tcf_dump_walker function that can cause some actions
    to not be reported when dumping a large number of actions. This issue
    became more aggrevated when cookies feature was added. In particular
    this issue is manifest when large cookie values are assigned to the
    actions and when enough actions are created that the resulting table
    must be dumped in multiple batches.

    The number of actions returned in each batch is limited by the total
    number of actions and the memory buffer size.  With small cookies
    the numeric limit is reached before the buffer size limit, which avoids
    the code path triggering this bug. When large cookies are used buffer
    fills before the numeric limit, and the erroneous code path is hit.

    For example after creating 32 csum actions with the cookie
    aaaabbbbccccdddd
 ...
    Note that the action with index 27 is omitted from the report.

Fixes: 4b3550ef530c ("[NET_SCHED]: Use nla_nest_start/nla_nest_end")"
Signed-off-by: Craig Dillabaugh <redacted>
Applied and queued up for -stable.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help