re: [PATCH] cfg80211: read wmm rules from regulatory database

re: [PATCH] cfg80211: read wmm rules from regulatory database

[Colin Ian King]

Hi Haim,

I think there may be an issue with the commit:

>From 230ebaa189af44d50dccb4a1846e39ca594e347b Mon Sep 17 00:00:00 2001
From: Haim Dreyfuss <haim.dreyfuss@intel.com>
Date: Wed, 28 Mar 2018 13:24:09 +0300
Subject: [PATCH] cfg80211: read wmm rules from regulatory database

specifically in function: reg_copy_regd()

+       for (i = 0; i < src_regd->n_reg_rules; i++) {
                memcpy(&regd->reg_rules[i], &src_regd->reg_rules[i],
                       sizeof(struct ieee80211_reg_rule));
+               if (!src_regd->reg_rules[i].wmm_rule)
+                       continue;

+               regd->reg_rules[i].wmm_rule = d_wmm +
+                       (src_regd->reg_rules[i].wmm_rule - s_wmm) /
+                       sizeof(struct ieee80211_wmm_rule);
+       }

The pointer arithmetic (src_regd->reg_rules[i].wmm_rule - s_wmm) is
performed in terms of the size of struct ieee80211_wmm_rule and not in
bytes and I believe that the division by sizeof(struct
ieee80211_wmm_rule) is not required.

This issue was detected by static analysis with Coverity Scan,
CID#1467451 ("Extra sizeof expression"), 'suspicious_division'

I'm not 100% sure that is this a false positive or not, but I think it
looks incorrect to me.

Colin

re: [PATCH] cfg80211: read wmm rules from regulatory database

[Colin Ian King]

Hi Haim,

I think there may be an issue with the commit:

From 230ebaa189af44d50dccb4a1846e39ca594e347b Mon Sep 17 00:00:00 2001
From: Haim Dreyfuss <haim.dreyfuss@intel.com>
Date: Wed, 28 Mar 2018 13:24:09 +0300
Subject: [PATCH] cfg80211: read wmm rules from regulatory database

specifically in function: reg_copy_regd()

+       for (i = 0; i < src_regd->n_reg_rules; i++) {
                memcpy(&regd->reg_rules[i], &src_regd->reg_rules[i],
                       sizeof(struct ieee80211_reg_rule));
+               if (!src_regd->reg_rules[i].wmm_rule)
+                       continue;

+               regd->reg_rules[i].wmm_rule = d_wmm +
+                       (src_regd->reg_rules[i].wmm_rule - s_wmm) /
+                       sizeof(struct ieee80211_wmm_rule);
+       }

The pointer arithmetic (src_regd->reg_rules[i].wmm_rule - s_wmm) is
performed in terms of the size of struct ieee80211_wmm_rule and not in
bytes and I believe that the division by sizeof(struct
ieee80211_wmm_rule) is not required.

This issue was detected by static analysis with Coverity Scan,
CID#1467451 ("Extra sizeof expression"), 'suspicious_division'

I'm not 100% sure that is this a false positive or not, but I think it
looks incorrect to me.

Colin

RE: [PATCH] cfg80211: read wmm rules from regulatory database

