[mt76/mt7603/mac] Question about missing variable assignment

All of lore.kernel.org
 help / color / mirror / Atom feed

* [mt76/mt7603/mac] Question about missing variable assignment
@ 2019-03-02 21:10 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-02 21:10 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi, Kalle Valo, David S. Miller,
	Matthias Brugger
  Cc: Gustavo A. R. Silva, netdev, linux-wireless, linux-kernel,
	linux-mediatek, linux-arm-kernel

Hi all,

The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
is missing a variable assignment before line 1058.  Notice that there
is a potential execution path in which variable *i* is compared against
magic number 15 at line 1075 without being initialized previously
(this was reported by Coverity):

1055 out:
1056         final_rate_flags = info->status.rates[final_idx].flags;
1057 
1058         switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
1059         case MT_PHY_TYPE_CCK:
1060                 cck = true;
1061                 /* fall through */
1062         case MT_PHY_TYPE_OFDM:
1063                 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
1064                         sband = &dev->mt76.sband_5g.sband;
1065                 else
1066                         sband = &dev->mt76.sband_2g.sband;
1067                 final_rate &= GENMASK(5, 0);
1068                 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
1069                 final_rate_flags = 0;
1070                 break;
1071         case MT_PHY_TYPE_HT_GF:
1072         case MT_PHY_TYPE_HT:
1073                 final_rate_flags |= IEEE80211_TX_RC_MCS;
1074                 final_rate &= GENMASK(5, 0);
1075                 if (i > 15)
1076                         return false;
1077                 break;
1078         default:
1079                 return false;
1080         }

My guess is that such missing assignment should be something similar
to the one at line 566:

	i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);

but I'm not sure what the proper arguments for macro FIELD_GET should
be.

This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe

What do you think?

Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [mt76/mt7603/mac] Question about missing variable assignment
@ 2019-03-02 21:10 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-02 21:10 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi, Kalle Valo, David S. Miller,
	Matthias Brugger
  Cc: Gustavo A. R. Silva, netdev, linux-wireless, linux-kernel,
	linux-mediatek, linux-arm-kernel

Hi all,

The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
is missing a variable assignment before line 1058.  Notice that there
is a potential execution path in which variable *i* is compared against
magic number 15 at line 1075 without being initialized previously
(this was reported by Coverity):

1055 out:
1056         final_rate_flags = info->status.rates[final_idx].flags;
1057 
1058         switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
1059         case MT_PHY_TYPE_CCK:
1060                 cck = true;
1061                 /* fall through */
1062         case MT_PHY_TYPE_OFDM:
1063                 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
1064                         sband = &dev->mt76.sband_5g.sband;
1065                 else
1066                         sband = &dev->mt76.sband_2g.sband;
1067                 final_rate &= GENMASK(5, 0);
1068                 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
1069                 final_rate_flags = 0;
1070                 break;
1071         case MT_PHY_TYPE_HT_GF:
1072         case MT_PHY_TYPE_HT:
1073                 final_rate_flags |= IEEE80211_TX_RC_MCS;
1074                 final_rate &= GENMASK(5, 0);
1075                 if (i > 15)
1076                         return false;
1077                 break;
1078         default:
1079                 return false;
1080         }

My guess is that such missing assignment should be something similar
to the one at line 566:

	i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);

but I'm not sure what the proper arguments for macro FIELD_GET should
be.

This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe

What do you think?

Thanks
--
Gustavo


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [mt76/mt7603/mac] Question about missing variable assignment
@ 2019-03-02 21:10 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-02 21:10 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi, Kalle Valo, David S. Miller,
	Matthias Brugger
  Cc: linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
	linux-kernel, Gustavo A. R. Silva

Hi all,

The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
is missing a variable assignment before line 1058.  Notice that there
is a potential execution path in which variable *i* is compared against
magic number 15 at line 1075 without being initialized previously
(this was reported by Coverity):

