Commit 5cbdbd1a1f30a083aada44595ca42952fc31e866
Committed by
David S. Miller
David S. Miller
1 parent
a337531b94
tipc: refactor function tipc_msg_reverse()
The function tipc_msg_reverse() is reversing the header of a message while reusing the original buffer. We have seen at several occasions that this may have unfortunate side effects when the buffer to be reversed is a clone. In one of the following commits we will again need to reverse cloned buffers, so this is the right time to permanently eliminate this problem. In this commit we let the said function always consume the original buffer and replace it with a new one when applicable. Acked-by: Ying Xue <ying.xue@windriver.com> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 1 changed file with 28 additions and 30 deletions Side-by-side Diff
net/tipc/msg.c
| ... | ... | @@ -499,54 +499,52 @@ |
| 499 | 499 | /** |
| 500 | 500 | * tipc_msg_reverse(): swap source and destination addresses and add error code |
| 501 | 501 | * @own_node: originating node id for reversed message |
| 502 | - * @skb: buffer containing message to be reversed; may be replaced. | |
| 502 | + * @skb: buffer containing message to be reversed; will be consumed | |
| 503 | 503 | * @err: error code to be set in message, if any |
| 504 | - * Consumes buffer at failure | |
| 504 | + * Replaces consumed buffer with new one when successful | |
| 505 | 505 | * Returns true if success, otherwise false |
| 506 | 506 | */ |
| 507 | 507 | bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, int err) |
| 508 | 508 | { |
| 509 | 509 | struct sk_buff *_skb = *skb; |
| 510 | - struct tipc_msg *hdr; | |
| 511 | - struct tipc_msg ohdr; | |
| 512 | - int dlen; | |
| 510 | + struct tipc_msg *_hdr, *hdr; | |
| 511 | + int hlen, dlen; | |
| 513 | 512 | |
| 514 | 513 | if (skb_linearize(_skb)) |
| 515 | 514 | goto exit; |
| 516 | - hdr = buf_msg(_skb); | |
| 517 | - dlen = min_t(uint, msg_data_sz(hdr), MAX_FORWARD_SIZE); | |
| 518 | - if (msg_dest_droppable(hdr)) | |
| 515 | + _hdr = buf_msg(_skb); | |
| 516 | + dlen = min_t(uint, msg_data_sz(_hdr), MAX_FORWARD_SIZE); | |
| 517 | + hlen = msg_hdr_sz(_hdr); | |
| 518 | + | |
| 519 | + if (msg_dest_droppable(_hdr)) | |
| 519 | 520 | goto exit; |
| 520 | - if (msg_errcode(hdr)) | |
| 521 | + if (msg_errcode(_hdr)) | |
| 521 | 522 | goto exit; |
| 522 | 523 | |
| 523 | - /* Take a copy of original header before altering message */ | |
| 524 | - memcpy(&ohdr, hdr, msg_hdr_sz(hdr)); | |
| 524 | + /* Never return SHORT header */ | |
| 525 | + if (hlen == SHORT_H_SIZE) | |
| 526 | + hlen = BASIC_H_SIZE; | |
| 525 | 527 | |
| 526 | - /* Never return SHORT header; expand by replacing buffer if necessary */ | |
| 527 | - if (msg_short(hdr)) { | |
| 528 | - *skb = tipc_buf_acquire(BASIC_H_SIZE + dlen, GFP_ATOMIC); | |
| 529 | - if (!*skb) | |
| 530 | - goto exit; | |
| 531 | - memcpy((*skb)->data + BASIC_H_SIZE, msg_data(hdr), dlen); | |
| 532 | - kfree_skb(_skb); | |
| 533 | - _skb = *skb; | |
| 534 | - hdr = buf_msg(_skb); | |
| 535 | - memcpy(hdr, &ohdr, BASIC_H_SIZE); | |
| 536 | - msg_set_hdr_sz(hdr, BASIC_H_SIZE); | |
| 537 | - } | |
| 528 | + /* Allocate new buffer to return */ | |
| 529 | + *skb = tipc_buf_acquire(hlen + dlen, GFP_ATOMIC); | |
| 530 | + if (!*skb) | |
| 531 | + goto exit; | |
| 532 | + memcpy((*skb)->data, _skb->data, msg_hdr_sz(_hdr)); | |
| 533 | + memcpy((*skb)->data + hlen, msg_data(_hdr), dlen); | |
| 538 | 534 | |
| 539 | - /* Now reverse the concerned fields */ | |
| 535 | + /* Build reverse header in new buffer */ | |
| 536 | + hdr = buf_msg(*skb); | |
| 537 | + msg_set_hdr_sz(hdr, hlen); | |
| 540 | 538 | msg_set_errcode(hdr, err); |
| 541 | 539 | msg_set_non_seq(hdr, 0); |
| 542 | - msg_set_origport(hdr, msg_destport(&ohdr)); | |
| 543 | - msg_set_destport(hdr, msg_origport(&ohdr)); | |
| 544 | - msg_set_destnode(hdr, msg_prevnode(&ohdr)); | |
| 540 | + msg_set_origport(hdr, msg_destport(_hdr)); | |
| 541 | + msg_set_destport(hdr, msg_origport(_hdr)); | |
| 542 | + msg_set_destnode(hdr, msg_prevnode(_hdr)); | |
| 545 | 543 | msg_set_prevnode(hdr, own_node); |
| 546 | 544 | msg_set_orignode(hdr, own_node); |
| 547 | - msg_set_size(hdr, msg_hdr_sz(hdr) + dlen); | |
| 548 | - skb_trim(_skb, msg_size(hdr)); | |
| 545 | + msg_set_size(hdr, hlen + dlen); | |
| 549 | 546 | skb_orphan(_skb); |
| 547 | + kfree_skb(_skb); | |
| 550 | 548 | return true; |
| 551 | 549 | exit: |
| 552 | 550 | kfree_skb(_skb); |