LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] Assorted fixes discovered with gcc 4.1
@ 2019-05-28 14:24 Geert Uytterhoeven
  2019-05-28 14:24 ` [PATCH 1/5] lightnvm: Fix uninitialized pointer in nvm_remove_tgt() Geert Uytterhoeven
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-05-28 14:24 UTC (permalink / raw)
  To: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Arnd Bergmann,
	Dan Carpenter
  Cc: linux-block, netdev, linux-afs, alsa-devel, linux-kernel,
	Geert Uytterhoeven

	Hi all,

Ever since commit cafa0010cd51fb71 ("Raise the minimum required gcc
version to 4.6"), I felt bored when looking at my test build logs, as I
was no longer discovering many real issues.  Hence I started wondering
if the modern gcc versions are really catching these classes of bugs
caught before with gcc 4.1, or if they just go undetected.

I reverted some changes and applied some fixes, which allowed me to
compile most of the kernel with gcc 4.1 again.  I built an
m68k/allmodconfig kernel, looked at all new warnings, and fixed the ones
that are not false positives.  The result is a patch series of 5
patches, of which one or two fix real bugs.

Thanks for your comments, and for applying where appropriate!

Geert Uytterhoeven (5):
  lightnvm: Fix uninitialized pointer in nvm_remove_tgt()
  rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  net: sched: pie: Use ULL suffix for 64-bit constant
  ALSA: fireface: Use ULL suffixes for 64-bit constants
  [RFC] devlink: Fix uninitialized error code in
    devlink_fmsg_prepare_skb()

 drivers/lightnvm/core.c                      |  2 +-
 net/core/devlink.c                           |  2 +-
 net/rxrpc/output.c                           |  4 +++-
 net/sched/sch_pie.c                          |  2 +-
 sound/firewire/fireface/ff-protocol-latter.c | 10 +++++-----
 5 files changed, 11 insertions(+), 9 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

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

* [PATCH 1/5] lightnvm: Fix uninitialized pointer in nvm_remove_tgt()
  2019-05-28 14:24 [PATCH 0/5] Assorted fixes discovered with gcc 4.1 Geert Uytterhoeven
@ 2019-05-28 14:24 ` Geert Uytterhoeven
  2019-05-29  8:08   ` Matias Bjørling
  2019-05-28 14:24 ` [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet() Geert Uytterhoeven
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-05-28 14:24 UTC (permalink / raw)
  To: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Arnd Bergmann,
	Dan Carpenter
  Cc: linux-block, netdev, linux-afs, alsa-devel, linux-kernel,
	Geert Uytterhoeven

With gcc 4.1:

    drivers/lightnvm/core.c: In function ‘nvm_remove_tgt’:
    drivers/lightnvm/core.c:510: warning: ‘t’ is used uninitialized in this function

Indeed, if no NVM devices have been registered, t will be an
uninitialized pointer, and may be dereferenced later.  A call to
nvm_remove_tgt() can be triggered from userspace by issuing the
NVM_DEV_REMOVE ioctl on the lightnvm control device.

Fix this by preinitializing t to NULL.

Fixes: 843f2edbdde085b4 ("lightnvm: do not remove instance under global lock")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/lightnvm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 0df7454832efe082..aa017f48eb8c588c 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -492,7 +492,7 @@ static void __nvm_remove_target(struct nvm_target *t, bool graceful)
  */
 static int nvm_remove_tgt(struct nvm_ioctl_remove *remove)
 {
-	struct nvm_target *t;
+	struct nvm_target *t = NULL;
 	struct nvm_dev *dev;
 
 	down_read(&nvm_lock);
-- 
2.17.1


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

* [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  2019-05-28 14:24 [PATCH 0/5] Assorted fixes discovered with gcc 4.1 Geert Uytterhoeven
  2019-05-28 14:24 ` [PATCH 1/5] lightnvm: Fix uninitialized pointer in nvm_remove_tgt() Geert Uytterhoeven
@ 2019-05-28 14:24 ` Geert Uytterhoeven
  2019-05-29 11:49   ` Arnd Bergmann
  2019-05-28 14:24 ` [PATCH 3/5] net: sched: pie: Use ULL suffix for 64-bit constant Geert Uytterhoeven
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-05-28 14:24 UTC (permalink / raw)
  To: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Arnd Bergmann,
	Dan Carpenter
  Cc: linux-block, netdev, linux-afs, alsa-devel, linux-kernel,
	Geert Uytterhoeven

With gcc 4.1:

    net/rxrpc/output.c: In function ‘rxrpc_send_data_packet’:
    net/rxrpc/output.c:338: warning: ‘ret’ may be used uninitialized in this function

Indeed, if the first jump to the send_fragmentable label is made, and
the address family is not handled in the switch() statement, ret will be
used uninitialized.

Fix this by initializing err to zero before the jump, like is already
done for the jump to the done label.

Fixes: 5a924b8951f835b5 ("rxrpc: Don't store the rxrpc header in the Tx queue sk_buffs")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
While this is not a real false-positive, I believe it cannot cause harm
in practice, as AF_RXRPC cannot be used with other transport families
than IPv4 and IPv6.
---
 net/rxrpc/output.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 004c762c2e8d063c..1473d774d67100c5 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -403,8 +403,10 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
 
 	/* send the packet with the don't fragment bit set if we currently
 	 * think it's small enough */
-	if (iov[1].iov_len >= call->peer->maxdata)
+	if (iov[1].iov_len >= call->peer->maxdata) {
+		ret = 0;
 		goto send_fragmentable;
+	}
 
 	down_read(&conn->params.local->defrag_sem);
 
-- 
2.17.1


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

* [PATCH 3/5] net: sched: pie: Use ULL suffix for 64-bit constant
  2019-05-28 14:24 [PATCH 0/5] Assorted fixes discovered with gcc 4.1 Geert Uytterhoeven
  2019-05-28 14:24 ` [PATCH 1/5] lightnvm: Fix uninitialized pointer in nvm_remove_tgt() Geert Uytterhoeven
  2019-05-28 14:24 ` [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet() Geert Uytterhoeven
@ 2019-05-28 14:24 ` Geert Uytterhoeven
  2019-05-29 11:39   ` Arnd Bergmann
  2019-05-28 14:24 ` [PATCH 4/5] ALSA: fireface: Use ULL suffixes for 64-bit constants Geert Uytterhoeven
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-05-28 14:24 UTC (permalink / raw)
  To: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Arnd Bergmann,
	Dan Carpenter
  Cc: linux-block, netdev, linux-afs, alsa-devel, linux-kernel,
	Geert Uytterhoeven

With gcc 4.1, when compiling for a 32-bit platform:

    net/sched/sch_pie.c: In function ‘drop_early’:
    net/sched/sch_pie.c:116: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:138: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:144: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:147: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c: In function ‘pie_qdisc_enqueue’:
    net/sched/sch_pie.c:173: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c: In function ‘calculate_probability’:
    net/sched/sch_pie.c:371: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:372: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:377: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:382: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:397: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:398: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:399: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:407: warning: integer constant is too large for ‘long’ type
    net/sched/sch_pie.c:414: warning: integer constant is too large for ‘long’ type

Fix this by adding the missing "ULL" suffix.

Fixes: 3f7ae5f3dc5295ac ("net: sched: pie: add more cases to auto-tune alpha and beta")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 net/sched/sch_pie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 8fa129d3943e32ad..f3424833e6a7cd3b 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -31,7 +31,7 @@
 
 #define QUEUE_THRESHOLD 16384
 #define DQCOUNT_INVALID -1
-#define MAX_PROB 0xffffffffffffffff
+#define MAX_PROB 0xffffffffffffffffULL
 #define PIE_SCALE 8
 
 /* parameters used */
-- 
2.17.1


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

* [PATCH 4/5] ALSA: fireface: Use ULL suffixes for 64-bit constants
  2019-05-28 14:24 [PATCH 0/5] Assorted fixes discovered with gcc 4.1 Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2019-05-28 14:24 ` [PATCH 3/5] net: sched: pie: Use ULL suffix for 64-bit constant Geert Uytterhoeven
@ 2019-05-28 14:24 ` Geert Uytterhoeven
  2019-05-29  9:57   ` Takashi Sakamoto
  2019-05-28 14:24 ` [PATCH 5/5] [RFC] devlink: Fix uninitialized error code in devlink_fmsg_prepare_skb() Geert Uytterhoeven
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-05-28 14:24 UTC (permalink / raw)
  To: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Arnd Bergmann,
	Dan Carpenter
  Cc: linux-block, netdev, linux-afs, alsa-devel, linux-kernel,
	Geert Uytterhoeven

With gcc 4.1:

    sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_switch_fetching_mode’:
    sound/firewire/fireface/ff-protocol-latter.c:97: warning: integer constant is too large for ‘long’ type
    sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_begin_session’:
    sound/firewire/fireface/ff-protocol-latter.c:170: warning: integer constant is too large for ‘long’ type
    sound/firewire/fireface/ff-protocol-latter.c:197: warning: integer constant is too large for ‘long’ type
    sound/firewire/fireface/ff-protocol-latter.c:205: warning: integer constant is too large for ‘long’ type
    sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_finish_session’:
    sound/firewire/fireface/ff-protocol-latter.c:214: warning: integer constant is too large for ‘long’ type

Fix this by adding the missing "ULL" suffixes.
Add the same suffix to the last constant, to maintain consistency.

Fixes: fd1cc9de64c2ca6c ("ALSA: fireface: add support for Fireface UCX")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 sound/firewire/fireface/ff-protocol-latter.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sound/firewire/fireface/ff-protocol-latter.c b/sound/firewire/fireface/ff-protocol-latter.c
index c8236ff89b7fb9de..b30d02d359b1d21b 100644
--- a/sound/firewire/fireface/ff-protocol-latter.c
+++ b/sound/firewire/fireface/ff-protocol-latter.c
@@ -9,11 +9,11 @@
 
 #include "ff.h"
 
-#define LATTER_STF		0xffff00000004
-#define LATTER_ISOC_CHANNELS	0xffff00000008
-#define LATTER_ISOC_START	0xffff0000000c
-#define LATTER_FETCH_MODE	0xffff00000010
-#define LATTER_SYNC_STATUS	0x0000801c0000
+#define LATTER_STF		0xffff00000004ULL
+#define LATTER_ISOC_CHANNELS	0xffff00000008ULL
+#define LATTER_ISOC_START	0xffff0000000cULL
+#define LATTER_FETCH_MODE	0xffff00000010ULL
+#define LATTER_SYNC_STATUS	0x0000801c0000ULL
 
 static int parse_clock_bits(u32 data, unsigned int *rate,
 			    enum snd_ff_clock_src *src)
-- 
2.17.1


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

* [PATCH 5/5] [RFC] devlink: Fix uninitialized error code in devlink_fmsg_prepare_skb()
  2019-05-28 14:24 [PATCH 0/5] Assorted fixes discovered with gcc 4.1 Geert Uytterhoeven
                   ` (3 preceding siblings ...)
  2019-05-28 14:24 ` [PATCH 4/5] ALSA: fireface: Use ULL suffixes for 64-bit constants Geert Uytterhoeven
@ 2019-05-28 14:24 ` Geert Uytterhoeven
  2019-05-28 14:40   ` Eran Ben Elisha
  2019-05-29  8:04   ` Jiri Pirko
  2019-05-28 16:30 ` [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet() David Howells
  2019-05-31 10:34 ` [PATCH] " David Howells
  6 siblings, 2 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-05-28 14:24 UTC (permalink / raw)
  To: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Arnd Bergmann,
	Dan Carpenter
  Cc: linux-block, netdev, linux-afs, alsa-devel, linux-kernel,
	Geert Uytterhoeven

With gcc 4.1:

    net/core/devlink.c: In function ‘devlink_fmsg_prepare_skb’:
    net/core/devlink.c:4325: warning: ‘err’ may be used uninitialized in this function

Indeed, if the list has less than *start entries, an uninitialized error
code will be returned.

Fix this by preinitializing err to zero.

Fixes: 1db64e8733f65381 ("devlink: Add devlink formatted message (fmsg) API")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
I don't know if this can really happen, and if this is the right fix.
Perhaps err should be initialized to some valid error code instead?
---
 net/core/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index d43bc52b8840d76b..91377e4eae9a43c1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4321,8 +4321,8 @@ devlink_fmsg_prepare_skb(struct devlink_fmsg *fmsg, struct sk_buff *skb,
 {
 	struct devlink_fmsg_item *item;
 	struct nlattr *fmsg_nlattr;
+	int err = 0;
 	int i = 0;
-	int err;
 
 	fmsg_nlattr = nla_nest_start_noflag(skb, DEVLINK_ATTR_FMSG);
 	if (!fmsg_nlattr)
-- 
2.17.1


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

* Re: [PATCH 5/5] [RFC] devlink: Fix uninitialized error code in devlink_fmsg_prepare_skb()
  2019-05-28 14:24 ` [PATCH 5/5] [RFC] devlink: Fix uninitialized error code in devlink_fmsg_prepare_skb() Geert Uytterhoeven
@ 2019-05-28 14:40   ` Eran Ben Elisha
  2019-05-29  8:04   ` Jiri Pirko
  1 sibling, 0 replies; 20+ messages in thread
From: Eran Ben Elisha @ 2019-05-28 14:40 UTC (permalink / raw)
  To: Geert Uytterhoeven, Igor Konopko, David Howells,
	Mohit P . Tahiliani, Takashi Sakamoto, Matias Bjorling,
	Jiri Pirko, David S . Miller, Jamal Hadi Salim, Cong Wang,
	Clemens Ladisch, Jaroslav Kysela, Takashi Iwai, Joe Perches,
	Arnd Bergmann, Dan Carpenter
  Cc: linux-block, netdev, linux-afs, alsa-devel, linux-kernel



On 5/28/2019 5:24 PM, Geert Uytterhoeven wrote:
> With gcc 4.1:
> 
>      net/core/devlink.c: In function ‘devlink_fmsg_prepare_skb’:
>      net/core/devlink.c:4325: warning: ‘err’ may be used uninitialized in this function
> 
> Indeed, if the list has less than *start entries, an uninitialized error
> code will be returned.

The logic guarantees that start is smaller than the length of the list.
but I guess that the compiler can't detect that.

Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>

> 
> Fix this by preinitializing err to zero.
> 
> Fixes: 1db64e8733f65381 ("devlink: Add devlink formatted message (fmsg) API")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> I don't know if this can really happen, and if this is the right fix.
> Perhaps err should be initialized to some valid error code instead?
> ---
>   net/core/devlink.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index d43bc52b8840d76b..91377e4eae9a43c1 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -4321,8 +4321,8 @@ devlink_fmsg_prepare_skb(struct devlink_fmsg *fmsg, struct sk_buff *skb,
>   {
>   	struct devlink_fmsg_item *item;
>   	struct nlattr *fmsg_nlattr;
> +	int err = 0;
>   	int i = 0;
> -	int err;
>   
>   	fmsg_nlattr = nla_nest_start_noflag(skb, DEVLINK_ATTR_FMSG);
>   	if (!fmsg_nlattr)
> 

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

* Re: [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  2019-05-28 14:24 [PATCH 0/5] Assorted fixes discovered with gcc 4.1 Geert Uytterhoeven
                   ` (4 preceding siblings ...)
  2019-05-28 14:24 ` [PATCH 5/5] [RFC] devlink: Fix uninitialized error code in devlink_fmsg_prepare_skb() Geert Uytterhoeven
@ 2019-05-28 16:30 ` David Howells
  2019-05-31 10:34 ` [PATCH] " David Howells
  6 siblings, 0 replies; 20+ messages in thread
From: David Howells @ 2019-05-28 16:30 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: dhowells, Igor Konopko, Mohit P . Tahiliani, Takashi Sakamoto,
	Eran Ben Elisha, Matias Bjorling, Jiri Pirko, David S . Miller,
	Jamal Hadi Salim, Cong Wang, Clemens Ladisch, Jaroslav Kysela,
	Takashi Iwai, Joe Perches, Arnd Bergmann, Dan Carpenter,
	linux-block, netdev, linux-afs, alsa-devel, linux-kernel

Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> While this is not a real false-positive, I believe it cannot cause harm
> in practice, as AF_RXRPC cannot be used with other transport families
> than IPv4 and IPv6.

Agreed.

> ---
>  net/rxrpc/output.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
> index 004c762c2e8d063c..1473d774d67100c5 100644
> --- a/net/rxrpc/output.c
> +++ b/net/rxrpc/output.c
> @@ -403,8 +403,10 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
>  
>  	/* send the packet with the don't fragment bit set if we currently
>  	 * think it's small enough */
> -	if (iov[1].iov_len >= call->peer->maxdata)
> +	if (iov[1].iov_len >= call->peer->maxdata) {
> +		ret = 0;
>  		goto send_fragmentable;
> +	}
>  
>  	down_read(&conn->params.local->defrag_sem);
>  

Simply setting 0 is wrong.  That would give the impression that the thing
worked if support for a new transport address family was added and came
through this function without full modification (say AF_INET7 becomes a
thing).

A better way to do things would be to add a default case into the
send_fragmentable switch statement that either BUG's or sets -EAFNOSUPPORT.

David

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

* Re: [PATCH 5/5] [RFC] devlink: Fix uninitialized error code in devlink_fmsg_prepare_skb()
  2019-05-28 14:24 ` [PATCH 5/5] [RFC] devlink: Fix uninitialized error code in devlink_fmsg_prepare_skb() Geert Uytterhoeven
  2019-05-28 14:40   ` Eran Ben Elisha
@ 2019-05-29  8:04   ` Jiri Pirko
  1 sibling, 0 replies; 20+ messages in thread
From: Jiri Pirko @ 2019-05-29  8:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Arnd Bergmann,
	Dan Carpenter, linux-block, netdev, linux-afs, alsa-devel,
	linux-kernel

Tue, May 28, 2019 at 04:24:24PM CEST, geert@linux-m68k.org wrote:
>With gcc 4.1:
>
>    net/core/devlink.c: In function ‘devlink_fmsg_prepare_skb’:
>    net/core/devlink.c:4325: warning: ‘err’ may be used uninitialized in this function
>
>Indeed, if the list has less than *start entries, an uninitialized error
>code will be returned.
>
>Fix this by preinitializing err to zero.
>
>Fixes: 1db64e8733f65381 ("devlink: Add devlink formatted message (fmsg) API")
>Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>---
>I don't know if this can really happen, and if this is the right fix.
>Perhaps err should be initialized to some valid error code instead?

0 is correct here.
Acked-by: Jiri Pirko <jiri@mellanox.com>

Thanks!

>---
> net/core/devlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index d43bc52b8840d76b..91377e4eae9a43c1 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -4321,8 +4321,8 @@ devlink_fmsg_prepare_skb(struct devlink_fmsg *fmsg, struct sk_buff *skb,
> {
> 	struct devlink_fmsg_item *item;
> 	struct nlattr *fmsg_nlattr;
>+	int err = 0;
> 	int i = 0;
>-	int err;
> 
> 	fmsg_nlattr = nla_nest_start_noflag(skb, DEVLINK_ATTR_FMSG);
> 	if (!fmsg_nlattr)
>-- 
>2.17.1
>

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

* Re: [PATCH 1/5] lightnvm: Fix uninitialized pointer in nvm_remove_tgt()
  2019-05-28 14:24 ` [PATCH 1/5] lightnvm: Fix uninitialized pointer in nvm_remove_tgt() Geert Uytterhoeven
@ 2019-05-29  8:08   ` Matias Bjørling
  2019-05-29  8:12     ` Geert Uytterhoeven
  0 siblings, 1 reply; 20+ messages in thread
From: Matias Bjørling @ 2019-05-29  8:08 UTC (permalink / raw)
  To: Geert Uytterhoeven, Igor Konopko, David Howells,
	Mohit P . Tahiliani, Takashi Sakamoto, Eran Ben Elisha,
	Jiri Pirko, David S . Miller, Jamal Hadi Salim, Cong Wang,
	Clemens Ladisch, Jaroslav Kysela, Takashi Iwai, Joe Perches,
	Arnd Bergmann, Dan Carpenter
  Cc: linux-block, netdev, linux-afs, alsa-devel, linux-kernel

On 5/28/19 4:24 PM, Geert Uytterhoeven wrote:
> With gcc 4.1:
> 
>      drivers/lightnvm/core.c: In function ‘nvm_remove_tgt’:
>      drivers/lightnvm/core.c:510: warning: ‘t’ is used uninitialized in this function
> 
> Indeed, if no NVM devices have been registered, t will be an
> uninitialized pointer, and may be dereferenced later.  A call to
> nvm_remove_tgt() can be triggered from userspace by issuing the
> NVM_DEV_REMOVE ioctl on the lightnvm control device.
> 
> Fix this by preinitializing t to NULL.
> 
> Fixes: 843f2edbdde085b4 ("lightnvm: do not remove instance under global lock")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>   drivers/lightnvm/core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 0df7454832efe082..aa017f48eb8c588c 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -492,7 +492,7 @@ static void __nvm_remove_target(struct nvm_target *t, bool graceful)
>    */
>   static int nvm_remove_tgt(struct nvm_ioctl_remove *remove)
>   {
> -	struct nvm_target *t;
> +	struct nvm_target *t = NULL;
>   	struct nvm_dev *dev;
>   
>   	down_read(&nvm_lock);
> 

Thanks Geert. Would you like me to carry the patch?

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

* Re: [PATCH 1/5] lightnvm: Fix uninitialized pointer in nvm_remove_tgt()
  2019-05-29  8:08   ` Matias Bjørling
@ 2019-05-29  8:12     ` Geert Uytterhoeven
  0 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-05-29  8:12 UTC (permalink / raw)
  To: Matias Bjørling
  Cc: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Jiri Pirko, David S . Miller,
	Jamal Hadi Salim, Cong Wang, Clemens Ladisch, Jaroslav Kysela,
	Takashi Iwai, Joe Perches, Arnd Bergmann, Dan Carpenter,
	linux-block, netdev, linux-afs, ALSA Development Mailing List,
	Linux Kernel Mailing List

Hi Matias,

On Wed, May 29, 2019 at 10:08 AM Matias Bjørling <mb@lightnvm.io> wrote:
> On 5/28/19 4:24 PM, Geert Uytterhoeven wrote:
> > With gcc 4.1:
> >
> >      drivers/lightnvm/core.c: In function ‘nvm_remove_tgt’:
> >      drivers/lightnvm/core.c:510: warning: ‘t’ is used uninitialized in this function
> >
> > Indeed, if no NVM devices have been registered, t will be an
> > uninitialized pointer, and may be dereferenced later.  A call to
> > nvm_remove_tgt() can be triggered from userspace by issuing the
> > NVM_DEV_REMOVE ioctl on the lightnvm control device.
> >
> > Fix this by preinitializing t to NULL.
> >
> > Fixes: 843f2edbdde085b4 ("lightnvm: do not remove instance under global lock")
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> >   drivers/lightnvm/core.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> > index 0df7454832efe082..aa017f48eb8c588c 100644
> > --- a/drivers/lightnvm/core.c
> > +++ b/drivers/lightnvm/core.c
> > @@ -492,7 +492,7 @@ static void __nvm_remove_target(struct nvm_target *t, bool graceful)
> >    */
> >   static int nvm_remove_tgt(struct nvm_ioctl_remove *remove)
> >   {
> > -     struct nvm_target *t;
> > +     struct nvm_target *t = NULL;
> >       struct nvm_dev *dev;
> >
> >       down_read(&nvm_lock);
> >
>
> Thanks Geert. Would you like me to carry the patch?

Yes please. Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 4/5] ALSA: fireface: Use ULL suffixes for 64-bit constants
  2019-05-28 14:24 ` [PATCH 4/5] ALSA: fireface: Use ULL suffixes for 64-bit constants Geert Uytterhoeven
@ 2019-05-29  9:57   ` Takashi Sakamoto
  2019-05-29 10:07     ` Takashi Iwai
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Sakamoto @ 2019-05-29  9:57 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Eran Ben Elisha, Matias Bjorling, Jiri Pirko, David S . Miller,
	Jamal Hadi Salim, Cong Wang, Clemens Ladisch, Jaroslav Kysela,
	Takashi Iwai, Joe Perches, Arnd Bergmann, Dan Carpenter,
	linux-block, netdev, linux-afs, alsa-devel, linux-kernel

Hi,

On Tue, May 28, 2019 at 04:24:23PM +0200, Geert Uytterhoeven wrote:
> With gcc 4.1:
> 
>     sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_switch_fetching_mode’:
>     sound/firewire/fireface/ff-protocol-latter.c:97: warning: integer constant is too large for ‘long’ type
>     sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_begin_session’:
>     sound/firewire/fireface/ff-protocol-latter.c:170: warning: integer constant is too large for ‘long’ type
>     sound/firewire/fireface/ff-protocol-latter.c:197: warning: integer constant is too large for ‘long’ type
>     sound/firewire/fireface/ff-protocol-latter.c:205: warning: integer constant is too large for ‘long’ type
>     sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_finish_session’:
>     sound/firewire/fireface/ff-protocol-latter.c:214: warning: integer constant is too large for ‘long’ type
> 
> Fix this by adding the missing "ULL" suffixes.
> Add the same suffix to the last constant, to maintain consistency.
> 
> Fixes: fd1cc9de64c2ca6c ("ALSA: fireface: add support for Fireface UCX")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  sound/firewire/fireface/ff-protocol-latter.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Thanks for your care.

Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

> diff --git a/sound/firewire/fireface/ff-protocol-latter.c b/sound/firewire/fireface/ff-protocol-latter.c
> index c8236ff89b7fb9de..b30d02d359b1d21b 100644
> --- a/sound/firewire/fireface/ff-protocol-latter.c
> +++ b/sound/firewire/fireface/ff-protocol-latter.c
> @@ -9,11 +9,11 @@
>  
>  #include "ff.h"
>  
> -#define LATTER_STF		0xffff00000004
> -#define LATTER_ISOC_CHANNELS	0xffff00000008
> -#define LATTER_ISOC_START	0xffff0000000c
> -#define LATTER_FETCH_MODE	0xffff00000010
> -#define LATTER_SYNC_STATUS	0x0000801c0000
> +#define LATTER_STF		0xffff00000004ULL
> +#define LATTER_ISOC_CHANNELS	0xffff00000008ULL
> +#define LATTER_ISOC_START	0xffff0000000cULL
> +#define LATTER_FETCH_MODE	0xffff00000010ULL
> +#define LATTER_SYNC_STATUS	0x0000801c0000ULL
>  
>  static int parse_clock_bits(u32 data, unsigned int *rate,
>  			    enum snd_ff_clock_src *src)
> -- 
> 2.17.1
> 


Regards

Takashi Sakamoto

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

* Re: [PATCH 4/5] ALSA: fireface: Use ULL suffixes for 64-bit constants
  2019-05-29  9:57   ` Takashi Sakamoto
@ 2019-05-29 10:07     ` Takashi Iwai
  0 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-29 10:07 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: Geert Uytterhoeven, alsa-devel, Arnd Bergmann, David S . Miller,
	Cong Wang, Igor Konopko, Clemens Ladisch, Matias Bjorling,
	linux-afs, Eran Ben Elisha, Jiri Pirko, Jamal Hadi Salim,
	Mohit P . Tahiliani, Dan Carpenter, Joe Perches, Jaroslav Kysela,
	David Howells, linux-block, linux-kernel, netdev

On Wed, 29 May 2019 11:57:31 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Tue, May 28, 2019 at 04:24:23PM +0200, Geert Uytterhoeven wrote:
> > With gcc 4.1:
> > 
> >     sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_switch_fetching_mode’:
> >     sound/firewire/fireface/ff-protocol-latter.c:97: warning: integer constant is too large for ‘long’ type
> >     sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_begin_session’:
> >     sound/firewire/fireface/ff-protocol-latter.c:170: warning: integer constant is too large for ‘long’ type
> >     sound/firewire/fireface/ff-protocol-latter.c:197: warning: integer constant is too large for ‘long’ type
> >     sound/firewire/fireface/ff-protocol-latter.c:205: warning: integer constant is too large for ‘long’ type
> >     sound/firewire/fireface/ff-protocol-latter.c: In function ‘latter_finish_session’:
> >     sound/firewire/fireface/ff-protocol-latter.c:214: warning: integer constant is too large for ‘long’ type
> > 
> > Fix this by adding the missing "ULL" suffixes.
> > Add the same suffix to the last constant, to maintain consistency.
> > 
> > Fixes: fd1cc9de64c2ca6c ("ALSA: fireface: add support for Fireface UCX")
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> >  sound/firewire/fireface/ff-protocol-latter.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> Thanks for your care.
> 
> Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Applied to sound git tree now.  Thanks.


Takashi

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

* Re: [PATCH 3/5] net: sched: pie: Use ULL suffix for 64-bit constant
  2019-05-28 14:24 ` [PATCH 3/5] net: sched: pie: Use ULL suffix for 64-bit constant Geert Uytterhoeven
@ 2019-05-29 11:39   ` Arnd Bergmann
  0 siblings, 0 replies; 20+ messages in thread
From: Arnd Bergmann @ 2019-05-29 11:39 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Dan Carpenter,
	linux-block, Networking, linux-afs,
	ALSA Development Mailing List, Linux Kernel Mailing List

On Tue, May 28, 2019 at 4:24 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> With gcc 4.1, when compiling for a 32-bit platform:
>
>     net/sched/sch_pie.c: In function ‘drop_early’:
>     net/sched/sch_pie.c:116: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:138: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:144: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:147: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c: In function ‘pie_qdisc_enqueue’:
>     net/sched/sch_pie.c:173: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c: In function ‘calculate_probability’:
>     net/sched/sch_pie.c:371: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:372: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:377: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:382: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:397: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:398: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:399: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:407: warning: integer constant is too large for ‘long’ type
>     net/sched/sch_pie.c:414: warning: integer constant is too large for ‘long’ type
>
> Fix this by adding the missing "ULL" suffix.
>
> Fixes: 3f7ae5f3dc5295ac ("net: sched: pie: add more cases to auto-tune alpha and beta")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

I created patches for all instances of this issue at some point in the past,
but did not send those as we raised the minimum compiler version to one
that handles this in the expected way without a warning.

Maybe you can just ignore these as well?

      Arnd

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

* Re: [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  2019-05-28 14:24 ` [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet() Geert Uytterhoeven
@ 2019-05-29 11:49   ` Arnd Bergmann
  0 siblings, 0 replies; 20+ messages in thread
From: Arnd Bergmann @ 2019-05-29 11:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Igor Konopko, David Howells, Mohit P . Tahiliani,
	Takashi Sakamoto, Eran Ben Elisha, Matias Bjorling, Jiri Pirko,
	David S . Miller, Jamal Hadi Salim, Cong Wang, Clemens Ladisch,
	Jaroslav Kysela, Takashi Iwai, Joe Perches, Dan Carpenter,
	linux-block, Networking, linux-afs,
	ALSA Development Mailing List, Linux Kernel Mailing List,
	clang-built-linux

On Tue, May 28, 2019 at 4:24 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> With gcc 4.1:
>
>     net/rxrpc/output.c: In function ‘rxrpc_send_data_packet’:
>     net/rxrpc/output.c:338: warning: ‘ret’ may be used uninitialized in this function
>
> Indeed, if the first jump to the send_fragmentable label is made, and
> the address family is not handled in the switch() statement, ret will be
> used uninitialized.
>
> Fix this by initializing err to zero before the jump, like is already
> done for the jump to the done label.
>
> Fixes: 5a924b8951f835b5 ("rxrpc: Don't store the rxrpc header in the Tx queue sk_buffs")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> While this is not a real false-positive, I believe it cannot cause harm
> in practice, as AF_RXRPC cannot be used with other transport families
> than IPv4 and IPv6.

This looks like a variant of the infamous bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501

What I don't understand is why clang fails to warn about it with
-Wsometimes-uninitialized.
(cc clang-built-linux mailing list).

      Arnd

>  net/rxrpc/output.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
> index 004c762c2e8d063c..1473d774d67100c5 100644
> --- a/net/rxrpc/output.c
> +++ b/net/rxrpc/output.c
> @@ -403,8 +403,10 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
>
>         /* send the packet with the don't fragment bit set if we currently
>          * think it's small enough */
> -       if (iov[1].iov_len >= call->peer->maxdata)
> +       if (iov[1].iov_len >= call->peer->maxdata) {
> +               ret = 0;
>                 goto send_fragmentable;
> +       }
>
>         down_read(&conn->params.local->defrag_sem);
>

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

* [PATCH] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  2019-05-28 14:24 [PATCH 0/5] Assorted fixes discovered with gcc 4.1 Geert Uytterhoeven
                   ` (5 preceding siblings ...)
  2019-05-28 16:30 ` [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet() David Howells
@ 2019-05-31 10:34 ` David Howells
  2019-06-03 21:30   ` David Miller
                     ` (2 more replies)
  6 siblings, 3 replies; 20+ messages in thread
From: David Howells @ 2019-05-31 10:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: dhowells, Igor Konopko, Mohit P . Tahiliani, Takashi Sakamoto,
	Eran Ben Elisha, Matias Bjorling, Jiri Pirko, David S . Miller,
	Jamal Hadi Salim, Cong Wang, Clemens Ladisch, Jaroslav Kysela,
	Takashi Iwai, Joe Perches, Arnd Bergmann, Dan Carpenter,
	linux-block, netdev, linux-afs, alsa-devel, linux-kernel


Hi Geert,

Here's my take on the patch.

David
---
rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()    
    
With gcc 4.1:

    net/rxrpc/output.c: In function ‘rxrpc_send_data_packet’:
    net/rxrpc/output.c:338: warning: ‘ret’ may be used uninitialized in this function

Indeed, if the first jump to the send_fragmentable label is made, and
the address family is not handled in the switch() statement, ret will be
used uninitialized.

Fix this by BUG()'ing as is done in other places in rxrpc where internal
support for future address families will need adding.  It should not be
possible to reach this normally as the address families are checked
up-front.

Fixes: 5a924b8951f835b5 ("rxrpc: Don't store the rxrpc header in the Tx queue sk_buffs")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: David Howells <dhowells@redhat.com>
---
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 004c762c2e8d..6f2b4fb4b0aa 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -523,6 +523,9 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
 		}
 		break;
 #endif
+
+	default:
+		BUG();
 	}
 
 	if (ret < 0)

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

* Re: [PATCH] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  2019-05-31 10:34 ` [PATCH] " David Howells
@ 2019-06-03 21:30   ` David Miller
  2019-06-04  7:17   ` Geert Uytterhoeven
  2019-06-04  7:34   ` David Howells
  2 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2019-06-03 21:30 UTC (permalink / raw)
  To: dhowells
  Cc: geert, igor.j.konopko, tahiliani, o-takashi, eranbe, mb, jiri,
	jhs, xiyou.wangcong, clemens, perex, tiwai, joe, arnd,
	dan.carpenter, linux-block, netdev, linux-afs, alsa-devel,
	linux-kernel

From: David Howells <dhowells@redhat.com>
Date: Fri, 31 May 2019 11:34:44 +0100

> Here's my take on the patch.

I assume I'll see a final version of this under separate cover.

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

* Re: [PATCH] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  2019-05-31 10:34 ` [PATCH] " David Howells
  2019-06-03 21:30   ` David Miller
@ 2019-06-04  7:17   ` Geert Uytterhoeven
  2019-06-04  7:34   ` David Howells
  2 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-06-04  7:17 UTC (permalink / raw)
  To: David Howells
  Cc: Igor Konopko, Mohit P . Tahiliani, Takashi Sakamoto,
	Eran Ben Elisha, Matias Bjorling, Jiri Pirko, David S . Miller,
	Jamal Hadi Salim, Cong Wang, Clemens Ladisch, Jaroslav Kysela,
	Takashi Iwai, Joe Perches, Arnd Bergmann, Dan Carpenter,
	linux-block, netdev, linux-afs, ALSA Development Mailing List,
	Linux Kernel Mailing List

Hi David,

On Fri, May 31, 2019 at 12:35 PM David Howells <dhowells@redhat.com> wrote:
> Here's my take on the patch.
>
> David
> ---
> rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
>
> With gcc 4.1:
>
>     net/rxrpc/output.c: In function ‘rxrpc_send_data_packet’:
>     net/rxrpc/output.c:338: warning: ‘ret’ may be used uninitialized in this function
>
> Indeed, if the first jump to the send_fragmentable label is made, and
> the address family is not handled in the switch() statement, ret will be
> used uninitialized.
>
> Fix this by BUG()'ing as is done in other places in rxrpc where internal
> support for future address families will need adding.  It should not be
> possible to reach this normally as the address families are checked
> up-front.
>
> Fixes: 5a924b8951f835b5 ("rxrpc: Don't store the rxrpc header in the Tx queue sk_buffs")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: David Howells <dhowells@redhat.com>

I'm not such a big fan of BUG(), so I'd go for ret = -EAFNOSUPPORT, but given
rxrpc is already full of BUG() calls, I guess it is an acceptable solution.

> ---
> diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
> index 004c762c2e8d..6f2b4fb4b0aa 100644
> --- a/net/rxrpc/output.c
> +++ b/net/rxrpc/output.c
> @@ -523,6 +523,9 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
>                 }
>                 break;
>  #endif
> +
> +       default:
> +               BUG();
>         }
>
>         if (ret < 0)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  2019-05-31 10:34 ` [PATCH] " David Howells
  2019-06-03 21:30   ` David Miller
  2019-06-04  7:17   ` Geert Uytterhoeven
@ 2019-06-04  7:34   ` David Howells
  2019-06-04  7:35     ` Geert Uytterhoeven
  2 siblings, 1 reply; 20+ messages in thread
From: David Howells @ 2019-06-04  7:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: dhowells, Igor Konopko, Mohit P . Tahiliani, Takashi Sakamoto,
	Eran Ben Elisha, Matias Bjorling, Jiri Pirko, David S . Miller,
	Jamal Hadi Salim, Cong Wang, Clemens Ladisch, Jaroslav Kysela,
	Takashi Iwai, Joe Perches, Arnd Bergmann, Dan Carpenter,
	linux-block, netdev, linux-afs, ALSA Development Mailing List,
	Linux Kernel Mailing List

Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> I'm not such a big fan of BUG(), so I'd go for ret = -EAFNOSUPPORT, but given
> rxrpc is already full of BUG() calls, I guess it is an acceptable solution.

Okay.  Are you okay with this going through net-next?

David

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

* Re: [PATCH] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
  2019-06-04  7:34   ` David Howells
@ 2019-06-04  7:35     ` Geert Uytterhoeven
  0 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2019-06-04  7:35 UTC (permalink / raw)
  To: David Howells
  Cc: Igor Konopko, Mohit P . Tahiliani, Takashi Sakamoto,
	Eran Ben Elisha, Matias Bjorling, Jiri Pirko, David S . Miller,
	Jamal Hadi Salim, Cong Wang, Clemens Ladisch, Jaroslav Kysela,
	Takashi Iwai, Joe Perches, Arnd Bergmann, Dan Carpenter,
	linux-block, netdev, linux-afs, ALSA Development Mailing List,
	Linux Kernel Mailing List

Hi David,

On Tue, Jun 4, 2019 at 9:34 AM David Howells <dhowells@redhat.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> > I'm not such a big fan of BUG(), so I'd go for ret = -EAFNOSUPPORT, but given
> > rxrpc is already full of BUG() calls, I guess it is an acceptable solution.
>
> Okay.  Are you okay with this going through net-next?

Yes, I am.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

end of thread, other threads:[~2019-06-04  7:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28 14:24 [PATCH 0/5] Assorted fixes discovered with gcc 4.1 Geert Uytterhoeven
2019-05-28 14:24 ` [PATCH 1/5] lightnvm: Fix uninitialized pointer in nvm_remove_tgt() Geert Uytterhoeven
2019-05-29  8:08   ` Matias Bjørling
2019-05-29  8:12     ` Geert Uytterhoeven
2019-05-28 14:24 ` [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet() Geert Uytterhoeven
2019-05-29 11:49   ` Arnd Bergmann
2019-05-28 14:24 ` [PATCH 3/5] net: sched: pie: Use ULL suffix for 64-bit constant Geert Uytterhoeven
2019-05-29 11:39   ` Arnd Bergmann
2019-05-28 14:24 ` [PATCH 4/5] ALSA: fireface: Use ULL suffixes for 64-bit constants Geert Uytterhoeven
2019-05-29  9:57   ` Takashi Sakamoto
2019-05-29 10:07     ` Takashi Iwai
2019-05-28 14:24 ` [PATCH 5/5] [RFC] devlink: Fix uninitialized error code in devlink_fmsg_prepare_skb() Geert Uytterhoeven
2019-05-28 14:40   ` Eran Ben Elisha
2019-05-29  8:04   ` Jiri Pirko
2019-05-28 16:30 ` [PATCH 2/5] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet() David Howells
2019-05-31 10:34 ` [PATCH] " David Howells
2019-06-03 21:30   ` David Miller
2019-06-04  7:17   ` Geert Uytterhoeven
2019-06-04  7:34   ` David Howells
2019-06-04  7:35     ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).