1055 out:
1056         final_rate_flags = info->status.rates[final_idx].flags;
1057 
1058         switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
1059         case MT_PHY_TYPE_CCK:
1060                 cck = true;
1061                 /* fall through */
1062         case MT_PHY_TYPE_OFDM:
1063                 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
1064                         sband = &dev->mt76.sband_5g.sband;
1065                 else
1066                         sband = &dev->mt76.sband_2g.sband;
1067                 final_rate &= GENMASK(5, 0);
1068                 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
1069                 final_rate_flags = 0;
1070                 break;
1071         case MT_PHY_TYPE_HT_GF:
1072         case MT_PHY_TYPE_HT:
1073                 final_rate_flags |= IEEE80211_TX_RC_MCS;
1074                 final_rate &= GENMASK(5, 0);
1075                 if (i > 15)
1076                         return false;
1077                 break;
1078         default:
1079                 return false;
1080         }

My guess is that such missing assignment should be something similar
to the one at line 566:

	i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);

but I'm not sure what the proper arguments for macro FIELD_GET should
be.

This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe

What do you think?

Thanks
--
Gustavo


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [mt76/mt7603/mac] Question about missing variable assignment
  2019-03-02 21:10 ` Gustavo A. R. Silva
@ 2019-03-03 10:05   ` Felix Fietkau
  -1 siblings, 0 replies; 8+ messages in thread
From: Felix Fietkau @ 2019-03-03 10:05 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Lorenzo Bianconi, Kalle Valo,
	David S. Miller, Matthias Brugger
  Cc: linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
	linux-kernel

On 2019-03-02 22:10, Gustavo A. R. Silva wrote:
> Hi all,
> 
> The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
> is missing a variable assignment before line 1058.  Notice that there
> is a potential execution path in which variable *i* is compared against
> magic number 15 at line 1075 without being initialized previously
> (this was reported by Coverity):
> 
> 1055 out:
> 1056         final_rate_flags = info->status.rates[final_idx].flags;
> 1057 
> 1058         switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
> 1059         case MT_PHY_TYPE_CCK:
> 1060                 cck = true;
> 1061                 /* fall through */
> 1062         case MT_PHY_TYPE_OFDM:
> 1063                 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
> 1064                         sband = &dev->mt76.sband_5g.sband;
> 1065                 else
> 1066                         sband = &dev->mt76.sband_2g.sband;
> 1067                 final_rate &= GENMASK(5, 0);
> 1068                 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
> 1069                 final_rate_flags = 0;
> 1070                 break;
> 1071         case MT_PHY_TYPE_HT_GF:
> 1072         case MT_PHY_TYPE_HT:
> 1073                 final_rate_flags |= IEEE80211_TX_RC_MCS;
> 1074                 final_rate &= GENMASK(5, 0);
> 1075                 if (i > 15)
> 1076                         return false;
> 1077                 break;
> 1078         default:
> 1079                 return false;
> 1080         }
> 
> My guess is that such missing assignment should be something similar
> to the one at line 566:
> 
> 	i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);
> 
> but I'm not sure what the proper arguments for macro FIELD_GET should
> be.
> 
> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe
> 
> What do you think?
Thanks for reporting this. The fix is simpler than that, the check
should be: if (final_rate > 15)
I will send a fix.

- Felix

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [mt76/mt7603/mac] Question about missing variable assignment
@ 2019-03-03 10:05   ` Felix Fietkau
  0 siblings, 0 replies; 8+ messages in thread
From: Felix Fietkau @ 2019-03-03 10:05 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Lorenzo Bianconi, Kalle Valo,
	David S. Miller, Matthias Brugger
  Cc: netdev, linux-mediatek, linux-wireless, linux-kernel,
	linux-arm-kernel

On 2019-03-02 22:10, Gustavo A. R. Silva wrote:
> Hi all,
> 
> The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
> is missing a variable assignment before line 1058.  Notice that there
> is a potential execution path in which variable *i* is compared against
> magic number 15 at line 1075 without being initialized previously
> (this was reported by Coverity):
> 
> 1055 out:
> 1056         final_rate_flags = info->status.rates[final_idx].flags;
> 1057 
> 1058         switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
> 1059         case MT_PHY_TYPE_CCK:
> 1060                 cck = true;
> 1061                 /* fall through */
> 1062         case MT_PHY_TYPE_OFDM:
> 1063                 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
> 1064                         sband = &dev->mt76.sband_5g.sband;
> 1065                 else
> 1066                         sband = &dev->mt76.sband_2g.sband;
> 1067                 final_rate &= GENMASK(5, 0);
> 1068                 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
> 1069                 final_rate_flags = 0;
> 1070                 break;
> 1071         case MT_PHY_TYPE_HT_GF:
> 1072         case MT_PHY_TYPE_HT:
> 1073                 final_rate_flags |= IEEE80211_TX_RC_MCS;
> 1074                 final_rate &= GENMASK(5, 0);
> 1075                 if (i > 15)
> 1076                         return false;
> 1077                 break;
> 1078         default:
> 1079                 return false;
> 1080         }
> 
> My guess is that such missing assignment should be something similar
> to the one at line 566:
> 
> 	i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);
> 
> but I'm not sure what the proper arguments for macro FIELD_GET should
> be.
> 
> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe
> 
> What do you think?
Thanks for reporting this. The fix is simpler than that, the check
should be: if (final_rate > 15)
I will send a fix.

- Felix

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

[parent not found: <827d2ee6-04de-f4f9-15b4-40dac90bda9c-Vt+b4OUoWG0@public.gmane.org>]

* Re: [mt76/mt7603/mac] Question about missing variable assignment
  2019-03-03 10:05   ` Felix Fietkau
  (?)
