Re: [PATCH] fix dst_entry leak in icmp_push_reply()

Patrick McHardy
2005-08-18T18:59:38Z

[-- Attachment #1: Type: text/plain, Size: 634 bytes --]

Ollie Wild wrote:
> That said, I appreciate that the if-else condition doesn't seem quite
> right. The problem is, the icmp_push_reply() routine is implicitly
> using the queue as a success indicator. I put the
> ip_flush_pending_frames() call inside the else block because I wanted to
> guarantee that one of ip_push_pending_frames() and
> ip_flush_pending_frames() is always called. Both will do proper cleanup.
>
> I'm open to suggestions if you think there's a cleaner way to implement
> this.

Checking the return value of ip_append_data seems cleaner to me.
Patch attached.

Signed-off-by: Patrick McHardy <kaber@trash.net>

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 844 bytes --]

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -349,12 +349,12 @@ static void icmp_push_reply(struct icmp_
{
struct sk_buff *skb;

- ip_append_data(icmp_socket->sk, icmp_glue_bits, icmp_param,
- icmp_param->data_len+icmp_param->head_len,
- icmp_param->head_len,
- ipc, rt, MSG_DONTWAIT);
-
- if ((skb = skb_peek(&icmp_socket->sk->sk_write_queue)) != NULL) {
+ if (ip_append_data(icmp_socket->sk, icmp_glue_bits, icmp_param,
+ icmp_param->data_len+icmp_param->head_len,
+ icmp_param->head_len,
+ ipc, rt, MSG_DONTWAIT) < 0)
+ ip_flush_pending_frames(icmp_socket->sk);
+ else if ((skb = skb_peek(&icmp_socket->sk->sk_write_queue)) != NULL) {
struct icmphdr *icmph = skb->h.icmph;
unsigned int csum = 0;
struct sk_buff *skb1;