[PATCH] mac80211: Fix oops in OCB mode when receiving data when not joined

[PATCH] mac80211: Fix oops in OCB mode when receiving data when not joined

[Bertold Van den Bergh]

The current implementation in ocb.c can cause a kernel oops when the
interface is up, but no ocb has been joined. When data is received
with the broadcast BSSID rx_no_sta is called. This function uses
uninitialized variables because the join function has not yet been used.

Signed-off-by: Bertold Van den Bergh <bertold.vandenbergh@esat.kuleuven.be>
---
 net/mac80211/ocb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mac80211/ocb.c b/net/mac80211/ocb.c
index 573b81a..5da2bd3 100644
--- a/net/mac80211/ocb.c
+++ b/net/mac80211/ocb.c
@@ -50,6 +50,9 @@ void ieee80211_ocb_rx_no_sta(struct ieee80211_sub_if_data *sdata,
 	struct sta_info *sta;
 	int band;
 
+	if (!ifocb->joined)
+		return;
+
 	/* XXX: Consider removing the least recently used entry and
 	 *      allow new one to be added.
 	 */
-- 
1.9.1

Re: [PATCH] mac80211: Fix oops in OCB mode when receiving data when not joined

[Johannes Berg]

On Wed, 2015-08-05 at 15:38 +0200, Bertold Van den Bergh wrote:
> The current implementation in ocb.c can cause a kernel oops when the
> interface is up, but no ocb has been joined. When data is received
> with the broadcast BSSID rx_no_sta is called. This function uses
> uninitialized variables because the join function has not yet been 
> used.
> 
> Signed-off-by: Bertold Van den Bergh <
> bertold.vandenbergh@esat.kuleuven.be>
> ---
>  net/mac80211/ocb.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/mac80211/ocb.c b/net/mac80211/ocb.c
> index 573b81a..5da2bd3 100644
> --- a/net/mac80211/ocb.c
> +++ b/net/mac80211/ocb.c
> @@ -50,6 +50,9 @@ void ieee80211_ocb_rx_no_sta(struct 
> ieee80211_sub_if_data *sdata,
>  	struct sta_info *sta;
>  	int band;
>  
> +	if (!ifocb->joined)
> +		return;
> 
Wouldn't it make more sense to put this check into
ieee80211_accept_frame() and, in addition to not doing any station
processing, simply dropping the frame completely? Like such:

--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3314,6 +3314,8 @@ static bool ieee80211_accept_frame(struct ieee80211_rx_data *rx)
 		}
 		return true;
 	case NL80211_IFTYPE_OCB:
+		if (!sdata->u.ocb.joined)
+			return false;
 		if (!bssid)
 			return false;
 		if (ieee80211_is_beacon(hdr->frame_control))

johannes

Re: [PATCH] mac80211: Fix oops in OCB mode when receiving data when not joined

[Johannes Berg]

On Thu, 2015-08-13 at 11:09 +0200, Johannes Berg wrote:
> On Wed, 2015-08-05 at 15:38 +0200, Bertold Van den Bergh wrote:
> > The current implementation in ocb.c can cause a kernel oops when 
> > the
> > interface is up, but no ocb has been joined. When data is received
> > with the broadcast BSSID rx_no_sta is called. This function uses
> > uninitialized variables because the join function has not yet been 
> > used.
> > 
> > Signed-off-by: Bertold Van den Bergh <
> > bertold.vandenbergh@esat.kuleuven.be>
> > ---
> >  net/mac80211/ocb.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/net/mac80211/ocb.c b/net/mac80211/ocb.c
> > index 573b81a..5da2bd3 100644
> > --- a/net/mac80211/ocb.c
> > +++ b/net/mac80211/ocb.c
> > @@ -50,6 +50,9 @@ void ieee80211_ocb_rx_no_sta(struct 
> > ieee80211_sub_if_data *sdata,
> >  	struct sta_info *sta;
> >  	int band;
> >  
> > +	if (!ifocb->joined)
> > +		return;
> > 
> Wouldn't it make more sense to put this check into
> ieee80211_accept_frame() and, in addition to not doing any station
> processing, simply dropping the frame completely? Like such:
> 

Reminder? I'm not sure what to do with this patch - I don't
particularly like processing the frame at all.

johannes

Re: [PATCH] mac80211: Fix oops in OCB mode when receiving data when not joined

[Bertold Van den Bergh]

Hello,

Sorry. I missed your previous email. I agree that your solution is
better and cleaner.

Do you want me to submit a new patch?

Sincerely,
Bertold Van den Bergh

On Tue, Sep 8, 2015 at 6:03 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2015-08-13 at 11:09 +0200, Johannes Berg wrote:
>> On Wed, 2015-08-05 at 15:38 +0200, Bertold Van den Bergh wrote:
>> > The current implementation in ocb.c can cause a kernel oops when
>> > the
>> > interface is up, but no ocb has been joined. When data is received
>> > with the broadcast BSSID rx_no_sta is called. This function uses
>> > uninitialized variables because the join function has not yet been
>> > used.
>> >
>> > Signed-off-by: Bertold Van den Bergh <
>> > bertold.vandenbergh@esat.kuleuven.be>
>> > ---
>> >  net/mac80211/ocb.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/net/mac80211/ocb.c b/net/mac80211/ocb.c
>> > index 573b81a..5da2bd3 100644
>> > --- a/net/mac80211/ocb.c
>> > +++ b/net/mac80211/ocb.c
>> > @@ -50,6 +50,9 @@ void ieee80211_ocb_rx_no_sta(struct
>> > ieee80211_sub_if_data *sdata,
>> >     struct sta_info *sta;
>> >     int band;
>> >
>> > +   if (!ifocb->joined)
>> > +           return;
>> >
>> Wouldn't it make more sense to put this check into
>> ieee80211_accept_frame() and, in addition to not doing any station
>> processing, simply dropping the frame completely? Like such:
>>
>
> Reminder? I'm not sure what to do with this patch - I don't
> particularly like processing the frame at all.
>
> johannes

Re: [PATCH] mac80211: Fix oops in OCB mode when receiving data when not joined

[Johannes Berg]

On Tue, 2015-09-08 at 21:17 +0200, Bertold Van den Bergh wrote:
> Hello,
> 
> Sorry. I missed your previous email. I agree that your solution is
> better and cleaner.
> 
> Do you want me to submit a new patch?
> 

Yes, please do.

Thanks,
johannes