@ 2019-03-03 15:06       ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-03 15:06 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi, Kalle Valo, David S. Miller,
	Matthias Brugger
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA



On 3/3/19 4:05 AM, Felix Fietkau wrote:
> On 2019-03-02 22:10, Gustavo A. R. Silva wrote:
>> Hi all,
>>
>> The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> is missing a variable assignment before line 1058.  Notice that there
>> is a potential execution path in which variable *i* is compared against
>> magic number 15 at line 1075 without being initialized previously
>> (this was reported by Coverity):
>>
>> 1055 out:
>> 1056         final_rate_flags = info->status.rates[final_idx].flags;
>> 1057 
>> 1058         switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
>> 1059         case MT_PHY_TYPE_CCK:
>> 1060                 cck = true;
>> 1061                 /* fall through */
>> 1062         case MT_PHY_TYPE_OFDM:
>> 1063                 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
>> 1064                         sband = &dev->mt76.sband_5g.sband;
>> 1065                 else
>> 1066                         sband = &dev->mt76.sband_2g.sband;
>> 1067                 final_rate &= GENMASK(5, 0);
>> 1068                 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
>> 1069                 final_rate_flags = 0;
>> 1070                 break;
>> 1071         case MT_PHY_TYPE_HT_GF:
>> 1072         case MT_PHY_TYPE_HT:
>> 1073                 final_rate_flags |= IEEE80211_TX_RC_MCS;
>> 1074                 final_rate &= GENMASK(5, 0);
>> 1075                 if (i > 15)
>> 1076                         return false;
>> 1077                 break;
>> 1078         default:
>> 1079                 return false;
>> 1080         }
>>
>> My guess is that such missing assignment should be something similar
>> to the one at line 566:
>>
>> 	i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);
>>
>> but I'm not sure what the proper arguments for macro FIELD_GET should
>> be.
>>
>> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe
>>
>> What do you think?
> Thanks for reporting this. The fix is simpler than that, the check
> should be: if (final_rate > 15)
> I will send a fix.
> 

