Commit c85c2951d4da1236e32f1858db418221e624aba5
Committed by
David S. Miller
1 parent
bee925db9a
Exists in
master
and in
4 other branches
caif: Handle dev_queue_xmit errors.
Do proper handling of dev_queue_xmit errors in order to avoid double free of skb and leaks in error conditions. In cfctrl pending requests are removed when CAIF Link layer goes down. Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 7 changed files with 119 additions and 47 deletions Side-by-side Diff
include/net/caif/cfctrl.h
... | ... | @@ -124,7 +124,8 @@ |
124 | 124 | |
125 | 125 | struct cflayer *cfctrl_create(void); |
126 | 126 | struct cfctrl_rsp *cfctrl_get_respfuncs(struct cflayer *layer); |
127 | -void cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer); | |
127 | +int cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer); | |
128 | +void cfctrl_remove(struct cflayer *layr); | |
128 | 129 | |
129 | 130 | #endif /* CFCTRL_H_ */ |
net/caif/caif_dev.c
... | ... | @@ -118,6 +118,7 @@ |
118 | 118 | |
119 | 119 | static int transmit(struct cflayer *layer, struct cfpkt *pkt) |
120 | 120 | { |
121 | + int err; | |
121 | 122 | struct caif_device_entry *caifd = |
122 | 123 | container_of(layer, struct caif_device_entry, layer); |
123 | 124 | struct sk_buff *skb; |
124 | 125 | |
... | ... | @@ -125,9 +126,11 @@ |
125 | 126 | skb = cfpkt_tonative(pkt); |
126 | 127 | skb->dev = caifd->netdev; |
127 | 128 | |
128 | - dev_queue_xmit(skb); | |
129 | + err = dev_queue_xmit(skb); | |
130 | + if (err > 0) | |
131 | + err = -EIO; | |
129 | 132 | |
130 | - return 0; | |
133 | + return err; | |
131 | 134 | } |
132 | 135 | |
133 | 136 | /* |
net/caif/caif_socket.c
... | ... | @@ -604,7 +604,9 @@ |
604 | 604 | goto err; |
605 | 605 | ret = transmit_skb(skb, cf_sk, noblock, timeo); |
606 | 606 | if (ret < 0) |
607 | - goto err; | |
607 | + /* skb is already freed */ | |
608 | + return ret; | |
609 | + | |
608 | 610 | return len; |
609 | 611 | err: |
610 | 612 | kfree_skb(skb); |
611 | 613 | |
... | ... | @@ -933,9 +935,9 @@ |
933 | 935 | * caif_queue_rcv_skb checks SOCK_DEAD holding the queue lock, |
934 | 936 | * this ensures no packets when sock is dead. |
935 | 937 | */ |
936 | - spin_lock(&sk->sk_receive_queue.lock); | |
938 | + spin_lock_bh(&sk->sk_receive_queue.lock); | |
937 | 939 | sock_set_flag(sk, SOCK_DEAD); |
938 | - spin_unlock(&sk->sk_receive_queue.lock); | |
940 | + spin_unlock_bh(&sk->sk_receive_queue.lock); | |
939 | 941 | sock->sk = NULL; |
940 | 942 | |
941 | 943 | dbfs_atomic_inc(&cnt.num_disconnect); |
net/caif/cfcnfg.c
net/caif/cfctrl.c
... | ... | @@ -17,7 +17,6 @@ |
17 | 17 | #define UTILITY_NAME_LENGTH 16 |
18 | 18 | #define CFPKT_CTRL_PKT_LEN 20 |
19 | 19 | |
20 | - | |
21 | 20 | #ifdef CAIF_NO_LOOP |
22 | 21 | static int handle_loop(struct cfctrl *ctrl, |
23 | 22 | int cmd, struct cfpkt *pkt){ |
24 | 23 | |
25 | 24 | |
26 | 25 | |
... | ... | @@ -51,13 +50,29 @@ |
51 | 50 | this->serv.layer.receive = cfctrl_recv; |
52 | 51 | sprintf(this->serv.layer.name, "ctrl"); |
53 | 52 | this->serv.layer.ctrlcmd = cfctrl_ctrlcmd; |
53 | +#ifndef CAIF_NO_LOOP | |
54 | 54 | spin_lock_init(&this->loop_linkid_lock); |
55 | + this->loop_linkid = 1; | |
56 | +#endif | |
55 | 57 | spin_lock_init(&this->info_list_lock); |
56 | 58 | INIT_LIST_HEAD(&this->list); |
57 | - this->loop_linkid = 1; | |
58 | 59 | return &this->serv.layer; |
59 | 60 | } |
60 | 61 | |
62 | +void cfctrl_remove(struct cflayer *layer) | |
63 | +{ | |
64 | + struct cfctrl_request_info *p, *tmp; | |
65 | + struct cfctrl *ctrl = container_obj(layer); | |
66 | + | |
67 | + spin_lock_bh(&ctrl->info_list_lock); | |
68 | + list_for_each_entry_safe(p, tmp, &ctrl->list, list) { | |
69 | + list_del(&p->list); | |
70 | + kfree(p); | |
71 | + } | |
72 | + spin_unlock_bh(&ctrl->info_list_lock); | |
73 | + kfree(layer); | |
74 | +} | |
75 | + | |
61 | 76 | static bool param_eq(const struct cfctrl_link_param *p1, |
62 | 77 | const struct cfctrl_link_param *p2) |
63 | 78 | { |
64 | 79 | |
... | ... | @@ -116,11 +131,11 @@ |
116 | 131 | static void cfctrl_insert_req(struct cfctrl *ctrl, |
117 | 132 | struct cfctrl_request_info *req) |
118 | 133 | { |
119 | - spin_lock(&ctrl->info_list_lock); | |
134 | + spin_lock_bh(&ctrl->info_list_lock); | |
120 | 135 | atomic_inc(&ctrl->req_seq_no); |
121 | 136 | req->sequence_no = atomic_read(&ctrl->req_seq_no); |
122 | 137 | list_add_tail(&req->list, &ctrl->list); |
123 | - spin_unlock(&ctrl->info_list_lock); | |
138 | + spin_unlock_bh(&ctrl->info_list_lock); | |
124 | 139 | } |
125 | 140 | |
126 | 141 | /* Compare and remove request */ |
... | ... | @@ -129,7 +144,6 @@ |
129 | 144 | { |
130 | 145 | struct cfctrl_request_info *p, *tmp, *first; |
131 | 146 | |
132 | - spin_lock(&ctrl->info_list_lock); | |
133 | 147 | first = list_first_entry(&ctrl->list, struct cfctrl_request_info, list); |
134 | 148 | |
135 | 149 | list_for_each_entry_safe(p, tmp, &ctrl->list, list) { |
... | ... | @@ -145,7 +159,6 @@ |
145 | 159 | } |
146 | 160 | p = NULL; |
147 | 161 | out: |
148 | - spin_unlock(&ctrl->info_list_lock); | |
149 | 162 | return p; |
150 | 163 | } |
151 | 164 | |
... | ... | @@ -179,10 +192,6 @@ |
179 | 192 | cfpkt_addbdy(pkt, physlinkid); |
180 | 193 | ret = |
181 | 194 | cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt); |
182 | - if (ret < 0) { | |
183 | - pr_err("Could not transmit enum message\n"); | |
184 | - cfpkt_destroy(pkt); | |
185 | - } | |
186 | 195 | } |
187 | 196 | |
188 | 197 | int cfctrl_linkup_request(struct cflayer *layer, |
189 | 198 | |
... | ... | @@ -196,14 +205,23 @@ |
196 | 205 | struct cfctrl_request_info *req; |
197 | 206 | int ret; |
198 | 207 | char utility_name[16]; |
199 | - struct cfpkt *pkt = cfpkt_create(CFPKT_CTRL_PKT_LEN); | |
208 | + struct cfpkt *pkt; | |
209 | + | |
210 | + if (cfctrl_cancel_req(layer, user_layer) > 0) { | |
211 | + /* Slight Paranoia, check if already connecting */ | |
212 | + pr_err("Duplicate connect request for same client\n"); | |
213 | + WARN_ON(1); | |
214 | + return -EALREADY; | |
215 | + } | |
216 | + | |
217 | + pkt = cfpkt_create(CFPKT_CTRL_PKT_LEN); | |
200 | 218 | if (!pkt) { |
201 | 219 | pr_warn("Out of memory\n"); |
202 | 220 | return -ENOMEM; |
203 | 221 | } |
204 | 222 | cfpkt_addbdy(pkt, CFCTRL_CMD_LINK_SETUP); |
205 | - cfpkt_addbdy(pkt, (param->chtype << 4) + param->linktype); | |
206 | - cfpkt_addbdy(pkt, (param->priority << 3) + param->phyid); | |
223 | + cfpkt_addbdy(pkt, (param->chtype << 4) | param->linktype); | |
224 | + cfpkt_addbdy(pkt, (param->priority << 3) | param->phyid); | |
207 | 225 | cfpkt_addbdy(pkt, param->endpoint & 0x03); |
208 | 226 | |
209 | 227 | switch (param->linktype) { |
... | ... | @@ -266,9 +284,13 @@ |
266 | 284 | ret = |
267 | 285 | cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt); |
268 | 286 | if (ret < 0) { |
269 | - pr_err("Could not transmit linksetup request\n"); | |
270 | - cfpkt_destroy(pkt); | |
271 | - return -ENODEV; | |
287 | + int count; | |
288 | + | |
289 | + count = cfctrl_cancel_req(&cfctrl->serv.layer, | |
290 | + user_layer); | |
291 | + if (count != 1) | |
292 | + pr_err("Could not remove request (%d)", count); | |
293 | + return -ENODEV; | |
272 | 294 | } |
273 | 295 | return 0; |
274 | 296 | } |
275 | 297 | |
276 | 298 | |
277 | 299 | |
278 | 300 | |
279 | 301 | |
... | ... | @@ -288,28 +310,29 @@ |
288 | 310 | init_info(cfpkt_info(pkt), cfctrl); |
289 | 311 | ret = |
290 | 312 | cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt); |
291 | - if (ret < 0) { | |
292 | - pr_err("Could not transmit link-down request\n"); | |
293 | - cfpkt_destroy(pkt); | |
294 | - } | |
313 | +#ifndef CAIF_NO_LOOP | |
314 | + cfctrl->loop_linkused[channelid] = 0; | |
315 | +#endif | |
295 | 316 | return ret; |
296 | 317 | } |
297 | 318 | |
298 | -void cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer) | |
319 | +int cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer) | |
299 | 320 | { |
300 | 321 | struct cfctrl_request_info *p, *tmp; |
301 | 322 | struct cfctrl *ctrl = container_obj(layr); |
302 | - spin_lock(&ctrl->info_list_lock); | |
323 | + int found = 0; | |
324 | + spin_lock_bh(&ctrl->info_list_lock); | |
303 | 325 | |
304 | 326 | list_for_each_entry_safe(p, tmp, &ctrl->list, list) { |
305 | 327 | if (p->client_layer == adap_layer) { |
306 | - pr_debug("cancel req :%d\n", p->sequence_no); | |
307 | 328 | list_del(&p->list); |
308 | 329 | kfree(p); |
330 | + found++; | |
309 | 331 | } |
310 | 332 | } |
311 | 333 | |
312 | - spin_unlock(&ctrl->info_list_lock); | |
334 | + spin_unlock_bh(&ctrl->info_list_lock); | |
335 | + return found; | |
313 | 336 | } |
314 | 337 | |
315 | 338 | static int cfctrl_recv(struct cflayer *layer, struct cfpkt *pkt) |
... | ... | @@ -461,6 +484,7 @@ |
461 | 484 | |
462 | 485 | rsp.cmd = cmd; |
463 | 486 | rsp.param = linkparam; |
487 | + spin_lock_bh(&cfctrl->info_list_lock); | |
464 | 488 | req = cfctrl_remove_req(cfctrl, &rsp); |
465 | 489 | |
466 | 490 | if (CFCTRL_ERR_BIT == (CFCTRL_ERR_BIT & cmdrsp) || |
... | ... | @@ -480,6 +504,8 @@ |
480 | 504 | |
481 | 505 | if (req != NULL) |
482 | 506 | kfree(req); |
507 | + | |
508 | + spin_unlock_bh(&cfctrl->info_list_lock); | |
483 | 509 | } |
484 | 510 | break; |
485 | 511 | case CFCTRL_CMD_LINK_DESTROY: |
486 | 512 | |
487 | 513 | |
... | ... | @@ -523,12 +549,29 @@ |
523 | 549 | switch (ctrl) { |
524 | 550 | case _CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND: |
525 | 551 | case CAIF_CTRLCMD_FLOW_OFF_IND: |
526 | - spin_lock(&this->info_list_lock); | |
552 | + spin_lock_bh(&this->info_list_lock); | |
527 | 553 | if (!list_empty(&this->list)) { |
528 | 554 | pr_debug("Received flow off in control layer\n"); |
529 | 555 | } |
530 | - spin_unlock(&this->info_list_lock); | |
556 | + spin_unlock_bh(&this->info_list_lock); | |
531 | 557 | break; |
558 | + case _CAIF_CTRLCMD_PHYIF_DOWN_IND: { | |
559 | + struct cfctrl_request_info *p, *tmp; | |
560 | + | |
561 | + /* Find all connect request and report failure */ | |
562 | + spin_lock_bh(&this->info_list_lock); | |
563 | + list_for_each_entry_safe(p, tmp, &this->list, list) { | |
564 | + if (p->param.phyid == phyid) { | |
565 | + list_del(&p->list); | |
566 | + p->client_layer->ctrlcmd(p->client_layer, | |
567 | + CAIF_CTRLCMD_INIT_FAIL_RSP, | |
568 | + phyid); | |
569 | + kfree(p); | |
570 | + } | |
571 | + } | |
572 | + spin_unlock_bh(&this->info_list_lock); | |
573 | + break; | |
574 | + } | |
532 | 575 | default: |
533 | 576 | break; |
534 | 577 | } |
535 | 578 | |
536 | 579 | |
537 | 580 | |
538 | 581 | |
... | ... | @@ -538,27 +581,33 @@ |
538 | 581 | static int handle_loop(struct cfctrl *ctrl, int cmd, struct cfpkt *pkt) |
539 | 582 | { |
540 | 583 | static int last_linkid; |
584 | + static int dec; | |
541 | 585 | u8 linkid, linktype, tmp; |
542 | 586 | switch (cmd) { |
543 | 587 | case CFCTRL_CMD_LINK_SETUP: |
544 | - spin_lock(&ctrl->loop_linkid_lock); | |
545 | - for (linkid = last_linkid + 1; linkid < 255; linkid++) | |
546 | - if (!ctrl->loop_linkused[linkid]) | |
547 | - goto found; | |
588 | + spin_lock_bh(&ctrl->loop_linkid_lock); | |
589 | + if (!dec) { | |
590 | + for (linkid = last_linkid + 1; linkid < 255; linkid++) | |
591 | + if (!ctrl->loop_linkused[linkid]) | |
592 | + goto found; | |
593 | + } | |
594 | + dec = 1; | |
548 | 595 | for (linkid = last_linkid - 1; linkid > 0; linkid--) |
549 | 596 | if (!ctrl->loop_linkused[linkid]) |
550 | 597 | goto found; |
551 | - spin_unlock(&ctrl->loop_linkid_lock); | |
552 | - pr_err("Out of link-ids\n"); | |
553 | - return -EINVAL; | |
598 | + spin_unlock_bh(&ctrl->loop_linkid_lock); | |
599 | + | |
554 | 600 | found: |
601 | + if (linkid < 10) | |
602 | + dec = 0; | |
603 | + | |
555 | 604 | if (!ctrl->loop_linkused[linkid]) |
556 | 605 | ctrl->loop_linkused[linkid] = 1; |
557 | 606 | |
558 | 607 | last_linkid = linkid; |
559 | 608 | |
560 | 609 | cfpkt_add_trail(pkt, &linkid, 1); |
561 | - spin_unlock(&ctrl->loop_linkid_lock); | |
610 | + spin_unlock_bh(&ctrl->loop_linkid_lock); | |
562 | 611 | cfpkt_peek_head(pkt, &linktype, 1); |
563 | 612 | if (linktype == CFCTRL_SRV_UTIL) { |
564 | 613 | tmp = 0x01; |
565 | 614 | |
... | ... | @@ -568,10 +617,10 @@ |
568 | 617 | break; |
569 | 618 | |
570 | 619 | case CFCTRL_CMD_LINK_DESTROY: |
571 | - spin_lock(&ctrl->loop_linkid_lock); | |
620 | + spin_lock_bh(&ctrl->loop_linkid_lock); | |
572 | 621 | cfpkt_peek_head(pkt, &linkid, 1); |
573 | 622 | ctrl->loop_linkused[linkid] = 0; |
574 | - spin_unlock(&ctrl->loop_linkid_lock); | |
623 | + spin_unlock_bh(&ctrl->loop_linkid_lock); | |
575 | 624 | break; |
576 | 625 | default: |
577 | 626 | break; |
net/caif/cffrml.c
... | ... | @@ -33,7 +33,6 @@ |
33 | 33 | static u32 cffrml_rcv_error; |
34 | 34 | static u32 cffrml_rcv_checsum_error; |
35 | 35 | struct cflayer *cffrml_create(u16 phyid, bool use_fcs) |
36 | - | |
37 | 36 | { |
38 | 37 | struct cffrml *this = kmalloc(sizeof(struct cffrml), GFP_ATOMIC); |
39 | 38 | if (!this) { |
... | ... | @@ -128,6 +127,13 @@ |
128 | 127 | cfpkt_destroy(pkt); |
129 | 128 | return -EPROTO; |
130 | 129 | } |
130 | + | |
131 | + if (layr->up == NULL) { | |
132 | + pr_err("Layr up is missing!\n"); | |
133 | + cfpkt_destroy(pkt); | |
134 | + return -EINVAL; | |
135 | + } | |
136 | + | |
131 | 137 | return layr->up->receive(layr->up, pkt); |
132 | 138 | } |
133 | 139 | |
134 | 140 | |
135 | 141 | |
... | ... | @@ -150,15 +156,22 @@ |
150 | 156 | cfpkt_info(pkt)->hdr_len += 2; |
151 | 157 | if (cfpkt_erroneous(pkt)) { |
152 | 158 | pr_err("Packet is erroneous!\n"); |
159 | + cfpkt_destroy(pkt); | |
153 | 160 | return -EPROTO; |
154 | 161 | } |
162 | + | |
163 | + if (layr->dn == NULL) { | |
164 | + cfpkt_destroy(pkt); | |
165 | + return -ENODEV; | |
166 | + | |
167 | + } | |
155 | 168 | return layr->dn->transmit(layr->dn, pkt); |
156 | 169 | } |
157 | 170 | |
158 | 171 | static void cffrml_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl, |
159 | 172 | int phyid) |
160 | 173 | { |
161 | - if (layr->up->ctrlcmd) | |
174 | + if (layr->up && layr->up->ctrlcmd) | |
162 | 175 | layr->up->ctrlcmd(layr->up, ctrl, layr->id); |
163 | 176 | } |
164 | 177 |
net/caif/cfveil.c
... | ... | @@ -82,13 +82,14 @@ |
82 | 82 | int ret; |
83 | 83 | struct cfsrvl *service = container_obj(layr); |
84 | 84 | if (!cfsrvl_ready(service, &ret)) |
85 | - return ret; | |
85 | + goto err; | |
86 | 86 | caif_assert(layr->dn != NULL); |
87 | 87 | caif_assert(layr->dn->transmit != NULL); |
88 | 88 | |
89 | 89 | if (cfpkt_add_head(pkt, &tmp, 1) < 0) { |
90 | 90 | pr_err("Packet is erroneous!\n"); |
91 | - return -EPROTO; | |
91 | + ret = -EPROTO; | |
92 | + goto err; | |
92 | 93 | } |
93 | 94 | |
94 | 95 | /* Add info-> for MUX-layer to route the packet out. */ |
... | ... | @@ -97,5 +98,8 @@ |
97 | 98 | info->hdr_len = 1; |
98 | 99 | info->dev_info = &service->dev_info; |
99 | 100 | return layr->dn->transmit(layr->dn, pkt); |
101 | +err: | |
102 | + cfpkt_destroy(pkt); | |
103 | + return ret; | |
100 | 104 | } |