2006-05-25 00:08:27

by Florin Malita

Subject: [PATCH] irda: missing allocation result check in irlap_change_speed()

The skb allocation may fail, which can result in a NULL pointer
dereference in irlap_queue_xmit().

Coverity CID: 434.

Signed-off-by: Florin Malita <[email protected]>
---

diff --git a/net/irda/irlap.c b/net/irda/irlap.c
index 7029618..a165286 100644
--- a/net/irda/irlap.c
+++ b/net/irda/irlap.c
@@ -884,7 +884,8 @@ static void irlap_change_speed(struct ir
if (now) {
/* Send down empty frame to trigger speed change */
skb = dev_alloc_skb(0);
- irlap_queue_xmit(self, skb);
+ if (skb)
+ irlap_queue_xmit(self, skb);
}
}

2006-05-25 23:22:18

by David Miller

Subject: Re: [PATCH] irda: missing allocation result check in irlap_change_speed()

From: Florin Malita <[email protected]>
Date: Wed, 24 May 2006 20:13:26 -0400

> The skb allocation may fail, which can result in a NULL pointer
> dereference in irlap_queue_xmit().
>
> Coverity CID: 434.
>
> Signed-off-by: Florin Malita <[email protected]>

If the allocation fails we should probably do something
more interesting here, such as schedule a timer to try
again later. Otherwise the speed change will silently
never occur.

2006-05-26 00:10:27

by Florin Malita

Subject: Re: [PATCH] irda: missing allocation result check in irlap_change_speed()

David Miller wrote:
> If the allocation fails we should probably do something
> more interesting here, such as schedule a timer to try
> again later. Otherwise the speed change will silently
> never occur.
>
I thought the speed change would be piggybacked on the next frame, same
as when 'now' == 0. Is that not the case?

self->speed = speed;

/* Change speed now, or just piggyback speed on frames */
if (now) {
/* Send down empty frame to trigger speed change */
skb = dev_alloc_skb(0);
if (skb)
irlap_queue_xmit(self, skb);
}