* [PATCH 1/5] gpio: pca954x: fix undefined error code from remove
2016-08-26 15:25 [PATCH 0/5] -Wmaybe-uninitialized bug fixes for linux-next Arnd Bergmann
@ 2016-08-26 15:25 ` Arnd Bergmann
2016-08-26 15:47 ` Phil Reid
2016-09-07 14:15 ` Linus Walleij
2016-08-26 15:25 ` [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic Arnd Bergmann
` (5 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Arnd Bergmann @ 2016-08-26 15:25 UTC (permalink / raw)
To: linux-kernel
Cc: Linus Torvalds, Arnd Bergmann, Phil Reid, Linus Walleij,
Alexandre Courbot, linux-gpio
The recent addition of the regulator support has led to the pca953x_remove
function returning uninitialized data when no platform data pointer is
provided, as gcc warns when using -Wmaybe-uninitialized:
drivers/gpio/gpio-pca953x.c: In function 'pca953x_remove':
drivers/gpio/gpio-pca953x.c:860:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This restores the previous behavior, returning 0 on success.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: e23efa311110 ("gpio: pca954x: Add vcc regulator and enable it")
Cc: Phil Reid <preid@electromag.com.au>
---
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: linux-gpio@vger.kernel.org
drivers/gpio/gpio-pca953x.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index cbe2824461eb..b9d31d737dbf 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -853,6 +853,8 @@ static int pca953x_remove(struct i2c_client *client)
if (ret < 0)
dev_err(&client->dev, "%s failed, %d\n",
"teardown", ret);
+ } else {
+ ret = 0;
}
regulator_disable(chip->regulator);
--
2.9.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] gpio: pca954x: fix undefined error code from remove
2016-08-26 15:25 ` [PATCH 1/5] gpio: pca954x: fix undefined error code from remove Arnd Bergmann
@ 2016-08-26 15:47 ` Phil Reid
2016-09-07 14:15 ` Linus Walleij
1 sibling, 0 replies; 17+ messages in thread
From: Phil Reid @ 2016-08-26 15:47 UTC (permalink / raw)
To: Arnd Bergmann, linux-kernel
Cc: Linus Torvalds, Linus Walleij, Alexandre Courbot, linux-gpio
On 26/08/2016 23:25, Arnd Bergmann wrote:
> The recent addition of the regulator support has led to the pca953x_remove
> function returning uninitialized data when no platform data pointer is
> provided, as gcc warns when using -Wmaybe-uninitialized:
>
> drivers/gpio/gpio-pca953x.c: In function 'pca953x_remove':
> drivers/gpio/gpio-pca953x.c:860:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This restores the previous behavior, returning 0 on success.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: e23efa311110 ("gpio: pca954x: Add vcc regulator and enable it")
> Cc: Phil Reid <preid@electromag.com.au>
> ---
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: linux-gpio@vger.kernel.org
>
> drivers/gpio/gpio-pca953x.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index cbe2824461eb..b9d31d737dbf 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -853,6 +853,8 @@ static int pca953x_remove(struct i2c_client *client)
> if (ret < 0)
> dev_err(&client->dev, "%s failed, %d\n",
> "teardown", ret);
> + } else {
> + ret = 0;
> }
>
> regulator_disable(chip->regulator);
>
Ahh, commit 8c7a92dad1621f38d1ff4fe9eaac898d6f33a0a3 gpio: pca953x: remove redundant assignments
removed the 'redundant' initialisation of ret in this function.
Looks like I did not have this commit in my tree when I submitted did the regulator patch.
Sorry about that. Alternative to the 'else' is to add init at definition.
Acked-by: Phil Reid <preid@electromag.com.au>
--
Regards
Phil Reid
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] gpio: pca954x: fix undefined error code from remove
2016-08-26 15:25 ` [PATCH 1/5] gpio: pca954x: fix undefined error code from remove Arnd Bergmann
2016-08-26 15:47 ` Phil Reid
@ 2016-09-07 14:15 ` Linus Walleij
1 sibling, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2016-09-07 14:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, Linus Torvalds, Phil Reid, Alexandre Courbot, linux-gpio
On Fri, Aug 26, 2016 at 5:25 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> The recent addition of the regulator support has led to the pca953x_remove
> function returning uninitialized data when no platform data pointer is
> provided, as gcc warns when using -Wmaybe-uninitialized:
>
> drivers/gpio/gpio-pca953x.c: In function 'pca953x_remove':
> drivers/gpio/gpio-pca953x.c:860:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This restores the previous behavior, returning 0 on success.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: e23efa311110 ("gpio: pca954x: Add vcc regulator and enable it")
> Cc: Phil Reid <preid@electromag.com.au>
> ---
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: linux-gpio@vger.kernel.org
Patch applied with Phil's ACK.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic
2016-08-26 15:25 [PATCH 0/5] -Wmaybe-uninitialized bug fixes for linux-next Arnd Bergmann
2016-08-26 15:25 ` [PATCH 1/5] gpio: pca954x: fix undefined error code from remove Arnd Bergmann
@ 2016-08-26 15:25 ` Arnd Bergmann
2016-08-29 13:18 ` Linus Walleij
2016-08-26 15:25 ` [PATCH 3/5] rxrpc: fix last_call processing Arnd Bergmann
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2016-08-26 15:25 UTC (permalink / raw)
To: linux-kernel
Cc: Linus Torvalds, Arnd Bergmann, Linus Walleij, linux-fbdev,
Russell King, Tomi Valkeinen
The addition of the Nomadik support in this driver introduced
a bug in clcdfb_of_init_display(), which now calls init_panel
with an uninitialized 'endpoint' pointer, as "gcc -Wmaybe-uninitialized"
warns:
drivers/video/fbdev/amba-clcd.c: In function 'clcdfb_of_init_display':
drivers/video/fbdev/amba-clcd.c:785:5: error: 'endpoint' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This reverts the broken part of the function to what it was before
the patch, which is the best guess I have to what it should be.
I assume this was left over from an attempted rework of the
code that was partially backed out.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 046ad6cdeb3f ("video: ARM CLCD: support Nomadik variant")
Cc: Linus Walleij <linus.walleij@linaro.org>
---
Cc: linux-fbdev@vger.kernel.org
Cc: Russell King <linux@armlinux.org.uk>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
drivers/video/fbdev/amba-clcd.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index c342ff370108..ec2671d98abc 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -782,12 +782,9 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
/*
* Fetch the panel endpoint.
*/
- if (!endpoint) {
- endpoint = of_graph_get_next_endpoint(fb->dev->dev.of_node,
- NULL);
- if (!endpoint)
- return -ENODEV;
- }
+ endpoint = of_graph_get_next_endpoint(fb->dev->dev.of_node, NULL);
+ if (!endpoint)
+ return -ENODEV;
if (fb->vendor->init_panel) {
err = fb->vendor->init_panel(fb, endpoint);
--
2.9.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic
2016-08-26 15:25 ` [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic Arnd Bergmann
@ 2016-08-29 13:18 ` Linus Walleij
2016-08-29 13:41 ` Arnd Bergmann
2016-08-30 8:35 ` Tomi Valkeinen
0 siblings, 2 replies; 17+ messages in thread
From: Linus Walleij @ 2016-08-29 13:18 UTC (permalink / raw)
To: Arnd Bergmann, Colin Ian King
Cc: linux-kernel, linux-fbdev, Russell King, Tomi Valkeinen
On Fri, Aug 26, 2016 at 5:25 PM, Arnd Bergmann <arnd@arndb.de> wrote:
[I don't see why Torvalds was CC'ed on this patch? Was there
some specific complaint from his side that I screw things up
or just the wrong Linus?]
> The addition of the Nomadik support in this driver introduced
> a bug in clcdfb_of_init_display(), which now calls init_panel
> with an uninitialized 'endpoint' pointer, as "gcc -Wmaybe-uninitialized"
> warns:
>
> drivers/video/fbdev/amba-clcd.c: In function 'clcdfb_of_init_display':
> drivers/video/fbdev/amba-clcd.c:785:5: error: 'endpoint' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This reverts the broken part of the function to what it was before
> the patch, which is the best guess I have to what it should be.
> I assume this was left over from an attempted rework of the
> code that was partially backed out.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 046ad6cdeb3f ("video: ARM CLCD: support Nomadik variant")
> Cc: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Tomi: I think this bug was also reported by Ian King, so suggest
adding his
Reported-by: Colin Ian King <colin.king@canonical.com>
When applying.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic
2016-08-29 13:18 ` Linus Walleij
@ 2016-08-29 13:41 ` Arnd Bergmann
2016-08-30 8:35 ` Tomi Valkeinen
1 sibling, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2016-08-29 13:41 UTC (permalink / raw)
To: Linus Walleij
Cc: Colin Ian King, linux-kernel, linux-fbdev, Russell King, Tomi Valkeinen
On Monday 29 August 2016, Linus Walleij wrote:
> On Fri, Aug 26, 2016 at 5:25 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> [I don't see why Torvalds was CC'ed on this patch? Was there
> some specific complaint from his side that I screw things up
> or just the wrong Linus?]
It was intentional: as mentioned in the introductory mail, he did
the patch that prevented you from seeing the five warnings that I got
after reverting 6e8d666e9253.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic
2016-08-29 13:18 ` Linus Walleij
2016-08-29 13:41 ` Arnd Bergmann
@ 2016-08-30 8:35 ` Tomi Valkeinen
1 sibling, 0 replies; 17+ messages in thread
From: Tomi Valkeinen @ 2016-08-30 8:35 UTC (permalink / raw)
To: Linus Walleij, Arnd Bergmann, Colin Ian King
Cc: linux-kernel, linux-fbdev, Russell King
[-- Attachment #1.1: Type: text/plain, Size: 1380 bytes --]
On 29/08/16 16:18, Linus Walleij wrote:
> On Fri, Aug 26, 2016 at 5:25 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> [I don't see why Torvalds was CC'ed on this patch? Was there
> some specific complaint from his side that I screw things up
> or just the wrong Linus?]
>
>> The addition of the Nomadik support in this driver introduced
>> a bug in clcdfb_of_init_display(), which now calls init_panel
>> with an uninitialized 'endpoint' pointer, as "gcc -Wmaybe-uninitialized"
>> warns:
>>
>> drivers/video/fbdev/amba-clcd.c: In function 'clcdfb_of_init_display':
>> drivers/video/fbdev/amba-clcd.c:785:5: error: 'endpoint' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>
>> This reverts the broken part of the function to what it was before
>> the patch, which is the best guess I have to what it should be.
>> I assume this was left over from an attempted rework of the
>> code that was partially backed out.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Fixes: 046ad6cdeb3f ("video: ARM CLCD: support Nomadik variant")
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Tomi: I think this bug was also reported by Ian King, so suggest
> adding his
> Reported-by: Colin Ian King <colin.king@canonical.com>
>
Thanks, queued for v4.9.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/5] rxrpc: fix last_call processing
2016-08-26 15:25 [PATCH 0/5] -Wmaybe-uninitialized bug fixes for linux-next Arnd Bergmann
2016-08-26 15:25 ` [PATCH 1/5] gpio: pca954x: fix undefined error code from remove Arnd Bergmann
2016-08-26 15:25 ` [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic Arnd Bergmann
@ 2016-08-26 15:25 ` Arnd Bergmann
2016-08-26 15:25 ` [PATCH 4/5] net_sched: fix use of uninitialized ethertype variable in cls_flower Arnd Bergmann
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2016-08-26 15:25 UTC (permalink / raw)
To: linux-kernel
Cc: Linus Torvalds, Arnd Bergmann, David Howells, David S. Miller, netdev
A change to the retransmission handling in rxrpc caused a use-before-init
bug in rxrpc_data_ready(), as indicated by "gcc -Wmaybe-uninitialized":
net/rxrpc/input.c: In function 'rxrpc_data_ready':
net/rxrpc/input.c:735:34: error: 'call' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This moves the initialization of the local variable before the first
user, which presumably is what was intended here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 18bfeba50dfd ("rxrpc: Perform terminal call ACK/ABORT retransmission from conn processor")
---
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
net/rxrpc/input.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 66cdeb56f44f..3c22e43a58fd 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -728,6 +728,10 @@ void rxrpc_data_ready(struct sock *sk)
if (sp->hdr.callNumber < chan->last_call)
goto discard_unlock;
+ call = rcu_dereference(chan->call);
+ if (!call || atomic_read(&call->usage) == 0)
+ goto cant_route_call;
+
if (sp->hdr.callNumber == chan->last_call) {
/* For the previous service call, if completed
* successfully, we discard all further packets.
@@ -744,10 +748,6 @@ void rxrpc_data_ready(struct sock *sk)
goto out_unlock;
}
- call = rcu_dereference(chan->call);
- if (!call || atomic_read(&call->usage) == 0)
- goto cant_route_call;
-
rxrpc_post_packet_to_call(call, skb);
goto out_unlock;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] net_sched: fix use of uninitialized ethertype variable in cls_flower
2016-08-26 15:25 [PATCH 0/5] -Wmaybe-uninitialized bug fixes for linux-next Arnd Bergmann
` (2 preceding siblings ...)
2016-08-26 15:25 ` [PATCH 3/5] rxrpc: fix last_call processing Arnd Bergmann
@ 2016-08-26 15:25 ` Arnd Bergmann
2016-08-29 4:30 ` David Miller
2016-08-26 15:25 ` [PATCH 5/5] net/xgene: fix error handling during reset Arnd Bergmann
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2016-08-26 15:25 UTC (permalink / raw)
To: linux-kernel
Cc: Linus Torvalds, Arnd Bergmann, Hadar Hen Zion, Jiri Pirko,
David S . Miller, netdev
The addition of VLAN support caused a possible use of uninitialized
data if we encounter a zero TCA_FLOWER_KEY_ETH_TYPE key, as pointed
out by "gcc -Wmaybe-uninitialized":
net/sched/cls_flower.c: In function 'fl_change':
net/sched/cls_flower.c:366:22: error: 'ethertype' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This changes the code to only set the ethertype field if it
was nonzero, as before the patch.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 9399ae9a6cb2 ("net_sched: flower: Add vlan support")
Cc: Hadar Hen Zion <hadarh@mellanox.com>
Cc: Jiri Pirko <jiri@mellanox.com>
---
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
net/sched/cls_flower.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 532ab6751343..cf9ad5b50889 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -353,18 +353,19 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
mask->eth.src, TCA_FLOWER_KEY_ETH_SRC_MASK,
sizeof(key->eth.src));
- if (tb[TCA_FLOWER_KEY_ETH_TYPE])
+ if (tb[TCA_FLOWER_KEY_ETH_TYPE]) {
ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_ETH_TYPE]);
- if (ethertype == htons(ETH_P_8021Q)) {
- fl_set_key_vlan(tb, &key->vlan, &mask->vlan);
- fl_set_key_val(tb, &key->basic.n_proto,
- TCA_FLOWER_KEY_VLAN_ETH_TYPE,
- &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
- sizeof(key->basic.n_proto));
- } else {
- key->basic.n_proto = ethertype;
- mask->basic.n_proto = cpu_to_be16(~0);
+ if (ethertype == htons(ETH_P_8021Q)) {
+ fl_set_key_vlan(tb, &key->vlan, &mask->vlan);
+ fl_set_key_val(tb, &key->basic.n_proto,
+ TCA_FLOWER_KEY_VLAN_ETH_TYPE,
+ &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
+ sizeof(key->basic.n_proto));
+ } else {
+ key->basic.n_proto = ethertype;
+ mask->basic.n_proto = cpu_to_be16(~0);
+ }
}
if (key->basic.n_proto == htons(ETH_P_IP) ||
--
2.9.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] net_sched: fix use of uninitialized ethertype variable in cls_flower
2016-08-26 15:25 ` [PATCH 4/5] net_sched: fix use of uninitialized ethertype variable in cls_flower Arnd Bergmann
@ 2016-08-29 4:30 ` David Miller
0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2016-08-29 4:30 UTC (permalink / raw)
To: arnd; +Cc: linux-kernel, torvalds, hadarh, jiri, netdev
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 26 Aug 2016 17:25:45 +0200
> The addition of VLAN support caused a possible use of uninitialized
> data if we encounter a zero TCA_FLOWER_KEY_ETH_TYPE key, as pointed
> out by "gcc -Wmaybe-uninitialized":
>
> net/sched/cls_flower.c: In function 'fl_change':
> net/sched/cls_flower.c:366:22: error: 'ethertype' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This changes the code to only set the ethertype field if it
> was nonzero, as before the patch.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 9399ae9a6cb2 ("net_sched: flower: Add vlan support")
Applied.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/5] net/xgene: fix error handling during reset
2016-08-26 15:25 [PATCH 0/5] -Wmaybe-uninitialized bug fixes for linux-next Arnd Bergmann
` (3 preceding siblings ...)
2016-08-26 15:25 ` [PATCH 4/5] net_sched: fix use of uninitialized ethertype variable in cls_flower Arnd Bergmann
@ 2016-08-26 15:25 ` Arnd Bergmann
2016-08-29 4:31 ` David Miller
2016-08-27 7:01 ` [PATCH 3/5] rxrpc: fix last_call processing David Howells
2016-08-28 8:42 ` David Howells
6 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2016-08-26 15:25 UTC (permalink / raw)
To: linux-kernel
Cc: Linus Torvalds, Arnd Bergmann, Fushen Chen, Iyappan Subramanian,
Keyur Chudgar, David S. Miller, netdev
The newly added reset logic uses helper functions for the MMIO that
may fail. However, when the read operation fails, we end up writing
back uninitialized data to the register, as gcc warns:
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c: In function 'xgene_enet_link_state':
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c:213:2: error: 'data' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c:209:6: note: 'data' was declared here
u32 data;
We already print a warning to the console log if that happens,
the best alternative that I can see is skip the rest of the reset
sequence if the register value cannot be read: Most likely the
write would fail as well, and if it succeeded, worse things could
happen.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 3eb7cb9dc946 ("drivers: net: xgene: XFI PCS reset when link is down")
Cc: Fushen Chen <fchen@apm.com>
---
Cc: Iyappan Subramanian <isubramanian@apm.com>
Cc: Keyur Chudgar <kchudgar@apm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
index d672e71b5a50..279ee27004f7 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
@@ -155,19 +155,23 @@ static void xgene_enet_rd_mac(struct xgene_enet_pdata *pdata,
rd_addr);
}
-static void xgene_enet_rd_pcs(struct xgene_enet_pdata *pdata,
+static bool xgene_enet_rd_pcs(struct xgene_enet_pdata *pdata,
u32 rd_addr, u32 *rd_data)
{
void __iomem *addr, *rd, *cmd, *cmd_done;
+ bool success;
addr = pdata->pcs_addr + PCS_ADDR_REG_OFFSET;
rd = pdata->pcs_addr + PCS_READ_REG_OFFSET;
cmd = pdata->pcs_addr + PCS_COMMAND_REG_OFFSET;
cmd_done = pdata->pcs_addr + PCS_COMMAND_DONE_REG_OFFSET;
- if (!xgene_enet_rd_indirect(addr, rd, cmd, cmd_done, rd_addr, rd_data))
+ success = xgene_enet_rd_indirect(addr, rd, cmd, cmd_done, rd_addr, rd_data);
+ if (!success)
netdev_err(pdata->ndev, "PCS read failed, addr: %04x\n",
rd_addr);
+
+ return success;
}
static int xgene_enet_ecc_init(struct xgene_enet_pdata *pdata)
@@ -208,7 +212,9 @@ static void xgene_pcs_reset(struct xgene_enet_pdata *pdata)
{
u32 data;
- xgene_enet_rd_pcs(pdata, PCS_CONTROL_1, &data);
+ if (!xgene_enet_rd_pcs(pdata, PCS_CONTROL_1, &data))
+ return;
+
xgene_enet_wr_pcs(pdata, PCS_CONTROL_1, data | PCS_CTRL_PCS_RST);
xgene_enet_wr_pcs(pdata, PCS_CONTROL_1, data & ~PCS_CTRL_PCS_RST);
}
--
2.9.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] net/xgene: fix error handling during reset
2016-08-26 15:25 ` [PATCH 5/5] net/xgene: fix error handling during reset Arnd Bergmann
@ 2016-08-29 4:31 ` David Miller
0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2016-08-29 4:31 UTC (permalink / raw)
To: arnd; +Cc: linux-kernel, torvalds, fchen, isubramanian, kchudgar, netdev
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 26 Aug 2016 17:25:46 +0200
> The newly added reset logic uses helper functions for the MMIO that
> may fail. However, when the read operation fails, we end up writing
> back uninitialized data to the register, as gcc warns:
>
> drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c: In function 'xgene_enet_link_state':
> drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c:213:2: error: 'data' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c:209:6: note: 'data' was declared here
> u32 data;
>
> We already print a warning to the console log if that happens,
> the best alternative that I can see is skip the rest of the reset
> sequence if the register value cannot be read: Most likely the
> write would fail as well, and if it succeeded, worse things could
> happen.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 3eb7cb9dc946 ("drivers: net: xgene: XFI PCS reset when link is down")
Applied.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] rxrpc: fix last_call processing
2016-08-26 15:25 [PATCH 0/5] -Wmaybe-uninitialized bug fixes for linux-next Arnd Bergmann
` (4 preceding siblings ...)
2016-08-26 15:25 ` [PATCH 5/5] net/xgene: fix error handling during reset Arnd Bergmann
@ 2016-08-27 7:01 ` David Howells
2016-08-28 8:42 ` David Howells
6 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2016-08-27 7:01 UTC (permalink / raw)
To: Arnd Bergmann
Cc: dhowells, linux-kernel, Linus Torvalds, David S. Miller, netdev
Arnd Bergmann <arnd@arndb.de> wrote:
> A change to the retransmission handling in rxrpc caused a use-before-init
> bug in rxrpc_data_ready(), as indicated by "gcc -Wmaybe-uninitialized":
>
> net/rxrpc/input.c: In function 'rxrpc_data_ready':
> net/rxrpc/input.c:735:34: error: 'call' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This moves the initialization of the local variable before the first
> user, which presumably is what was intended here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 18bfeba50dfd ("rxrpc: Perform terminal call ACK/ABORT retransmission from conn processor")
> ---
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
>
> net/rxrpc/input.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
> index 66cdeb56f44f..3c22e43a58fd 100644
> --- a/net/rxrpc/input.c
> +++ b/net/rxrpc/input.c
> @@ -728,6 +728,10 @@ void rxrpc_data_ready(struct sock *sk)
> if (sp->hdr.callNumber < chan->last_call)
> goto discard_unlock;
>
> + call = rcu_dereference(chan->call);
> + if (!call || atomic_read(&call->usage) == 0)
> + goto cant_route_call;
> +
> if (sp->hdr.callNumber == chan->last_call) {
> /* For the previous service call, if completed
> * successfully, we discard all further packets.
> @@ -744,10 +748,6 @@ void rxrpc_data_ready(struct sock *sk)
> goto out_unlock;
> }
>
> - call = rcu_dereference(chan->call);
> - if (!call || atomic_read(&call->usage) == 0)
> - goto cant_route_call;
> -
> rxrpc_post_packet_to_call(call, skb);
> goto out_unlock;
> }
You can't rearrange these like this. I have a different fix.
David
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] rxrpc: fix last_call processing
2016-08-26 15:25 [PATCH 0/5] -Wmaybe-uninitialized bug fixes for linux-next Arnd Bergmann
` (5 preceding siblings ...)
2016-08-27 7:01 ` [PATCH 3/5] rxrpc: fix last_call processing David Howells
@ 2016-08-28 8:42 ` David Howells
2016-08-31 11:53 ` Arnd Bergmann
2016-08-31 20:58 ` David Howells
6 siblings, 2 replies; 17+ messages in thread
From: David Howells @ 2016-08-28 8:42 UTC (permalink / raw)
To: Arnd Bergmann
Cc: dhowells, linux-kernel, Linus Torvalds, David S. Miller, netdev
This is fixed by:
commit 2266ffdef5737fdfa96005204fc5606dbd559956
subject: rxrpc: Fix conn-based retransmit
which is in net-next.
David
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] rxrpc: fix last_call processing
2016-08-28 8:42 ` David Howells
@ 2016-08-31 11:53 ` Arnd Bergmann
2016-08-31 20:58 ` David Howells
1 sibling, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2016-08-31 11:53 UTC (permalink / raw)
To: David Howells; +Cc: linux-kernel, Linus Torvalds, David S. Miller, netdev
On Sunday, August 28, 2016 9:42:17 AM CEST David Howells wrote:
> This is fixed by:
>
> commit 2266ffdef5737fdfa96005204fc5606dbd559956
> subject: rxrpc: Fix conn-based retransmit
>
> which is in net-next.
I've merged net-next into the last linux-next release now for
testing (no linux-next this week) and can confirm that your
fix is correct. However, I got a new (valid) warning after
your f5c17aaeb2ae ("rxrpc: Calls should only have one terminal
state"), and another (false-positive) one for another patch
in net-next.
I'll follow up with the fixes, both of which are rather
straightforward.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] rxrpc: fix last_call processing
2016-08-28 8:42 ` David Howells
2016-08-31 11:53 ` Arnd Bergmann
@ 2016-08-31 20:58 ` David Howells
1 sibling, 0 replies; 17+ messages in thread
From: David Howells @ 2016-08-31 20:58 UTC (permalink / raw)
To: Arnd Bergmann
Cc: dhowells, linux-kernel, Linus Torvalds, David S. Miller, netdev
Arnd Bergmann <arnd@arndb.de> wrote:
> I'll follow up with the fixes, both of which are rather
> straightforward.
Are they both in?
[PATCH 2/2] rxrpc: fix undefined behavior in rxrpc_mark_call_released
David
^ permalink raw reply [flat|nested] 17+ messages in thread