[parent not found: <1457965120-3155420-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>]
* [PATCH 1/5] mlx4: add missing braces in verify_qp_parameters
[not found] ` <1457965120-3155420-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
@ 2016-03-14 14:18 ` Arnd Bergmann
[not found] ` <1457965120-3155420-2-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, Yishai Hadas,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
The implementation of QP paravirtualization back in linux-3.7 included
some code that looks very dubious, and gcc-6 has grown smart enough
to warn about it:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function 'verify_qp_parameters':
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:3154:5: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
if (optpar & MLX4_QP_OPTPAR_ALT_ADDR_PATH) {
^~
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:3144:4: note: ...this 'if' clause, but it is not
if (slave != mlx4_master_func_num(dev))
>From looking at the context, I'm reasonably sure that the indentation
is correct but that it should have contained curly braces from the
start, as the update_gid() function in the same patch correctly does.
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Fixes: 54679e148287 ("mlx4: Implement QP paravirtualization and maintain phys_pkey_cache for smp_snoop")
---
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index 25ce1b030a00..cd9b2b28df88 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -3141,7 +3141,7 @@ static int verify_qp_parameters(struct mlx4_dev *dev,
case QP_TRANS_RTS2RTS:
case QP_TRANS_SQD2SQD:
case QP_TRANS_SQD2RTS:
- if (slave != mlx4_master_func_num(dev))
+ if (slave != mlx4_master_func_num(dev)) {
if (optpar & MLX4_QP_OPTPAR_PRIMARY_ADDR_PATH) {
port = (qp_ctx->pri_path.sched_queue >> 6 & 1) + 1;
if (dev->caps.port_mask[port] != MLX4_PORT_TYPE_IB)
@@ -3160,6 +3160,7 @@ static int verify_qp_parameters(struct mlx4_dev *dev,
if (qp_ctx->alt_path.mgid_index >= num_gids)
return -EINVAL;
}
+ }
break;
default:
break;
--
2.7.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] farsync: fix off-by-one bug in fst_add_one
2016-03-14 14:18 net: gcc-6.0 warning fixes Arnd Bergmann
[not found] ` <1457965120-3155420-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
@ 2016-03-14 14:18 ` Arnd Bergmann
2016-03-14 14:18 ` [PATCH 3/5] ath9k: fix buffer overrun for ar9287 Arnd Bergmann
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Arnd Bergmann, Kevin Curtis, linux-kernel
gcc-6 finds an out of bounds access in the fst_add_one function
when calculating the end of the mmio area:
drivers/net/wan/farsync.c: In function 'fst_add_one':
drivers/net/wan/farsync.c:418:53: error: index 2 denotes an offset greater than size of 'u8[2][8192] {aka unsigned char[2][8192]}' [-Werror=array-bounds]
#define BUF_OFFSET(X) (BFM_BASE + offsetof(struct buf_window, X))
^
include/linux/compiler-gcc.h:158:21: note: in definition of macro '__compiler_offsetof'
__builtin_offsetof(a, b)
^
drivers/net/wan/farsync.c:418:37: note: in expansion of macro 'offsetof'
#define BUF_OFFSET(X) (BFM_BASE + offsetof(struct buf_window, X))
^~~~~~~~
drivers/net/wan/farsync.c:2519:36: note: in expansion of macro 'BUF_OFFSET'
+ BUF_OFFSET ( txBuffer[i][NUM_TX_BUFFER][0]);
^~~~~~~~~~
The warning is correct, but not critical because this appears
to be a write-only variable that is set by each WAN driver but
never accessed afterwards.
I'm taking the minimal fix here, using the correct pointer by
pointing 'mem_end' to the last byte inside of the register area
as all other WAN drivers do, rather than the first byte outside of
it. An alternative would be to just remove the mem_end member
entirely.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wan/farsync.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wan/farsync.c b/drivers/net/wan/farsync.c
index 44541dbc5c28..69b994f3b8c5 100644
--- a/drivers/net/wan/farsync.c
+++ b/drivers/net/wan/farsync.c
@@ -2516,7 +2516,7 @@ fst_add_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dev->mem_start = card->phys_mem
+ BUF_OFFSET ( txBuffer[i][0][0]);
dev->mem_end = card->phys_mem
- + BUF_OFFSET ( txBuffer[i][NUM_TX_BUFFER][0]);
+ + BUF_OFFSET ( txBuffer[i][NUM_TX_BUFFER - 1][LEN_RX_BUFFER - 1]);
dev->base_addr = card->pci_conf;
dev->irq = card->irq;
--
2.7.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] ath9k: fix buffer overrun for ar9287
2016-03-14 14:18 net: gcc-6.0 warning fixes Arnd Bergmann
[not found] ` <1457965120-3155420-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2016-03-14 14:18 ` [PATCH 2/5] farsync: fix off-by-one bug in fst_add_one Arnd Bergmann
@ 2016-03-14 14:18 ` Arnd Bergmann
[not found] ` <1457965120-3155420-4-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2016-03-14 14:18 ` [PATCH 4/5] ath9k: fix misleading indentation Arnd Bergmann
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, netdev, linux-wireless, QCA ath9k Development,
linux-kernel, ath9k-devel, Kalle Valo
Code that was added back in 2.6.38 has an obvious overflow
when accessing a static array, and at the time it was added
only a code comment was put in front of it as a reminder
to have it reviewed properly.
This has not happened, but gcc-6 now points to the specific
overflow:
drivers/net/wireless/ath/ath9k/eeprom.c: In function 'ath9k_hw_get_gain_boundaries_pdadcs':
drivers/net/wireless/ath/ath9k/eeprom.c:483:44: error: array subscript is above array bounds [-Werror=array-bounds]
maxPwrT4[i] = data_9287[idxL].pwrPdg[i][4];
~~~~~~~~~~~~~~~~~~~~~~~~~^~~
It turns out that the correct array length exists in the local
'intercepts' variable of this function, so we can just use that
instead of hardcoding '4', so this patch changes all three
instances to use that variable. The other two instances were
already correct, but it's more consistent this way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 940cd2c12ebf ("ath9k_hw: merge the ar9287 version of ath9k_hw_get_gain_boundaries_pdadcs")
---
drivers/net/wireless/ath/ath9k/eeprom.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
index 73fb4232f9f2..a794157a147d 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom.c
@@ -477,10 +477,9 @@ void ath9k_hw_get_gain_boundaries_pdadcs(struct ath_hw *ah,
if (match) {
if (AR_SREV_9287(ah)) {
- /* FIXME: array overrun? */
for (i = 0; i < numXpdGains; i++) {
minPwrT4[i] = data_9287[idxL].pwrPdg[i][0];
- maxPwrT4[i] = data_9287[idxL].pwrPdg[i][4];
+ maxPwrT4[i] = data_9287[idxL].pwrPdg[i][intercepts - 1];
ath9k_hw_fill_vpd_table(minPwrT4[i], maxPwrT4[i],
data_9287[idxL].pwrPdg[i],
data_9287[idxL].vpdPdg[i],
@@ -490,7 +489,7 @@ void ath9k_hw_get_gain_boundaries_pdadcs(struct ath_hw *ah,
} else if (eeprom_4k) {
for (i = 0; i < numXpdGains; i++) {
minPwrT4[i] = data_4k[idxL].pwrPdg[i][0];
- maxPwrT4[i] = data_4k[idxL].pwrPdg[i][4];
+ maxPwrT4[i] = data_4k[idxL].pwrPdg[i][intercepts - 1];
ath9k_hw_fill_vpd_table(minPwrT4[i], maxPwrT4[i],
data_4k[idxL].pwrPdg[i],
data_4k[idxL].vpdPdg[i],
@@ -500,7 +499,7 @@ void ath9k_hw_get_gain_boundaries_pdadcs(struct ath_hw *ah,
} else {
for (i = 0; i < numXpdGains; i++) {
minPwrT4[i] = data_def[idxL].pwrPdg[i][0];
- maxPwrT4[i] = data_def[idxL].pwrPdg[i][4];
+ maxPwrT4[i] = data_def[idxL].pwrPdg[i][intercepts - 1];
ath9k_hw_fill_vpd_table(minPwrT4[i], maxPwrT4[i],
data_def[idxL].pwrPdg[i],
data_def[idxL].vpdPdg[i],
--
2.7.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/5] ath9k: fix misleading indentation
2016-03-14 14:18 net: gcc-6.0 warning fixes Arnd Bergmann
` (2 preceding siblings ...)
2016-03-14 14:18 ` [PATCH 3/5] ath9k: fix buffer overrun for ar9287 Arnd Bergmann
@ 2016-03-14 14:18 ` Arnd Bergmann
2016-03-21 14:24 ` Kalle Valo
2016-03-14 14:18 ` [PATCH 5/5] net: caif: " Arnd Bergmann
2016-03-14 17:10 ` net: gcc-6.0 warning fixes David Miller
5 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Arnd Bergmann, QCA ath9k Development, Kalle Valo,
linux-wireless, ath9k-devel, linux-kernel
A cleanup patch in linux-3.18 moved around some code in the ath9k
driver and left some code to be indented in a misleading way,
made worse by the addition of some new code for p2p mode, as
discovered by a new gcc-6 warning:
drivers/net/wireless/ath/ath9k/init.c: In function 'ath9k_set_hw_capab':
drivers/net/wireless/ath/ath9k/init.c:851:4: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
hw->wiphy->iface_combinations = if_comb;
^~
drivers/net/wireless/ath/ath9k/init.c:847:3: note: ...this 'if' clause, but it is not
if (ath9k_is_chanctx_enabled())
^~
The code is in fact correct, but the indentation is not, so I'm
reformatting it as it should have been after the original cleanup.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 499afaccf6f3 ("ath9k: Isolate ath9k_use_chanctx module parameter")
Fixes: eb61f9f623f7 ("ath9k: advertise p2p dev support when chanctx")
---
drivers/net/wireless/ath/ath9k/init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index d4e0ac946c3a..1c226d63bb03 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -848,8 +848,8 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
hw->wiphy->interface_modes |=
BIT(NL80211_IFTYPE_P2P_DEVICE);
- hw->wiphy->iface_combinations = if_comb;
- hw->wiphy->n_iface_combinations = ARRAY_SIZE(if_comb);
+ hw->wiphy->iface_combinations = if_comb;
+ hw->wiphy->n_iface_combinations = ARRAY_SIZE(if_comb);
}
hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
--
2.7.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/5] ath9k: fix misleading indentation
2016-03-14 14:18 ` [PATCH 4/5] ath9k: fix misleading indentation Arnd Bergmann
@ 2016-03-21 14:24 ` Kalle Valo
0 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2016-03-21 14:24 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, netdev, QCA ath9k Development, linux-wireless,
ath9k-devel, linux-kernel
Arnd Bergmann <arnd@arndb.de> writes:
> A cleanup patch in linux-3.18 moved around some code in the ath9k
> driver and left some code to be indented in a misleading way,
> made worse by the addition of some new code for p2p mode, as
> discovered by a new gcc-6 warning:
>
> drivers/net/wireless/ath/ath9k/init.c: In function 'ath9k_set_hw_capab':
> drivers/net/wireless/ath/ath9k/init.c:851:4: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
> hw->wiphy->iface_combinations = if_comb;
> ^~
> drivers/net/wireless/ath/ath9k/init.c:847:3: note: ...this 'if' clause, but it is not
> if (ath9k_is_chanctx_enabled())
> ^~
>
> The code is in fact correct, but the indentation is not, so I'm
> reformatting it as it should have been after the original cleanup.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 499afaccf6f3 ("ath9k: Isolate ath9k_use_chanctx module parameter")
> Fixes: eb61f9f623f7 ("ath9k: advertise p2p dev support when chanctx")
Dave already applied this so I can skip this.
--
Kalle Valo
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/5] net: caif: fix misleading indentation
2016-03-14 14:18 net: gcc-6.0 warning fixes Arnd Bergmann
` (3 preceding siblings ...)
2016-03-14 14:18 ` [PATCH 4/5] ath9k: fix misleading indentation Arnd Bergmann
@ 2016-03-14 14:18 ` Arnd Bergmann
2016-03-14 17:10 ` net: gcc-6.0 warning fixes David Miller
5 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Arnd Bergmann, Dmitry Tarnyagin, linux-kernel
gcc points out code that is not indented the way it is
interpreted:
net/caif/cfpkt_skbuff.c: In function 'cfpkt_setlen':
net/caif/cfpkt_skbuff.c:289:4: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
return cfpkt_getlen(pkt);
^~~~~~
net/caif/cfpkt_skbuff.c:286:3: note: ...this 'else' clause, but it is not
else
^~~~
It is clear from the context that not returning here would be
a bug, as we'd end up passing a negative length into a function
that takes a u16 length, so it is not missing curly braces
here, and I'm assuming that the indentation is the only part
that's wrong about it.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/caif/cfpkt_skbuff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
index f6c3b2137eea..59ce1fcc220c 100644
--- a/net/caif/cfpkt_skbuff.c
+++ b/net/caif/cfpkt_skbuff.c
@@ -286,7 +286,7 @@ int cfpkt_setlen(struct cfpkt *pkt, u16 len)
else
skb_trim(skb, len);
- return cfpkt_getlen(pkt);
+ return cfpkt_getlen(pkt);
}
/* Need to expand SKB */
--
2.7.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: net: gcc-6.0 warning fixes
2016-03-14 14:18 net: gcc-6.0 warning fixes Arnd Bergmann
` (4 preceding siblings ...)
2016-03-14 14:18 ` [PATCH 5/5] net: caif: " Arnd Bergmann
@ 2016-03-14 17:10 ` David Miller
5 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2016-03-14 17:10 UTC (permalink / raw)
To: arnd; +Cc: netdev
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 14 Mar 2016 15:18:33 +0100
> I've just installed gcc-6.0 to see what kinds of new warnings
> we get. It turns out that it's actually really useful once I
> disabled -Wunused-const-variable, and all of the warnings it
> found in network drivers seem valid.
>
> Sorry for the bad timing in the merge window, but I figured
> it would be better to send the fixes as I found the bugs
> rather than waiting for the next cycle. The first three
> look appropriate for stable backports.
>
> The other two only fix a gcc warning about incorrect whitespace,
> probably not worth backporting those.
Series applied, and patches 1, 2, and 3 queued up for -stable.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread