This patch adds a check on the return value of dev_alloc_skb() in
ieee80211_sta_join_ibss()
in net/mac80211/mlme.c.
The patch is to wireless-next-2.6.
Signed-off-by: Rami Rosen <[email protected]>
On Wed, 2008-10-22 at 09:00 +0200, Rami Rosen wrote:
> This patch adds a check on the return value of dev_alloc_skb() in
> ieee80211_sta_join_ibss()
> in net/mac80211/mlme.c.
What's wrong with the current code?
> + return -1;
don't return -1, return meaningful error values.
johannes
On Wed, 2008-10-22 at 09:42 +0200, Rami Rosen wrote:
> Hello,
>
> Do we want to continue as usual if dev_alloc_skb() in theis method fails?
well if anything then we shouldn't do half the stuff and then abort, so
the allocation/check should be moved to the beginning of the function...
johannes
Hello,
Do we want to continue as usual if dev_alloc_skb() in theis method fails?
Regards,
Rami
On Wed, Oct 22, 2008 at 9:36 AM, Johannes Berg
<[email protected]> wrote:
> On Wed, 2008-10-22 at 09:00 +0200, Rami Rosen wrote:
>> This patch adds a check on the return value of dev_alloc_skb() in
>> ieee80211_sta_join_ibss()
>> in net/mac80211/mlme.c.
>
> What's wrong with the current code?
>
>
>> + return -1;
>
> don't return -1, return meaningful error values.
>
> johannes
>
On Wed, Oct 22, 2008 at 09:42:37AM +0200, Rami Rosen wrote:
> Do we want to continue as usual if dev_alloc_skb() in theis method fails?
I think we should continue and as such, I would rather not see the patch
that proposes to abort here go in. The allocated skb is not used at this
point anyway; it is only used as an optimization to prepare a ProbeRsp
frame for possible future use. This could be done at the time when
ProbeReq is received (and we sent the last Beacon), i.e., only when
needed. Or if the optimization of generating this only once is
considered desirable, that place could try to allocate a new skb if the
one here failed (or do it on first need and cache the result).
--
Jouni Malinen PGP id EFC895FA
On Wed, 2008-10-22 at 11:11 +0300, Jouni Malinen wrote:
> On Wed, Oct 22, 2008 at 09:42:37AM +0200, Rami Rosen wrote:
>
> > Do we want to continue as usual if dev_alloc_skb() in theis method fails?
>
> I think we should continue and as such, I would rather not see the patch
> that proposes to abort here go in. The allocated skb is not used at this
> point anyway; it is only used as an optimization to prepare a ProbeRsp
> frame for possible future use. This could be done at the time when
> ProbeReq is received (and we sent the last Beacon), i.e., only when
> needed. Or if the optimization of generating this only once is
> considered desirable, that place could try to allocate a new skb if the
> one here failed (or do it on first need and cache the result).
Good point. We currently don't try again, but we could put this into a
new function and call it from when we need it. Then again, we'll need it
right away, the driver will probably call _get_beacon from
ieee80211_if_config(sdata, IEEE80211_IFCC_BEACON);
johannes
On Wed, Oct 22, 2008 at 10:17:33AM +0200, Johannes Berg wrote:
> Good point. We currently don't try again, but we could put this into a
> new function and call it from when we need it. Then again, we'll need it
> right away, the driver will probably call _get_beacon from
> ieee80211_if_config(sdata, IEEE80211_IFCC_BEACON);
Yes.. I forgot about that and only looked at probe_resp references
inside mlme.c. With that in mind, it probably is cleaner to make
ieee80211_sta_join_ibss() fail if it cannot allocate an skb for
ProbeResp. If we are going to be doing some cleanup here, I would
suggest moving the proberesp building into a helper function and just do
skb = ieee80211_sta_build_probe_resp(...); if (!skb) return -ENOMEM; in
ieee80211_sta_join_ibss().
--
Jouni Malinen PGP id EFC895FA