Great. Glad to help. :)

Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [mt76/mt7603/mac] Question about missing variable assignment
@ 2019-03-03 15:06       ` Gustavo A. R. Silva
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-03 15:06 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi, Kalle Valo, David S. Miller,
	Matthias Brugger
  Cc: netdev, linux-mediatek, linux-wireless, linux-kernel,
	linux-arm-kernel



On 3/3/19 4:05 AM, Felix Fietkau wrote:
> On 2019-03-02 22:10, Gustavo A. R. Silva wrote:
>> Hi all,
>>
>> The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> is missing a variable assignment before line 1058.  Notice that there
>> is a potential execution path in which variable *i* is compared against
>> magic number 15 at line 1075 without being initialized previously
>> (this was reported by Coverity):
>>
>> 1055 out:
>> 1056         final_rate_flags = info->status.rates[final_idx].flags;
>> 1057 
>> 1058         switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
>> 1059         case MT_PHY_TYPE_CCK:
>> 1060                 cck = true;
>> 1061                 /* fall through */
>> 1062         case MT_PHY_TYPE_OFDM:
>> 1063                 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
>> 1064                         sband = &dev->mt76.sband_5g.sband;
>> 1065                 else
>> 1066                         sband = &dev->mt76.sband_2g.sband;
>> 1067                 final_rate &= GENMASK(5, 0);
>> 1068                 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
>> 1069                 final_rate_flags = 0;
>> 1070                 break;
>> 1071         case MT_PHY_TYPE_HT_GF:
>> 1072         case MT_PHY_TYPE_HT:
>> 1073                 final_rate_flags |= IEEE80211_TX_RC_MCS;
>> 1074                 final_rate &= GENMASK(5, 0);
>> 1075                 if (i > 15)
>> 1076                         return false;
>> 1077                 break;
>> 1078         default:
>> 1079                 return false;
>> 1080         }
>>
>> My guess is that such missing assignment should be something similar
>> to the one at line 566:
>>
>> 	i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);
>>
>> but I'm not sure what the proper arguments for macro FIELD_GET should
>> be.
>>
>> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe
>>
>> What do you think?
> Thanks for reporting this. The fix is simpler than that, the check
> should be: if (final_rate > 15)
> I will send a fix.
> 

Great. Glad to help. :)

Thanks
--
Gustavo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [mt76/mt7603/mac] Question about missing variable assignment
@ 2019-03-03 15:06       ` Gustavo A. R. Silva
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-03 15:06 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi, Kalle Valo, David S. Miller,
	Matthias Brugger
  Cc: linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
	linux-kernel



On 3/3/19 4:05 AM, Felix Fietkau wrote:
> On 2019-03-02 22:10, Gustavo A. R. Silva wrote:
>> Hi all,
>>
>> The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> is missing a variable assignment before line 1058.  Notice that there
>> is a potential execution path in which variable *i* is compared against
>> magic number 15 at line 1075 without being initialized previously
>> (this was reported by Coverity):
>>
>> 1055 out:
>> 1056         final_rate_flags = info->status.rates[final_idx].flags;
>> 1057 
>> 1058         switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
>> 1059         case MT_PHY_TYPE_CCK:
>> 1060                 cck = true;
>> 1061                 /* fall through */
>> 1062         case MT_PHY_TYPE_OFDM:
>> 1063                 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
>> 1064                         sband = &dev->mt76.sband_5g.sband;
>> 1065                 else
>> 1066                         sband = &dev->mt76.sband_2g.sband;
>> 1067                 final_rate &= GENMASK(5, 0);
>> 1068                 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
>> 1069                 final_rate_flags = 0;
>> 1070                 break;
>> 1071         case MT_PHY_TYPE_HT_GF:
>> 1072         case MT_PHY_TYPE_HT:
>> 1073                 final_rate_flags |= IEEE80211_TX_RC_MCS;
>> 1074                 final_rate &= GENMASK(5, 0);
>> 1075                 if (i > 15)
>> 1076                         return false;
>> 1077                 break;
>> 1078         default:
>> 1079                 return false;
>> 1080         }
>>
>> My guess is that such missing assignment should be something similar
>> to the one at line 566:
>>
>> 	i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);
>>
>> but I'm not sure what the proper arguments for macro FIELD_GET should
>> be.
>>
>> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe
>>
>> What do you think?
> Thanks for reporting this. The fix is simpler than that, the check
> should be: if (final_rate > 15)
> I will send a fix.
> 

Great. Glad to help. :)

Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-03-03 15:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-02 21:10 [mt76/mt7603/mac] Question about missing variable assignment Gustavo A. R. Silva
2019-03-02 21:10 ` Gustavo A. R. Silva
2019-03-02 21:10 ` Gustavo A. R. Silva
2019-03-03 10:05 ` Felix Fietkau
2019-03-03 10:05   ` Felix Fietkau
     [not found]   ` <827d2ee6-04de-f4f9-15b4-40dac90bda9c-Vt+b4OUoWG0@public.gmane.org>
2019-03-03 15:06     ` Gustavo A. R. Silva
2019-03-03 15:06       ` Gustavo A. R. Silva
2019-03-03 15:06       ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.