[Dreyfuss, Haim]

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBDb2xpbiBJYW4gS2luZyBbbWFp
bHRvOmNvbGluLmtpbmdAY2Fub25pY2FsLmNvbV0NCj4gU2VudDogVHVlc2RheSwgSnVseSAzMSwg
MjAxOCAyOjI4IFBNDQo+IFRvOiBEcmV5ZnVzcywgSGFpbSA8aGFpbS5kcmV5ZnVzc0BpbnRlbC5j
b20+OyBEYXZpZCBTLiBNaWxsZXINCj4gPGRhdmVtQGRhdmVtbG9mdC5uZXQ+OyBKb2hhbm5lcyBC
ZXJnIDxqb2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0PjsNCj4gbmV0ZGV2QHZnZXIua2VybmVsLm9y
ZzsgbGludXgtd2lyZWxlc3NAdmdlci5rZXJuZWwub3JnDQo+IENjOiBsaW51eC1rZXJuZWxAdmdl
ci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IHJlOiBbUEFUQ0hdIGNmZzgwMjExOiByZWFkIHdtbSBy
dWxlcyBmcm9tIHJlZ3VsYXRvcnkgZGF0YWJhc2UNCj4gDQo+IEhpIEhhaW0sDQo+IA0KPiBJIHRo
aW5rIHRoZXJlIG1heSBiZSBhbiBpc3N1ZSB3aXRoIHRoZSBjb21taXQ6DQo+IA0KPiBGcm9tIDIz
MGViYWExODlhZjQ0ZDUwZGNjYjRhMTg0NmUzOWNhNTk0ZTM0N2IgTW9uIFNlcCAxNyAwMDowMDow
MA0KPiAyMDAxDQo+IEZyb206IEhhaW0gRHJleWZ1c3MgPGhhaW0uZHJleWZ1c3NAaW50ZWwuY29t
Pg0KPiBEYXRlOiBXZWQsIDI4IE1hciAyMDE4IDEzOjI0OjA5ICswMzAwDQo+IFN1YmplY3Q6IFtQ
QVRDSF0gY2ZnODAyMTE6IHJlYWQgd21tIHJ1bGVzIGZyb20gcmVndWxhdG9yeSBkYXRhYmFzZQ0K
PiANCj4gc3BlY2lmaWNhbGx5IGluIGZ1bmN0aW9uOiByZWdfY29weV9yZWdkKCkNCj4gDQo+ICsg
ICAgICAgZm9yIChpID0gMDsgaSA8IHNyY19yZWdkLT5uX3JlZ19ydWxlczsgaSsrKSB7DQo+ICAg
ICAgICAgICAgICAgICBtZW1jcHkoJnJlZ2QtPnJlZ19ydWxlc1tpXSwgJnNyY19yZWdkLT5yZWdf
cnVsZXNbaV0sDQo+ICAgICAgICAgICAgICAgICAgICAgICAgc2l6ZW9mKHN0cnVjdCBpZWVlODAy
MTFfcmVnX3J1bGUpKTsNCj4gKyAgICAgICAgICAgICAgIGlmICghc3JjX3JlZ2QtPnJlZ19ydWxl
c1tpXS53bW1fcnVsZSkNCj4gKyAgICAgICAgICAgICAgICAgICAgICAgY29udGludWU7DQo+IA0K
PiArICAgICAgICAgICAgICAgcmVnZC0+cmVnX3J1bGVzW2ldLndtbV9ydWxlID0gZF93bW0gKw0K
PiArICAgICAgICAgICAgICAgICAgICAgICAoc3JjX3JlZ2QtPnJlZ19ydWxlc1tpXS53bW1fcnVs
ZSAtIHNfd21tKSAvDQo+ICsgICAgICAgICAgICAgICAgICAgICAgIHNpemVvZihzdHJ1Y3QgaWVl
ZTgwMjExX3dtbV9ydWxlKTsNCj4gKyAgICAgICB9DQo+IA0KPiBUaGUgcG9pbnRlciBhcml0aG1l
dGljIChzcmNfcmVnZC0+cmVnX3J1bGVzW2ldLndtbV9ydWxlIC0gc193bW0pIGlzDQo+IHBlcmZv
cm1lZCBpbiB0ZXJtcyBvZiB0aGUgc2l6ZSBvZiBzdHJ1Y3QgaWVlZTgwMjExX3dtbV9ydWxlIGFu
ZCBub3QgaW4NCj4gYnl0ZXMgYW5kIEkgYmVsaWV2ZSB0aGF0IHRoZSBkaXZpc2lvbiBieSBzaXpl
b2Yoc3RydWN0DQo+IGllZWU4MDIxMV93bW1fcnVsZSkgaXMgbm90IHJlcXVpcmVkLg0KPiANCj4g
VGhpcyBpc3N1ZSB3YXMgZGV0ZWN0ZWQgYnkgc3RhdGljIGFuYWx5c2lzIHdpdGggQ292ZXJpdHkg
U2NhbiwNCj4gQ0lEIzE0Njc0NTEgKCJFeHRyYSBzaXplb2YgZXhwcmVzc2lvbiIpLCAnc3VzcGlj
aW91c19kaXZpc2lvbicNCj4gDQo+IEknbSBub3QgMTAwJSBzdXJlIHRoYXQgaXMgdGhpcyBhIGZh
bHNlIHBvc2l0aXZlIG9yIG5vdCwgYnV0IEkgdGhpbmsgaXQgbG9va3MNCj4gaW5jb3JyZWN0IHRv
IG1lLg0KDQpZZWFoIHlvdSdyZSByaWdodCwgdGhpcyBpcyBub3QgZmFsc2UgcG9zaXRpdmUuDQpK
b2hhbm5lcyBhbHJlYWR5IGZpeGVkIHRoYXQgYW5kIEx1Y2Egd2lsbCBwcm9iYWJseSBzZW5kIGl0
IGluIHRoZSBjb21pbmcgd2Vlay4NCj4gDQo=

RE: [PATCH] cfg80211: read wmm rules from regulatory database

[Dreyfuss, Haim]

> -----Original Message-----
> From: Colin Ian King [mailto:colin.king@canonical.com]
> Sent: Tuesday, July 31, 2018 2:28 PM
> To: Dreyfuss, Haim <haim.dreyfuss@intel.com>; David S. Miller
> <davem@davemloft.net>; Johannes Berg <johannes@sipsolutions.net>;
> netdev@vger.kernel.org; linux-wireless@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Subject: re: [PATCH] cfg80211: read wmm rules from regulatory database
> 
> Hi Haim,
> 
> I think there may be an issue with the commit:
> 
> From 230ebaa189af44d50dccb4a1846e39ca594e347b Mon Sep 17 00:00:00
> 2001
> From: Haim Dreyfuss <haim.dreyfuss@intel.com>
> Date: Wed, 28 Mar 2018 13:24:09 +0300
> Subject: [PATCH] cfg80211: read wmm rules from regulatory database
> 
> specifically in function: reg_copy_regd()
> 
> +       for (i = 0; i < src_regd->n_reg_rules; i++) {
>                 memcpy(&regd->reg_rules[i], &src_regd->reg_rules[i],
>                        sizeof(struct ieee80211_reg_rule));
> +               if (!src_regd->reg_rules[i].wmm_rule)
> +                       continue;
> 
> +               regd->reg_rules[i].wmm_rule = d_wmm +
> +                       (src_regd->reg_rules[i].wmm_rule - s_wmm) /
> +                       sizeof(struct ieee80211_wmm_rule);
> +       }
> 
> The pointer arithmetic (src_regd->reg_rules[i].wmm_rule - s_wmm) is
> performed in terms of the size of struct ieee80211_wmm_rule and not in
> bytes and I believe that the division by sizeof(struct
> ieee80211_wmm_rule) is not required.
> 
> This issue was detected by static analysis with Coverity Scan,
> CID#1467451 ("Extra sizeof expression"), 'suspicious_division'
> 
> I'm not 100% sure that is this a false positive or not, but I think it looks
> incorrect to me.

Yeah you're right, this is not false positive.
Johannes already fixed that and Luca will probably send it in the coming week.
>