Commit 13e9b9972fa0f34059e737ae215a26e43966b46f
Committed by
David S. Miller
1 parent
3fd0202a0d
tipc: make tipc_buf_append() more robust
As per comment from David Miller, we try to make the buffer reassembly function more resilient to user errors than it is today. - We check that the "*buf" parameter always is set, since this is mandatory input. - We ensure that *buf->next always is set to NULL before linking in the buffer, instead of relying of the caller to have done this. - We ensure that the "tail" pointer in the head buffer's control block is initialized to NULL when the first fragment arrives. Signed-off-by: Jon Maloy <jon.maloy@ericsson.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 1 changed file with 21 additions and 8 deletions Side-by-side Diff
net/tipc/msg.c
... | ... | @@ -72,27 +72,38 @@ |
72 | 72 | struct sk_buff *head = *headbuf; |
73 | 73 | struct sk_buff *frag = *buf; |
74 | 74 | struct sk_buff *tail; |
75 | - struct tipc_msg *msg = buf_msg(frag); | |
76 | - u32 fragid = msg_type(msg); | |
77 | - bool headstolen; | |
75 | + struct tipc_msg *msg; | |
76 | + u32 fragid; | |
78 | 77 | int delta; |
78 | + bool headstolen; | |
79 | 79 | |
80 | + if (!frag) | |
81 | + goto err; | |
82 | + | |
83 | + msg = buf_msg(frag); | |
84 | + fragid = msg_type(msg); | |
85 | + frag->next = NULL; | |
80 | 86 | skb_pull(frag, msg_hdr_sz(msg)); |
81 | 87 | |
82 | 88 | if (fragid == FIRST_FRAGMENT) { |
83 | - if (head || skb_unclone(frag, GFP_ATOMIC)) | |
84 | - goto out_free; | |
89 | + if (unlikely(head)) | |
90 | + goto err; | |
91 | + if (unlikely(skb_unclone(frag, GFP_ATOMIC))) | |
92 | + goto err; | |
85 | 93 | head = *headbuf = frag; |
86 | 94 | skb_frag_list_init(head); |
95 | + TIPC_SKB_CB(head)->tail = NULL; | |
87 | 96 | *buf = NULL; |
88 | 97 | return 0; |
89 | 98 | } |
99 | + | |
90 | 100 | if (!head) |
91 | - goto out_free; | |
92 | - tail = TIPC_SKB_CB(head)->tail; | |
101 | + goto err; | |
102 | + | |
93 | 103 | if (skb_try_coalesce(head, frag, &headstolen, &delta)) { |
94 | 104 | kfree_skb_partial(frag, headstolen); |
95 | 105 | } else { |
106 | + tail = TIPC_SKB_CB(head)->tail; | |
96 | 107 | if (!skb_has_frag_list(head)) |
97 | 108 | skb_shinfo(head)->frag_list = frag; |
98 | 109 | else |
... | ... | @@ -102,6 +113,7 @@ |
102 | 113 | head->len += frag->len; |
103 | 114 | TIPC_SKB_CB(head)->tail = frag; |
104 | 115 | } |
116 | + | |
105 | 117 | if (fragid == LAST_FRAGMENT) { |
106 | 118 | *buf = head; |
107 | 119 | TIPC_SKB_CB(head)->tail = NULL; |
... | ... | @@ -110,7 +122,8 @@ |
110 | 122 | } |
111 | 123 | *buf = NULL; |
112 | 124 | return 0; |
113 | -out_free: | |
125 | + | |
126 | +err: | |
114 | 127 | pr_warn_ratelimited("Unable to build fragment list\n"); |
115 | 128 | kfree_skb(*buf); |
116 | 129 | kfree_skb(*headbuf); |