linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/28] Reenable maybe-uninitialized warnings
@ 2016-10-17 22:03 Arnd Bergmann
  2016-10-17 22:05 ` [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning Arnd Bergmann
                   ` (28 more replies)
  0 siblings, 29 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:03 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Arnd Bergmann, x86, linux-media,
	Mauro Carvalho Chehab, Martin Schwidefsky, linux-s390,
	Ilya Dryomov, dri-devel, linux-mtd, Herbert Xu, linux-crypto,
	David S. Miller, netdev, Greg Kroah-Hartman, ceph-devel,
	linux-f2fs-devel, linux-ext4, netfilter-devel

This is a set of patches that I hope to get into v4.9 in some form
in order to turn on the -Wmaybe-uninitialized warnings again.

After talking to Linus in person at Linaro Connect about this, I
spent some time on finding all the remaining warnings, and this
is the resulting patch series. More details are in the description
of the last patch that actually enables the warning.

Let me know if there are other warnings that I missed, and whether
you think these are still appropriate for v4.9 or not.
A couple of patches are non-obvious, and could use some more
detailed review.

	Arnd

Arnd Bergmann (28):
  [v2] netfilter: nf_tables: avoid uninitialized variable warning
  [v2] mtd: mtk: avoid warning in mtk_ecc_encode
  [v2] infiniband: shut up a maybe-uninitialized warning
  f2fs: replace a build-time warning with runtime WARN_ON
  ext2: avoid bogus -Wmaybe-uninitialized warning
  NFSv4.1: work around -Wmaybe-uninitialized warning
  ceph: avoid false positive maybe-uninitialized warning
  staging: lustre: restore initialization of return code
  staging: lustre: remove broken dead code in
    cfs_cpt_table_create_pattern
  UBI: fix uninitialized access of vid_hdr pointer
  block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized
  [media] rc: print correct variable for z8f0811
  [media] dib0700: fix uninitialized data on 'repeat' event
  iio: accel: sca3000_core: avoid potentially uninitialized variable
  crypto: aesni: avoid -Wmaybe-uninitialized warning
  pcmcia: fix return value of soc_pcmcia_regulator_set
  spi: fsl-espi: avoid processing uninitalized data on error
  drm: avoid uninitialized timestamp use in wait_vblank
  brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  net: bcm63xx: avoid referencing uninitialized variable
  net/hyperv: avoid uninitialized variable
  x86: apm: avoid uninitialized data
  x86: mark target address as output in 'insb' asm
  x86: math-emu: possible uninitialized variable use
  s390: pci: don't print uninitialized data for debugging
  nios2: fix timer initcall return value
  rocker: fix maybe-uninitialized warning
  Kbuild: bring back -Wmaybe-uninitialized warning

 Makefile                                           |  10 +-
 arch/arc/Makefile                                  |   4 +-
 arch/nios2/kernel/time.c                           |   1 +
 arch/s390/pci/pci_dma.c                            |   2 +-
 arch/x86/crypto/aesni-intel_glue.c                 | 121 +++++++++++++--------
 arch/x86/include/asm/io.h                          |   4 +-
 arch/x86/kernel/apm_32.c                           |   5 +-
 arch/x86/math-emu/Makefile                         |   4 +-
 arch/x86/math-emu/reg_compare.c                    |  16 +--
 drivers/block/rbd.c                                |   1 +
 drivers/gpu/drm/drm_irq.c                          |   4 +-
 drivers/infiniband/core/cma.c                      |  56 +++++-----
 drivers/media/i2c/ir-kbd-i2c.c                     |   2 +-
 drivers/media/usb/dvb-usb/dib0700_core.c           |  10 +-
 drivers/mtd/nand/mtk_ecc.c                         |  19 ++--
 drivers/mtd/ubi/eba.c                              |   2 +-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c       |   3 +-
 drivers/net/ethernet/rocker/rocker_ofdpa.c         |   4 +-
 drivers/net/hyperv/netvsc_drv.c                    |   2 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         |   2 +-
 drivers/pcmcia/soc_common.c                        |   2 +-
 drivers/spi/spi-fsl-espi.c                         |   2 +-
 drivers/staging/iio/accel/sca3000_core.c           |   2 +
 .../staging/lustre/lnet/libcfs/linux/linux-cpu.c   |   7 --
 drivers/staging/lustre/lustre/lov/lov_pack.c       |   2 +
 fs/ceph/super.c                                    |   3 +-
 fs/ext2/inode.c                                    |   7 +-
 fs/f2fs/data.c                                     |   7 ++
 fs/nfs/nfs4session.c                               |  10 +-
 net/netfilter/nft_range.c                          |  10 +-
 scripts/Makefile.ubsan                             |   4 +
 31 files changed, 187 insertions(+), 141 deletions(-)

-- 
Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
2.9.0

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

* [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
@ 2016-10-17 22:05 ` Arnd Bergmann
  2016-10-18 15:23   ` Pablo Neira Ayuso
  2016-10-17 22:05 ` [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode Arnd Bergmann
                   ` (27 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:05 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik, David S. Miller
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, netfilter-devel,
	coreteam, netdev

The newly added nft_range_eval() function handles the two possible
nft range operations, but as the compiler warning points out,
any unexpected value would lead to the 'mismatch' variable being
used without being initialized:

net/netfilter/nft_range.c: In function 'nft_range_eval':
net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This removes the variable in question and instead moves the
condition into the switch itself, which is potentially more
efficient than adding a bogus 'default' clause as in my
first approach, and is nicer than using the 'uninitialized_var'
macro.

Fixes: 0f3cd9b36977 ("netfilter: nf_tables: add range expression")
Link: http://patchwork.ozlabs.org/patch/677114/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/netfilter/nft_range.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Cc: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nft_range.c b/net/netfilter/nft_range.c
index c6d5358..2dd80f4 100644
--- a/net/netfilter/nft_range.c
+++ b/net/netfilter/nft_range.c
@@ -28,22 +28,20 @@ static void nft_range_eval(const struct nft_expr *expr,
 			 const struct nft_pktinfo *pkt)
 {
 	const struct nft_range_expr *priv = nft_expr_priv(expr);
-	bool mismatch;
 	int d1, d2;
 
 	d1 = memcmp(&regs->data[priv->sreg], &priv->data_from, priv->len);
 	d2 = memcmp(&regs->data[priv->sreg], &priv->data_to, priv->len);
 	switch (priv->op) {
 	case NFT_RANGE_EQ:
-		mismatch = (d1 < 0 || d2 > 0);
+		if (d1 < 0 || d2 > 0)
+			regs->verdict.code = NFT_BREAK;
 		break;
 	case NFT_RANGE_NEQ:
-		mismatch = (d1 >= 0 && d2 <= 0);
+		if (d1 >= 0 && d2 <= 0)
+			regs->verdict.code = NFT_BREAK;
 		break;
 	}
-
-	if (mismatch)
-		regs->verdict.code = NFT_BREAK;
 }
 
 static const struct nla_policy nft_range_policy[NFTA_RANGE_MAX + 1] = {
-- 
2.9.0

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

* [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
  2016-10-17 22:05 ` [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning Arnd Bergmann
@ 2016-10-17 22:05 ` Arnd Bergmann
  2016-10-18  5:19   ` Boris Brezillon
  2016-10-17 22:05 ` [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning Arnd Bergmann
                   ` (26 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:05 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Richard Weinberger,
	David Woodhouse, Brian Norris, Matthias Brugger,
	Jorge Ramirez-Ortiz, RogerCC Lin, linux-mtd, linux-arm-kernel,
	linux-mediatek

When building with -Wmaybe-uninitialized, gcc produces a silly false positive
warning for the mtk_ecc_encode function:

drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]

The function for some reason contains a double byte swap on big-endian
builds to get the OOB data into the correct order again, and is written
in a slightly confusing way.

Using a simple memcpy32_fromio() to read the data simplifies it a lot
so it becomes more readable and produces no warning. However, the
output might not have 32-bit alignment, so we have to use another
memcpy to avoid taking alignment faults or writing beyond the end
of the array.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: move temporary buffer into struct mtk_ecc instead of having it
    on the stack, as suggested by Boris Brezillon
---
 drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index d54f666..dbf2562 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -86,6 +86,8 @@ struct mtk_ecc {
 	struct completion done;
 	struct mutex lock;
 	u32 sectors;
+
+	u8 eccdata[112];
 };
 
 static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
@@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 		   u8 *data, u32 bytes)
 {
 	dma_addr_t addr;
-	u8 *p;
-	u32 len, i, val;
-	int ret = 0;
+	u32 len;
+	int ret;
 
 	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
 	ret = dma_mapping_error(ecc->dev, addr);
@@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 
 	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
 	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
-	p = data + bytes;
 
-	/* write the parity bytes generated by the ECC back to the OOB region */
-	for (i = 0; i < len; i++) {
-		if ((i % 4) == 0)
-			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
-		p[i] = (val >> ((i % 4) * 8)) & 0xff;
-	}
+	/* write the parity bytes generated by the ECC back to temp buffer */
+	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
+
+	/* copy into possibly unaligned OOB region with actual length */
+	memcpy(data + bytes, ecc->eccdata, len);
 timeout:
 
 	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
-- 
2.9.0

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

* [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
  2016-10-17 22:05 ` [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning Arnd Bergmann
  2016-10-17 22:05 ` [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode Arnd Bergmann
@ 2016-10-17 22:05 ` Arnd Bergmann
  2016-10-18  6:47   ` Haggai Eran
  2016-10-17 22:05 ` [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON Arnd Bergmann
                   ` (25 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:05 UTC (permalink / raw)
  To: Doug Ledford
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Sean Hefty,
	Hal Rosenstock, Matan Barak, Haggai Eran, Leon Romanovsky,
	Sagi Grimberg, Bart Van Assche, Alex Vesker, Guy Shapiro,
	linux-rdma

Some configurations produce this harmless warning when built with
gcc -Wmaybe-uninitialized:

infiniband/core/cma.c: In function 'cma_get_net_dev':
infiniband/core/cma.c:1242:12: warning: 'src_addr_storage.sin_addr.s_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]

I previously reported this for the powerpc64 defconfig, but have now
reproduced the same thing for x86 as well, using gcc-5 or higher.

The code looks correct to me, and this change just rearranges it
by making sure we alway initialize the entire address structure
to make the warning disappear. My first approach added an
initialization at the time of the declaration, which Doug commented
may be too costly, so I hope this version doesn't add overhead.

Link: http://arm-soc.lixom.net/buildlogs/mainline/v4.7-rc6/buildall.powerpc.ppc64_defconfig.log.passed
Link: https://patchwork.kernel.org/patch/9212825/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/infiniband/core/cma.c | 56 ++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 36bf50e..24e0ea6 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1094,47 +1094,47 @@ static void cma_save_ib_info(struct sockaddr *src_addr,
 	}
 }
 
-static void cma_save_ip4_info(struct sockaddr *src_addr,
-			      struct sockaddr *dst_addr,
+static void cma_save_ip4_info(struct sockaddr_in *src_addr,
+			      struct sockaddr_in *dst_addr,
 			      struct cma_hdr *hdr,
 			      __be16 local_port)
 {
-	struct sockaddr_in *ip4;
-
 	if (src_addr) {
-		ip4 = (struct sockaddr_in *)src_addr;
-		ip4->sin_family = AF_INET;
-		ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
-		ip4->sin_port = local_port;
+		*src_addr = (struct sockaddr_in) {
+			.sin_family = AF_INET,
+			.sin_addr.s_addr = hdr->dst_addr.ip4.addr,
+			.sin_port = local_port,
+		};
 	}
 
 	if (dst_addr) {
-		ip4 = (struct sockaddr_in *)dst_addr;
-		ip4->sin_family = AF_INET;
-		ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
-		ip4->sin_port = hdr->port;
+		*dst_addr = (struct sockaddr_in) {
+			.sin_family = AF_INET,
+			.sin_addr.s_addr = hdr->src_addr.ip4.addr,
+			.sin_port = hdr->port,
+		};
 	}
 }
 
-static void cma_save_ip6_info(struct sockaddr *src_addr,
-			      struct sockaddr *dst_addr,
+static void cma_save_ip6_info(struct sockaddr_in6 *src_addr,
+			      struct sockaddr_in6 *dst_addr,
 			      struct cma_hdr *hdr,
 			      __be16 local_port)
 {
-	struct sockaddr_in6 *ip6;
-
 	if (src_addr) {
-		ip6 = (struct sockaddr_in6 *)src_addr;
-		ip6->sin6_family = AF_INET6;
-		ip6->sin6_addr = hdr->dst_addr.ip6;
-		ip6->sin6_port = local_port;
+		*src_addr = (struct sockaddr_in6) {
+			.sin6_family = AF_INET6,
+			.sin6_addr = hdr->dst_addr.ip6,
+			.sin6_port = local_port,
+		};
 	}
 
 	if (dst_addr) {
-		ip6 = (struct sockaddr_in6 *)dst_addr;
-		ip6->sin6_family = AF_INET6;
-		ip6->sin6_addr = hdr->src_addr.ip6;
-		ip6->sin6_port = hdr->port;
+		*dst_addr = (struct sockaddr_in6) {
+			.sin6_family = AF_INET6,
+			.sin6_addr = hdr->src_addr.ip6,
+			.sin6_port = hdr->port,
+		};
 	}
 }
 
@@ -1159,10 +1159,12 @@ static int cma_save_ip_info(struct sockaddr *src_addr,
 
 	switch (cma_get_ip_ver(hdr)) {
 	case 4:
-		cma_save_ip4_info(src_addr, dst_addr, hdr, port);
+		cma_save_ip4_info((struct sockaddr_in *)src_addr,
+				  (struct sockaddr_in *)dst_addr, hdr, port);
 		break;
 	case 6:
-		cma_save_ip6_info(src_addr, dst_addr, hdr, port);
+		cma_save_ip6_info((struct sockaddr_in6 *)src_addr,
+				  (struct sockaddr_in6 *)dst_addr, hdr, port);
 		break;
 	default:
 		return -EAFNOSUPPORT;
@@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
 static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
 					  const struct cma_req_info *req)
 {
-	struct sockaddr_storage listen_addr_storage, src_addr_storage;
+	struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
 	struct sockaddr *listen_addr = (struct sockaddr *)&listen_addr_storage,
 			*src_addr = (struct sockaddr *)&src_addr_storage;
 	struct net_device *net_dev;
-- 
2.9.0

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

* [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (2 preceding siblings ...)
  2016-10-17 22:05 ` [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning Arnd Bergmann
@ 2016-10-17 22:05 ` Arnd Bergmann
  2016-10-26 14:05   ` [f2fs-dev] " Chao Yu
  2016-10-17 22:05 ` [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning Arnd Bergmann
                   ` (24 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:05 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Fan Li, Weichao Guo,
	linux-f2fs-devel

gcc is unsure about the use of last_ofs_in_node, which might happen
without a prior initialization:

fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {

I'm not sure about it either, so to shut up the warning I initialize
it to a known invalid -1u and later check for this, so we get a
runtime warning if we ever hit the uninitialized case.

It would be much better to reorganize the code in some form that
made it obvious to both the compiler and the reader that this
variable use it ok.

Since I only see the warning with gcc-4.9 but not any later version,
it's possible that the compiler is actually smarter than I am here
and has learned to see the code as correct, in which case this
patch could just be disregarded. It would certainly be helpful
to get an opinion from the maintainers on the matter.

Fixes: 46008c6d4232 ("f2fs: support in batch multi blocks preallocation")
Cc: Chao Yu <yuchao0@huawei.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/f2fs/data.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9ae194f..1b17de2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -696,6 +696,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		goto out;
 	}
 
+	/*
+	 * FIXME: without this, we get "warning: ‘last_ofs_in_node’ may be
+	 * used uninitialized". It's not clear whether that can actually
+	 * happen, so there is now a WARN_ON() checking for this.
+	 */
+	last_ofs_in_node = -1u;
 next_dnode:
 	if (create)
 		f2fs_lock_op(sbi);
@@ -796,6 +802,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		allocated = dn.node_changed;
 
 		map->m_len += dn.ofs_in_node - ofs_in_node;
+		WARN_ON(last_ofs_in_node == -1u);
 		if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
 			err = -ENOSPC;
 			goto sync_out;
-- 
2.9.0

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

* [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (3 preceding siblings ...)
  2016-10-17 22:05 ` [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON Arnd Bergmann
@ 2016-10-17 22:05 ` Arnd Bergmann
  2016-10-18  5:15   ` Christoph Hellwig
  2016-10-17 22:05 ` [PATCH 06/28] NFSv4.1: work around " Arnd Bergmann
                   ` (23 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:05 UTC (permalink / raw)
  To: Jan Kara
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Christoph Hellwig,
	Dave Chinner, Ross Zwisler, Dave Chinner, Al Viro, Andrew Morton,
	Matthew Wilcox, Carlos Maiolino, linux-ext4

On ARM, we get this false-positive warning since the rework of
the ext2_get_blocks interface:

fs/ext2/inode.c: In function 'ext2_get_block':
include/linux/buffer_head.h:340:16: error: 'bno' may be used uninitialized in this function [-Werror=maybe-uninitialized]

The calling conventions for this function are rather complex, and it's
not surprising that the compiler gets this wrong, I spent a long time
trying to understand how it all fits together myself.

This change to avoid the warning makes sure the compiler sees that we
always set 'bno' pointer whenever we have a positive return code.
The transformation is correct because we always arrive at the 'got_it'
label with a positive count that gets used as the return value, while
any branch to the 'cleanup' label has a negative or zero 'err'.

Fixes: 6750ad71986d ("ext2: stop passing buffer_head to ext2_get_blocks")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Dave Chinner <david@fromorbit.com>
---
 fs/ext2/inode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index d831e24..41b8b44 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -622,7 +622,7 @@ static int ext2_get_blocks(struct inode *inode,
 			   u32 *bno, bool *new, bool *boundary,
 			   int create)
 {
-	int err = -EIO;
+	int err;
 	int offsets[4];
 	Indirect chain[4];
 	Indirect *partial;
@@ -639,7 +639,7 @@ static int ext2_get_blocks(struct inode *inode,
 	depth = ext2_block_to_path(inode,iblock,offsets,&blocks_to_boundary);
 
 	if (depth == 0)
-		return (err);
+		return -EIO;
 
 	partial = ext2_get_branch(inode, depth, offsets, chain, &err);
 	/* Simplest case - block found, no allocation needed */
@@ -761,7 +761,6 @@ static int ext2_get_blocks(struct inode *inode,
 	ext2_splice_branch(inode, iblock, partial, indirect_blks, count);
 	mutex_unlock(&ei->truncate_mutex);
 got_it:
-	*bno = le32_to_cpu(chain[depth-1].key);
 	if (count > blocks_to_boundary)
 		*boundary = true;
 	err = count;
@@ -772,6 +771,8 @@ static int ext2_get_blocks(struct inode *inode,
 		brelse(partial->bh);
 		partial--;
 	}
+	if (err > 0)
+		*bno = le32_to_cpu(chain[depth-1].key);
 	return err;
 }
 
-- 
2.9.0

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

* [PATCH 06/28] NFSv4.1: work around -Wmaybe-uninitialized warning
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (4 preceding siblings ...)
  2016-10-17 22:05 ` [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning Arnd Bergmann
@ 2016-10-17 22:05 ` Arnd Bergmann
  2016-10-17 22:08 ` [PATCH 07/28] ceph: avoid false positive maybe-uninitialized warning Arnd Bergmann
                   ` (22 subsequent siblings)
  28 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:05 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, linux-nfs

A bugfix introduced a harmless gcc warning in nfs4_slot_seqid_in_use
if we enable -Wmaybe-uninitialized again:

fs/nfs/nfs4session.c:203:54: error: 'cur_seq' may be used uninitialized in this function [-Werror=maybe-uninitialized]

gcc is not smart enough to conclude that the IS_ERR/PTR_ERR pair
results in a nonzero return value here. Using PTR_ERR_OR_ZERO()
instead makes this clear to the compiler.

The warning originally did not appear in v4.8 as it was globally
disabled, but the bugfix that introduced the warning got backported
to stable kernels which again enable it, and this is now the only
warning in the v4.7 builds.

Fixes: e09c978aae5b ("NFSv4.1: Fix Oopsable condition in server callback races")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
---
First submitted on Aug 31, but ended up not getting applied then
as the warning was disabled in v4.8-rc

 fs/nfs/nfs4session.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4session.c b/fs/nfs/nfs4session.c
index b629730..150c5a1 100644
--- a/fs/nfs/nfs4session.c
+++ b/fs/nfs/nfs4session.c
@@ -178,12 +178,14 @@ static int nfs4_slot_get_seqid(struct nfs4_slot_table  *tbl, u32 slotid,
 	__must_hold(&tbl->slot_tbl_lock)
 {
 	struct nfs4_slot *slot;
+	int ret;
 
 	slot = nfs4_lookup_slot(tbl, slotid);
-	if (IS_ERR(slot))
-		return PTR_ERR(slot);
-	*seq_nr = slot->seq_nr;
-	return 0;
+	ret = PTR_ERR_OR_ZERO(slot);
+	if (!ret)
+		*seq_nr = slot->seq_nr;
+
+	return ret;
 }
 
 /*
-- 
2.9.0

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

* [PATCH 07/28] ceph: avoid false positive maybe-uninitialized warning
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (5 preceding siblings ...)
  2016-10-17 22:05 ` [PATCH 06/28] NFSv4.1: work around " Arnd Bergmann
@ 2016-10-17 22:08 ` Arnd Bergmann
  2016-10-18  2:07   ` Yan, Zheng
  2016-10-17 22:08 ` [PATCH 08/28] staging: lustre: restore initialization of return code Arnd Bergmann
                   ` (21 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:08 UTC (permalink / raw)
  To: Yan, Zheng, Sage Weil, Ilya Dryomov
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Michal Hocko,
	Kirill A. Shutemov, Andrew Morton, Nikolay Borisov, ceph-devel

A recent rework removed the initialization of the local 'root'
variable that is returned from ceph_real_mount:

fs/ceph/super.c: In function 'ceph_mount':
fs/ceph/super.c:1016:38: error: 'root' may be used uninitialized in this function [-Werror=maybe-uninitialized]

It's not obvious to me what the correct fix is, so this just
returns the saved root as we did before.

Fixes: ce2728aaa82b ("ceph: avoid accessing / when mounting a subpath")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: "Yan, Zheng" <zyan@redhat.com>
---
 fs/ceph/super.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index a29ffce..79a4be8 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -821,7 +821,8 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc)
 	dout("mount start %p\n", fsc);
 	mutex_lock(&fsc->client->mount_mutex);
 
-	if (!fsc->sb->s_root) {
+	root = dget(fsc->sb->s_root);
+	if (!root) {
 		const char *path;
 		err = __ceph_open_session(fsc->client, started);
 		if (err < 0)
-- 
2.9.0

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

* [PATCH 08/28] staging: lustre: restore initialization of return code
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (6 preceding siblings ...)
  2016-10-17 22:08 ` [PATCH 07/28] ceph: avoid false positive maybe-uninitialized warning Arnd Bergmann
@ 2016-10-17 22:08 ` Arnd Bergmann
       [not found]   ` <CY4PR11MB1751050A7C7AF840E94DDE60CBD00@CY4PR11MB1751.namprd11.prod.outlook.com>
  2016-10-17 22:42   ` [PATCH 08/28 v2] " Arnd Bergmann
  2016-10-17 22:08 ` [PATCH 09/28] staging: lustre: remove broken dead code in cfs_cpt_table_create_pattern Arnd Bergmann
                   ` (20 subsequent siblings)
  28 siblings, 2 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:08 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, John L . Hammond,
	Jinshan Xiong, James Simmons, Andreas Dilger, Greg Kroah-Hartman,
	lustre-devel, devel

A recent rework removed the initialization of the successful return
code from lpfc_write_firmware:

drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
drivers/scsi/lpfc/lpfc_init.c:10333:214: error: format '%ld' expects argument of type 'long int', but argument 9 has type 'size_t {aka const unsigned int}' [-Werror=format=]

This adds it back.

Fixes: e10a431b3fd0 ("staging: lustre: lov: move LSM to LOV layer")
Cc: John L. Hammond <john.hammond@intel.com>
Cc: Jinshan Xiong <jinshan.xiong@intel.com>
Cc: James Simmons <jsimmons@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/lustre/lustre/lov/lov_pack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
index be6e985..0439f54 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -474,6 +474,8 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm,
 	((struct lov_user_md *)lmmk)->lmm_stripe_count = lum.lmm_stripe_count;
 	if (copy_to_user(lump, lmmk, lmm_size))
 		rc = -EFAULT;
+	else
+		rc = 0;
 
 out_free:
 	kfree(lmmk);
-- 
2.9.0

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

* [PATCH 09/28] staging: lustre: remove broken dead code in cfs_cpt_table_create_pattern
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (7 preceding siblings ...)
  2016-10-17 22:08 ` [PATCH 08/28] staging: lustre: restore initialization of return code Arnd Bergmann
@ 2016-10-17 22:08 ` Arnd Bergmann
  2016-10-17 22:10 ` [PATCH 10/28] UBI: fix uninitialized access of vid_hdr pointer Arnd Bergmann
                   ` (19 subsequent siblings)
  28 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:08 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Liang Zhen,
	James Simmons, Andreas Dilger, Greg Kroah-Hartman, lustre-devel,
	devel

After a recent bugfix, we get a warning about the use of an uninitialized
variable:

drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c: In function 'cfs_cpt_table_create_pattern':
drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c:833:7: error: 'str' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This part of the function used to not do anything as we would reassign
the 'str' pointer to something else right away, but now we pass an
uninitialized pointer into 'strchr', which can cause a kernel page fault
or worse.

Fixes: 239fd5d41f9b ("staging: lustre: libcfs: shortcut to create CPT from NUMA topology")
Cc: Liang Zhen <liang.zhen@intel.com>
Cc: James Simmons <jsimmons@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
index e8b1a61..1226cba 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
@@ -824,13 +824,6 @@ cfs_cpt_table_create_pattern(char *pattern)
 	int			ncpt;
 	int			c;
 
-	for (ncpt = 0;; ncpt++) { /* quick scan bracket */
-		str = strchr(str, '[');
-		if (!str)
-			break;
-		str++;
-	}
-
 	str = cfs_trimwhite(pattern);
 	if (*str == 'n' || *str == 'N') {
 		pattern = str + 1;
-- 
2.9.0

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

* [PATCH 10/28] UBI: fix uninitialized access of vid_hdr pointer
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (8 preceding siblings ...)
  2016-10-17 22:08 ` [PATCH 09/28] staging: lustre: remove broken dead code in cfs_cpt_table_create_pattern Arnd Bergmann
@ 2016-10-17 22:10 ` Arnd Bergmann
  2016-10-18  5:17   ` Boris Brezillon
  2016-10-17 22:10 ` [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized Arnd Bergmann
                   ` (18 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:10 UTC (permalink / raw)
  To: Artem Bityutskiy, Richard Weinberger
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Boris Brezillon,
	David Woodhouse, Brian Norris, linux-mtd

A rework of UBI that just appeared in linux-next during the merge
window introduced caused the recover_peb to use a variable that
is never initialized as seen from this gcc warning:

drivers/mtd/ubi/eba.c: In function ‘recover_peb’:
drivers/mtd/ubi/eba.c:744:40: error: ‘vid_hdr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

It seems clear that the change to the function arguments was missing
the initialization that I'm now adding back to restore the
way the function was working before.

Fixes: 3291b52f9ff0 ("UBI: introduce the VID buffer concept")
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Richard Weinberger <richard@nod.at>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mtd/ubi/eba.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 95c4048..2e152be 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -719,7 +719,7 @@ static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum,
 			   struct ubi_vid_io_buf *vidb, bool *retry)
 {
 	struct ubi_device *ubi = vol->ubi;
-	struct ubi_vid_hdr *vid_hdr;
+	struct ubi_vid_hdr *vid_hdr = ubi_get_vid_hdr(vidb);
 	int new_pnum, err, vol_id = vol->vol_id, data_size;
 	uint32_t crc;
 
-- 
2.9.0

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

* [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (9 preceding siblings ...)
  2016-10-17 22:10 ` [PATCH 10/28] UBI: fix uninitialized access of vid_hdr pointer Arnd Bergmann
@ 2016-10-17 22:10 ` Arnd Bergmann
  2016-10-18  9:57   ` Ilya Dryomov
  2016-10-17 22:12 ` [PATCH 12/28] [media] rc: print correct variable for z8f0811 Arnd Bergmann
                   ` (17 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:10 UTC (permalink / raw)
  To: Ilya Dryomov, Sage Weil
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Alex Elder,
	Mike Christie, ceph-devel

When building with gcc-4.9 -Wmaybe-uninitialized, we get a bogus
warning in rbd_watch_cb, as the variable is not used at all
in the one case in which it is not initialized first:

drivers/block/rbd.c: In function ‘rbd_watch_cb’:
drivers/block/rbd.c:3690:5: error: ‘struct_v’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/block/rbd.c:3759:5: note: ‘struct_v’ was declared here

Later compiler versions fix this, but adding another initialization
here is harmless and lets us build cleanly with 4.9 as well.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/block/rbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index abb7162..4ab990b 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3776,6 +3776,7 @@ static void rbd_watch_cb(void *arg, u64 notify_id, u64 cookie,
 	} else {
 		/* legacy notification for header updates */
 		notify_op = RBD_NOTIFY_OP_HEADER_UPDATE;
+		struct_v = 0;
 		len = 0;
 	}
 
-- 
2.9.0

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

* [PATCH 12/28] [media] rc: print correct variable for z8f0811
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (10 preceding siblings ...)
  2016-10-17 22:10 ` [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized Arnd Bergmann
@ 2016-10-17 22:12 ` Arnd Bergmann
  2016-10-17 22:13 ` [PATCH 13/28] [media] dib0700: fix uninitialized data on 'repeat' event Arnd Bergmann
                   ` (16 subsequent siblings)
  28 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Sean Young, linux-media

A recent rework accidentally left a debugging printk untouched
while changing the meaning of the variables, leading to an
uninitialized variable being printed:

drivers/media/i2c/ir-kbd-i2c.c: In function 'get_key_haup_common':
drivers/media/i2c/ir-kbd-i2c.c:62:2: error: 'toggle' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This prints the correct one instead, as we did before the patch.

Fixes: 00bb820755ed ("[media] rc: Hauppauge z8f0811 can decode RC6")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/i2c/ir-kbd-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index f95a6bc..cede397 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -118,7 +118,7 @@ static int get_key_haup_common(struct IR_i2c *ir, enum rc_type *protocol,
 			*protocol = RC_TYPE_RC6_MCE;
 			dev &= 0x7f;
 			dprintk(1, "ir hauppauge (rc6-mce): t%d vendor=%d dev=%d code=%d\n",
-						toggle, vendor, dev, code);
+						*ptoggle, vendor, dev, code);
 		} else {
 			*ptoggle = 0;
 			*protocol = RC_TYPE_RC6_6A_32;
-- 
2.9.0

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

* [PATCH 13/28] [media] dib0700: fix uninitialized data on 'repeat' event
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (11 preceding siblings ...)
  2016-10-17 22:12 ` [PATCH 12/28] [media] rc: print correct variable for z8f0811 Arnd Bergmann
@ 2016-10-17 22:13 ` Arnd Bergmann
  2016-10-17 22:13 ` [PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable Arnd Bergmann
                   ` (15 subsequent siblings)
  28 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Sean Young,
	Wolfram Sang, Kees Cook, Hans Verkuil, linux-media

After a recent cleanup patch, "gcc -Wmaybe-uninitialized" reports a new
warning about an existing bug:

drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
drivers/media/usb/dvb-usb/dib0700_core.c:763:2: error: ‘protocol’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

It turns out that the "0 0 0 FF" sequence of input data has already
caused an uninitialized data use for the keycode variable, but that
was hidden with the 'uninitialized_var()' macro. Now, the protocol
is also uninitialized.

This changes the code to not report any key for this sequence, which
fixes both problems, and allows us to also remove the misleading
uninitialized_var() annotation.

It is possible that we should call rc_repeat() here, but I'm not
sure about that.

Fixes: 2ceeca0499d7 ("[media] rc: split nec protocol into its three variants")
Fixes: d3c501d1938c ("V4L/DVB: dib0700: Fix RC protocol logic to properly handle NEC/NECx and RC-5")
Cc: Sean Young <sean@mess.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/usb/dvb-usb/dib0700_core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index f319665..3678ebf 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -677,7 +677,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
 	struct dvb_usb_device *d = purb->context;
 	struct dib0700_rc_response *poll_reply;
 	enum rc_type protocol;
-	u32 uninitialized_var(keycode);
+	u32 keycode;
 	u8 toggle;
 
 	deb_info("%s()\n", __func__);
@@ -742,11 +742,10 @@ static void dib0700_rc_urb_completion(struct urb *purb)
 			protocol = RC_TYPE_NEC;
 		}
 
+		rc_keydown(d->rc_dev, protocol, keycode, toggle);
 		break;
 	default:
 		deb_data("RC5 protocol\n");
-		protocol = RC_TYPE_RC5;
-		toggle = poll_reply->report_id;
 		keycode = RC_SCANCODE_RC5(poll_reply->rc5.system, poll_reply->rc5.data);
 
 		if ((poll_reply->rc5.data ^ poll_reply->rc5.not_data) != 0xff) {
@@ -754,14 +753,13 @@ static void dib0700_rc_urb_completion(struct urb *purb)
 			err("key failed integrity check: %02x %02x %02x %02x",
 			    poll_reply->rc5.not_used, poll_reply->rc5.system,
 			    poll_reply->rc5.data, poll_reply->rc5.not_data);
-			goto resubmit;
+			break;
 		}
 
+		rc_keydown(d->rc_dev, RC_TYPE_RC5, keycode, poll_reply->report_id);
 		break;
 	}
 
-	rc_keydown(d->rc_dev, protocol, keycode, toggle);
-
 resubmit:
 	/* Clean the buffer before we requeue */
 	memset(purb->transfer_buffer, 0, RC_MSG_SIZE_V1_20);
-- 
2.9.0

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

* [PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (12 preceding siblings ...)
  2016-10-17 22:13 ` [PATCH 13/28] [media] dib0700: fix uninitialized data on 'repeat' event Arnd Bergmann
@ 2016-10-17 22:13 ` Arnd Bergmann
  2016-10-23 21:25   ` Jonathan Cameron
  2016-10-17 22:13 ` [PATCH 15/28] crypto: aesni: avoid -Wmaybe-uninitialized warning Arnd Bergmann
                   ` (14 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Ico Doornekamp,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Greg Kroah-Hartman, Ioana Ciornei,
	Luis de Bethencourt, linux-iio, devel

The newly added __sca3000_get_base_freq function handles all valid
modes of the SCA3000_REG_ADDR_MODE register, but gcc notices
that any other value (i.e. 0x00) causes the base_freq variable to
not get initialized:

drivers/staging/iio/accel/sca3000_core.c: In function 'sca3000_write_raw':
drivers/staging/iio/accel/sca3000_core.c:527:23: error: 'base_freq' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds explicit error handling for unexpected register values,
to ensure this cannot happen.

Fixes: e0f3fc9b47e6 ("iio: accel: sca3000_core: implemented IIO_CHAN_INFO_SAMP_FREQ")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Ico Doornekamp <ico@pruts.nl>
Cc: Jonathan Cameron <jic23@kernel.org>
---
I submitted this on Sept 22, and Jonathan said he applied it to his
'togreg' tree, but it hasn't appeared in linux-next yet, presumably
since this was not considered material for v4.9.

If we enable the warning again by default, we may want to have the
fix merged for v4.9 after all.

 drivers/staging/iio/accel/sca3000_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
index d626125..564b36d 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -468,6 +468,8 @@ static inline int __sca3000_get_base_freq(struct sca3000_state *st,
 	case SCA3000_MEAS_MODE_OP_2:
 		*base_freq = info->option_mode_2_freq;
 		break;
+	default:
+		ret = -EINVAL;
 	}
 error_ret:
 	return ret;
-- 
2.9.0

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

* [PATCH 15/28] crypto: aesni: avoid -Wmaybe-uninitialized warning
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (13 preceding siblings ...)
  2016-10-17 22:13 ` [PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable Arnd Bergmann
@ 2016-10-17 22:13 ` Arnd Bergmann
  2016-10-17 22:13 ` [PATCH 16/28] pcmcia: fix return value of soc_pcmcia_regulator_set Arnd Bergmann
                   ` (13 subsequent siblings)
  28 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
  To: Herbert Xu, x86
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, David S. Miller,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Borislav Petkov,
	Stephan Mueller, linux-crypto

The rfc4106 encrypy/decrypt helper functions cause an annoying
false-positive warning in allmodconfig if we turn on
-Wmaybe-uninitialized warnings again:

arch/x86/crypto/aesni-intel_glue.c: In function ‘helper_rfc4106_decrypt’:
include/linux/scatterlist.h:67:31: warning: ‘dst_sg_walk.sg’ may be used uninitialized in this function [-Wmaybe-uninitialized]

The problem seems to be that the compiler doesn't track the state of the
'one_entry_in_sg' variable across the kernel_fpu_begin/kernel_fpu_end
section.

This reorganizes the code to avoid that variable and have the shared
code in a separate function to avoid some of the conditional branches.

The resulting functions are a bit longer but also slightly less complex,
leaving no room for speculation on the part of the compiler.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
The conversion is nontrivial, and I have only build-tested it, so this
could use a careful review and testing.
---
 arch/x86/crypto/aesni-intel_glue.c | 121 ++++++++++++++++++++++---------------
 1 file changed, 73 insertions(+), 48 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 0ab5ee1..054155b 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -269,6 +269,34 @@ static void (*aesni_gcm_dec_tfm)(void *ctx, u8 *out,
 			u8 *hash_subkey, const u8 *aad, unsigned long aad_len,
 			u8 *auth_tag, unsigned long auth_tag_len);
 
+static inline void aesni_do_gcm_enc_tfm(void *ctx, u8 *out,
+			const u8 *in, unsigned long plaintext_len, u8 *iv,
+			u8 *hash_subkey, const u8 *aad, unsigned long aad_len,
+			u8 *auth_tag, unsigned long auth_tag_len)
+{
+	kernel_fpu_begin();
+	aesni_gcm_enc_tfm(ctx, out, in, plaintext_len, iv, hash_subkey,
+			  aad, aad_len, auth_tag, auth_tag_len);
+	kernel_fpu_end();
+}
+
+static inline int aesni_do_gcm_dec_tfm(void *ctx, u8 *out,
+			const u8 *in, unsigned long ciphertext_len, u8 *iv,
+			u8 *hash_subkey, const u8 *aad, unsigned long aad_len,
+			u8 *auth_tag, unsigned long auth_tag_len)
+{
+	kernel_fpu_begin();
+	aesni_gcm_dec_tfm(ctx, out, in, ciphertext_len, iv, hash_subkey, aad,
+			  aad_len, auth_tag, auth_tag_len);
+	kernel_fpu_end();
+
+	/* Compare generated tag with passed in tag. */
+	if (crypto_memneq(in + ciphertext_len, auth_tag, auth_tag_len))
+		return -EBADMSG;
+
+	return 0;
+}
+
 static inline struct
 aesni_rfc4106_gcm_ctx *aesni_rfc4106_gcm_ctx_get(struct crypto_aead *tfm)
 {
@@ -879,7 +907,6 @@ static int rfc4106_set_authsize(struct crypto_aead *parent,
 
 static int helper_rfc4106_encrypt(struct aead_request *req)
 {
-	u8 one_entry_in_sg = 0;
 	u8 *src, *dst, *assoc;
 	__be32 counter = cpu_to_be32(1);
 	struct crypto_aead *tfm = crypto_aead_reqtfm(req);
@@ -908,7 +935,6 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 	    req->src->offset + req->src->length <= PAGE_SIZE &&
 	    sg_is_last(req->dst) &&
 	    req->dst->offset + req->dst->length <= PAGE_SIZE) {
-		one_entry_in_sg = 1;
 		scatterwalk_start(&src_sg_walk, req->src);
 		assoc = scatterwalk_map(&src_sg_walk);
 		src = assoc + req->assoclen;
@@ -916,7 +942,23 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 		if (unlikely(req->src != req->dst)) {
 			scatterwalk_start(&dst_sg_walk, req->dst);
 			dst = scatterwalk_map(&dst_sg_walk) + req->assoclen;
+
+			aesni_do_gcm_enc_tfm(aes_ctx, dst, src, req->cryptlen, iv,
+					     ctx->hash_subkey, assoc, req->assoclen - 8,
+					     dst + req->cryptlen, auth_tag_len);
+
+			scatterwalk_unmap(dst - req->assoclen);
+			scatterwalk_advance(&dst_sg_walk, req->dst->length);
+			scatterwalk_done(&dst_sg_walk, 1, 0);
+		} else {
+			aesni_do_gcm_enc_tfm(aes_ctx, dst, src, req->cryptlen, iv,
+					     ctx->hash_subkey, assoc, req->assoclen - 8,
+					     dst + req->cryptlen, auth_tag_len);
 		}
+
+		scatterwalk_unmap(assoc);
+		scatterwalk_advance(&src_sg_walk, req->src->length);
+		scatterwalk_done(&src_sg_walk, req->src == req->dst, 0);
 	} else {
 		/* Allocate memory for src, dst, assoc */
 		assoc = kmalloc(req->cryptlen + auth_tag_len + req->assoclen,
@@ -925,28 +967,14 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 			return -ENOMEM;
 		scatterwalk_map_and_copy(assoc, req->src, 0,
 					 req->assoclen + req->cryptlen, 0);
-		src = assoc + req->assoclen;
-		dst = src;
-	}
+		dst = src = assoc + req->assoclen;
 
-	kernel_fpu_begin();
-	aesni_gcm_enc_tfm(aes_ctx, dst, src, req->cryptlen, iv,
-			  ctx->hash_subkey, assoc, req->assoclen - 8,
-			  dst + req->cryptlen, auth_tag_len);
-	kernel_fpu_end();
+		aesni_gcm_enc_tfm(aes_ctx, dst, src, req->cryptlen, iv,
+				  ctx->hash_subkey, assoc, req->assoclen - 8,
+				  dst + req->cryptlen, auth_tag_len);
 
-	/* The authTag (aka the Integrity Check Value) needs to be written
-	 * back to the packet. */
-	if (one_entry_in_sg) {
-		if (unlikely(req->src != req->dst)) {
-			scatterwalk_unmap(dst - req->assoclen);
-			scatterwalk_advance(&dst_sg_walk, req->dst->length);
-			scatterwalk_done(&dst_sg_walk, 1, 0);
-		}
-		scatterwalk_unmap(assoc);
-		scatterwalk_advance(&src_sg_walk, req->src->length);
-		scatterwalk_done(&src_sg_walk, req->src == req->dst, 0);
-	} else {
+		/* The authTag (aka the Integrity Check Value) needs to be written
+		 * back to the packet. */
 		scatterwalk_map_and_copy(dst, req->dst, req->assoclen,
 					 req->cryptlen + auth_tag_len, 1);
 		kfree(assoc);
@@ -956,7 +984,6 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 
 static int helper_rfc4106_decrypt(struct aead_request *req)
 {
-	u8 one_entry_in_sg = 0;
 	u8 *src, *dst, *assoc;
 	unsigned long tempCipherLen = 0;
 	__be32 counter = cpu_to_be32(1);
@@ -990,47 +1017,45 @@ static int helper_rfc4106_decrypt(struct aead_request *req)
 	    req->src->offset + req->src->length <= PAGE_SIZE &&
 	    sg_is_last(req->dst) &&
 	    req->dst->offset + req->dst->length <= PAGE_SIZE) {
-		one_entry_in_sg = 1;
 		scatterwalk_start(&src_sg_walk, req->src);
 		assoc = scatterwalk_map(&src_sg_walk);
 		src = assoc + req->assoclen;
-		dst = src;
 		if (unlikely(req->src != req->dst)) {
 			scatterwalk_start(&dst_sg_walk, req->dst);
 			dst = scatterwalk_map(&dst_sg_walk) + req->assoclen;
-		}
-
-	} else {
-		/* Allocate memory for src, dst, assoc */
-		assoc = kmalloc(req->cryptlen + req->assoclen, GFP_ATOMIC);
-		if (!assoc)
-			return -ENOMEM;
-		scatterwalk_map_and_copy(assoc, req->src, 0,
-					 req->assoclen + req->cryptlen, 0);
-		src = assoc + req->assoclen;
-		dst = src;
-	}
 
-	kernel_fpu_begin();
-	aesni_gcm_dec_tfm(aes_ctx, dst, src, tempCipherLen, iv,
-			  ctx->hash_subkey, assoc, req->assoclen - 8,
-			  authTag, auth_tag_len);
-	kernel_fpu_end();
-
-	/* Compare generated tag with passed in tag. */
-	retval = crypto_memneq(src + tempCipherLen, authTag, auth_tag_len) ?
-		-EBADMSG : 0;
+			retval = aesni_do_gcm_dec_tfm(aes_ctx, dst, src,
+					tempCipherLen, iv, ctx->hash_subkey,
+					assoc, req->assoclen - 8, authTag,
+					auth_tag_len);
 
-	if (one_entry_in_sg) {
-		if (unlikely(req->src != req->dst)) {
 			scatterwalk_unmap(dst - req->assoclen);
 			scatterwalk_advance(&dst_sg_walk, req->dst->length);
 			scatterwalk_done(&dst_sg_walk, 1, 0);
+		} else {
+			dst = src;
+			retval = aesni_do_gcm_dec_tfm(aes_ctx, dst, src,
+					tempCipherLen, iv, ctx->hash_subkey,
+					assoc, req->assoclen - 8, authTag,
+					auth_tag_len);
 		}
 		scatterwalk_unmap(assoc);
 		scatterwalk_advance(&src_sg_walk, req->src->length);
 		scatterwalk_done(&src_sg_walk, req->src == req->dst, 0);
 	} else {
+		/* Allocate memory for src, dst, assoc */
+		assoc = kmalloc(req->cryptlen + req->assoclen, GFP_ATOMIC);
+		if (!assoc)
+			return -ENOMEM;
+		scatterwalk_map_and_copy(assoc, req->src, 0,
+					 req->assoclen + req->cryptlen, 0);
+		dst = src = assoc + req->assoclen;
+
+		retval = aesni_do_gcm_dec_tfm(aes_ctx, dst, src, tempCipherLen,
+					      iv, ctx->hash_subkey, assoc,
+					      req->assoclen - 8, authTag,
+					      auth_tag_len);
+
 		scatterwalk_map_and_copy(dst, req->dst, req->assoclen,
 					 tempCipherLen, 1);
 		kfree(assoc);
-- 
2.9.0

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

* [PATCH 16/28] pcmcia: fix return value of soc_pcmcia_regulator_set
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (14 preceding siblings ...)
  2016-10-17 22:13 ` [PATCH 15/28] crypto: aesni: avoid -Wmaybe-uninitialized warning Arnd Bergmann
@ 2016-10-17 22:13 ` Arnd Bergmann
  2016-10-18  9:42   ` Russell King - ARM Linux
  2016-10-17 22:13 ` [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error Arnd Bergmann
                   ` (12 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
  To: Russell King; +Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, linux-pcmcia

The newly introduced soc_pcmcia_regulator_set() function sometimes returns
without setting its return code, as shown by this warning:

drivers/pcmcia/soc_common.c: In function 'soc_pcmcia_regulator_set':
drivers/pcmcia/soc_common.c:112:5: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This changes it to propagate the regulator_disable() result instead.

Fixes: ac61b6001a63 ("pcmcia: soc_common: add support for Vcc and Vpp regulators")
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/pcmcia/soc_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index 153f312..b6b316d 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -107,7 +107,7 @@ int soc_pcmcia_regulator_set(struct soc_pcmcia_socket *skt,
 
 		ret = regulator_enable(r->reg);
 	} else {
-		regulator_disable(r->reg);
+		ret = regulator_disable(r->reg);
 	}
 	if (ret == 0)
 		r->on = on;
-- 
2.9.0

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

* [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (15 preceding siblings ...)
  2016-10-17 22:13 ` [PATCH 16/28] pcmcia: fix return value of soc_pcmcia_regulator_set Arnd Bergmann
@ 2016-10-17 22:13 ` Arnd Bergmann
  2016-10-24 17:27   ` Mark Brown
  2016-10-26 10:15   ` Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree Mark Brown
  2016-10-17 22:13 ` [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank Arnd Bergmann
                   ` (11 subsequent siblings)
  28 siblings, 2 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Heiner Kallweit,
	Nobuteru Hayashi, linux-spi

When we get a spurious interrupt in fsl_espi_irq, we end up
processing four uninitalized bytes of data, as shown in this
warning message:

   drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
   drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds another check so we skip the data in this case.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/spi/spi-fsl-espi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7451585..2c175b9 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
 
 		mspi->len -= rx_nr_bytes;
 
-		if (mspi->rx)
+		if (rx_nr_bytes && mspi->rx)
 			mspi->get_rx(rx_data, mspi);
 	}
 
-- 
2.9.0

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

* [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (16 preceding siblings ...)
  2016-10-17 22:13 ` [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error Arnd Bergmann
@ 2016-10-17 22:13 ` Arnd Bergmann
  2016-10-17 23:47   ` Mario Kleiner
  2016-10-17 22:13 ` [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap Arnd Bergmann
                   ` (10 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
  To: David Airlie
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Rob Clark,
	Daniel Vetter, Mario Kleiner, Dave Airlie,
	Ville Syrjälä,
	Alex Deucher, Gustavo Padovan, Matthew Auld, dri-devel

gcc warns about the timestamp in drm_wait_vblank being possibly
used without an initialization:

drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This can happen if drm_vblank_count_and_time() returns 0 in its
error path. To sanitize the error case, I'm changing that function
to return a zero timestamp when it fails.

Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
First submitted in January 2016, second submission in February,
the patch is still required.

 drivers/gpu/drm/drm_irq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b969a64..48a6167 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
 	u32 vblank_count;
 	unsigned int seq;
 
-	if (WARN_ON(pipe >= dev->num_crtcs))
+	if (WARN_ON(pipe >= dev->num_crtcs)) {
+		*vblanktime = (struct timeval) { 0 };
 		return 0;
+	}
 
 	do {
 		seq = read_seqbegin(&vblank->seqlock);
-- 
2.9.0

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

* [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (17 preceding siblings ...)
  2016-10-17 22:13 ` [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank Arnd Bergmann
@ 2016-10-17 22:13 ` Arnd Bergmann
  2016-10-26  6:49   ` Kalle Valo
  2016-10-27 15:05   ` [19/28] " Kalle Valo
  2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
                   ` (9 subsequent siblings)
  28 siblings, 2 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Hante Meuleman,
	Kalle Valo, Franky Lin, Pieter-Paul Giesberts,
	Franky (Zhenhui) Lin, Rafał Miłecki, linux-wireless,
	brcm80211-dev-list.pdl, netdev

A bugfix added a sanity check around the assignment and use of the
'is_11d' variable, which looks correct to me, but as the function is
rather complex already, this confuses the compiler to the point where
it can no longer figure out if the variable is always initialized
correctly:

brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds an initialization for the newly introduced case in which
the variable should not really be used, in order to make the warning
go away.

Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
Cc: Hante Meuleman <hante.meuleman@broadcom.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b777e1b..78d9966 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4516,7 +4516,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 	/* store current 11d setting */
 	if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
 				  &ifp->vif->is_11d)) {
-		supports_11d = false;
+		is_11d = supports_11d = false;
 	} else {
 		country_ie = brcmf_parse_tlvs((u8 *)settings->beacon.tail,
 					      settings->beacon.tail_len,
-- 
2.9.0

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

* [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (18 preceding siblings ...)
  2016-10-17 22:13 ` [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
  2016-10-18 18:21   ` David Miller
  2016-10-17 22:16 ` [PATCH 21/28] net/hyperv: avoid " Arnd Bergmann
                   ` (8 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
  To: David S. Miller
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Philippe Reynes,
	Florian Fainelli, bcm-kernel-feedback-list, Andrew Lunn, netdev,
	linux-arm-kernel

gcc found a reference to an uninitialized variable in the error handling
of bcm_enet_open, introduced by a recent cleanup:

drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]

This makes the use of that variable conditional, so we only reference it
here after it has been used before. Unlike my normal patches, I have not
build-tested this one, as I don't currently have mips test in my
randconfig setup.

Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
Cc: Philippe Reynes <tremyfr@gmail.com>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/bcm63xx_enet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index ae364c7..5370909 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1126,7 +1126,8 @@ static int bcm_enet_open(struct net_device *dev)
 	free_irq(dev->irq, dev);
 
 out_phy_disconnect:
-	phy_disconnect(phydev);
+	if (priv->has_phy)
+		phy_disconnect(phydev);
 
 	return ret;
 }
-- 
2.9.0

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

* [PATCH 21/28] net/hyperv: avoid uninitialized variable
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (19 preceding siblings ...)
  2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
  2016-10-18 18:21   ` David Miller
  2016-10-17 22:16 ` [PATCH 22/28] x86: apm: avoid uninitialized data Arnd Bergmann
                   ` (7 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, David S. Miller,
	Vitaly Kuznetsov, stephen hemminger, sixiao, devel, netdev

The hdr_offset variable is only if we deal with a TCP or UDP packet,
but as the check surrounding its usage tests for skb_is_gso()
instead, the compiler has no idea if the variable is initialized
or not at that point:

drivers/net/hyperv/netvsc_drv.c: In function ‘netvsc_start_xmit’:
drivers/net/hyperv/netvsc_drv.c:494:42: error: ‘hdr_offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds an additional check for the transport type, which
tells the compiler that this path cannot happen. Since the
get_net_transport_info() function should always be inlined
here, I don't expect this to result in additional runtime
checks.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/hyperv/netvsc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f0919bd..5d6e75a 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -447,7 +447,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	 * Setup the sendside checksum offload only if this is not a
 	 * GSO packet.
 	 */
-	if (skb_is_gso(skb)) {
+	if ((net_trans_info & (INFO_TCP | INFO_UDP)) && skb_is_gso(skb)) {
 		struct ndis_tcp_lso_info *lso_info;
 
 		rndis_msg_size += NDIS_LSO_PPI_SIZE;
-- 
2.9.0

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

* [PATCH 22/28] x86: apm: avoid uninitialized data
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (20 preceding siblings ...)
  2016-10-17 22:16 ` [PATCH 21/28] net/hyperv: avoid " Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
  2016-10-18 13:05   ` Jiri Kosina
  2016-10-18 21:35   ` Luis R. Rodriguez
  2016-10-17 22:16 ` [PATCH 23/28] x86: mark target address as output in 'insb' asm Arnd Bergmann
                   ` (6 subsequent siblings)
  28 siblings, 2 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, x86,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Luis R. Rodriguez

apm_bios_call() can fail, and return a status in its argument
structure. If that status however is zero during a call from
apm_get_power_status(), we end up using data that may have
never been set, as reported by "gcc -Wmaybe-uninitialized":

arch/x86/kernel/apm_32.c: In function ‘apm’:
arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here

This changes the function to return "APM_NO_ERROR" here, which
makes the code more robust to broken BIOS versions, and avoids
the warning.

Cc: x86@kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/kernel/apm_32.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index c7364bd..51287cd 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, u_short *bat, u_short *life)
 
 	if (apm_info.get_power_status_broken)
 		return APM_32_UNSUPPORTED;
-	if (apm_bios_call(&call))
+	if (apm_bios_call(&call)) {
+		if (!call.err)
+			return APM_NO_ERROR;
 		return call.err;
+	}
 	*status = call.ebx;
 	*bat = call.ecx;
 	if (apm_info.get_power_status_swabinminutes) {
-- 
2.9.0

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

* [PATCH 23/28] x86: mark target address as output in 'insb' asm
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (21 preceding siblings ...)
  2016-10-17 22:16 ` [PATCH 22/28] x86: apm: avoid uninitialized data Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
  2016-10-17 22:16 ` [PATCH 24/28] x86: math-emu: possible uninitialized variable use Arnd Bergmann
                   ` (5 subsequent siblings)
  28 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
  To: x86
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin

The -Wmaybe-uninitialized warning triggers for one driver using the output
of the 'insb' I/O helper on x86:

drivers/net/wireless/wl3501_cs.c: In function ‘wl3501_mgmt_scan_confirm’:
drivers/net/wireless/wl3501_cs.c:665:9: error: ‘sig.status’ is used uninitialized in this function [-Werror=uninitialized]
drivers/net/wireless/wl3501_cs.c:668:12: error: ‘sig.cap_info’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

Apparently the assember constraints are slightly off here, as marking the
'addr' argument as a memory output seems appropriate here and gets rid
of the warning. For consistency I'm also adding it as input for outsb().

I'm not an x86 person and gcc inline assembly mystifies me all the time,
so please review this carefully and suggest a better way if this is not
how it should be done.

Cc: x86@kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/include/asm/io.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index de25aad..287234c 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -304,13 +304,13 @@ static inline unsigned type in##bwl##_p(int port)			\
 static inline void outs##bwl(int port, const void *addr, unsigned long count) \
 {									\
 	asm volatile("rep; outs" #bwl					\
-		     : "+S"(addr), "+c"(count) : "d"(port));		\
+		     : "+S"(addr), "+c"(count) : "d"(port), "m" (addr));\
 }									\
 									\
 static inline void ins##bwl(int port, void *addr, unsigned long count)	\
 {									\
 	asm volatile("rep; ins" #bwl					\
-		     : "+D"(addr), "+c"(count) : "d"(port));		\
+		     : "+D"(addr), "+c"(count), "=m" (addr) : "d"(port));\
 }
 
 BUILDIO(b, b, char)
-- 
2.9.0

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

* [PATCH 24/28] x86: math-emu: possible uninitialized variable use
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (22 preceding siblings ...)
  2016-10-17 22:16 ` [PATCH 23/28] x86: mark target address as output in 'insb' asm Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
  2016-10-17 22:16 ` [PATCH 25/28] s390: pci: don't print uninitialized data for debugging Arnd Bergmann
                   ` (4 subsequent siblings)
  28 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
  To: x86
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Bill Metzenthen,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin

When building the kernel with "make EXTRA_CFLAGS=...", this overrides
the "PARANOID" preprocessor macro defined in arch/x86/math-emu/Makefile,
and we run into a build warning:

arch/x86/math-emu/reg_compare.c: In function ‘compare_i_st_st’:
arch/x86/math-emu/reg_compare.c:254:6: error: ‘f’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This fixes the implementation to work correctly even without the PARANOID
flag, and also fixes the Makefile to not use the EXTRA_CFLAGS variable
but instead use the ccflags-y variable in the Makefile that is meant
for this purpose.

Cc: x86@kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/math-emu/Makefile      |  4 ++--
 arch/x86/math-emu/reg_compare.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/math-emu/Makefile b/arch/x86/math-emu/Makefile
index 9b0c63b..1b2dac1 100644
--- a/arch/x86/math-emu/Makefile
+++ b/arch/x86/math-emu/Makefile
@@ -5,8 +5,8 @@
 #DEBUG	= -DDEBUGGING
 DEBUG	=
 PARANOID = -DPARANOID
-EXTRA_CFLAGS	:= $(PARANOID) $(DEBUG) -fno-builtin $(MATH_EMULATION)
-EXTRA_AFLAGS	:= $(PARANOID)
+ccflags-y += $(PARANOID) $(DEBUG) -fno-builtin $(MATH_EMULATION)
+asflags-y += $(PARANOID)
 
 # From 'C' language sources:
 C_OBJS =fpu_entry.o errors.o \
diff --git a/arch/x86/math-emu/reg_compare.c b/arch/x86/math-emu/reg_compare.c
index b77360f..19b33b50 100644
--- a/arch/x86/math-emu/reg_compare.c
+++ b/arch/x86/math-emu/reg_compare.c
@@ -168,7 +168,7 @@ static int compare(FPU_REG const *b, int tagb)
 /* This function requires that st(0) is not empty */
 int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 {
-	int f = 0, c;
+	int f, c;
 
 	c = compare(loaded_data, loaded_tag);
 
@@ -189,12 +189,12 @@ int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 		case COMP_No_Comp:
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#ifdef PARANOID
 		default:
+#ifdef PARANOID
 			EXCEPTION(EX_INTERNAL | 0x121);
+#endif /* PARANOID */
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#endif /* PARANOID */
 		}
 	setcc(f);
 	if (c & COMP_Denormal) {
@@ -205,7 +205,7 @@ int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 
 static int compare_st_st(int nr)
 {
-	int f = 0, c;
+	int f, c;
 	FPU_REG *st_ptr;
 
 	if (!NOT_EMPTY(0) || !NOT_EMPTY(nr)) {
@@ -235,12 +235,12 @@ static int compare_st_st(int nr)
 		case COMP_No_Comp:
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#ifdef PARANOID
 		default:
+#ifdef PARANOID
 			EXCEPTION(EX_INTERNAL | 0x122);
+#endif /* PARANOID */
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#endif /* PARANOID */
 		}
 	setcc(f);
 	if (c & COMP_Denormal) {
@@ -283,12 +283,12 @@ static int compare_i_st_st(int nr)
 	case COMP_No_Comp:
 		f = X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF;
 		break;
-#ifdef PARANOID
 	default:
+#ifdef PARANOID
 		EXCEPTION(EX_INTERNAL | 0x122);
+#endif /* PARANOID */
 		f = 0;
 		break;
-#endif /* PARANOID */
 	}
 	FPU_EFLAGS = (FPU_EFLAGS & ~(X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF)) | f;
 	if (c & COMP_Denormal) {
-- 
2.9.0

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

* [PATCH 25/28] s390: pci: don't print uninitialized data for debugging
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (23 preceding siblings ...)
  2016-10-17 22:16 ` [PATCH 24/28] x86: math-emu: possible uninitialized variable use Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
  2016-10-18  6:48   ` Martin Schwidefsky
  2016-10-17 22:16 ` [PATCH 26/28] nios2: fix timer initcall return value Arnd Bergmann
                   ` (3 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Sebastian Ott,
	Gerald Schaefer, Joerg Roedel, Christian Borntraeger, linux-s390

gcc correctly warns about an incorrect use of the 'pa' variable
in case we pass an empty scatterlist to __s390_dma_map_sg:

arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg':
arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds a bogus initialization to the function to sanitize
the debug output.  I would have preferred a solution without
the initialization, but I only got the report from the
kbuild bot after turning on the warning again, and didn't
manage to reproduce it myself.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/pci/pci_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index 7350c8b..6b2f72f 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -423,7 +423,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	dma_addr_t dma_addr_base, dma_addr;
 	int flags = ZPCI_PTE_VALID;
 	struct scatterlist *s;
-	unsigned long pa;
+	unsigned long pa = 0;
 	int ret;
 
 	size = PAGE_ALIGN(size);
-- 
2.9.0

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

* [PATCH 26/28] nios2: fix timer initcall return value
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (24 preceding siblings ...)
  2016-10-17 22:16 ` [PATCH 25/28] s390: pci: don't print uninitialized data for debugging Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
  2016-10-24  0:54   ` Ley Foon Tan
  2016-10-17 22:16 ` [PATCH 27/28] rocker: fix maybe-uninitialized warning Arnd Bergmann
                   ` (2 subsequent siblings)
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
  To: Ley Foon Tan
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Daniel Lezcano, nios2-dev

When called more than twice, the nios2_time_init() function
return an uninitialized value, as detected by gcc -Wmaybe-uninitialized

arch/nios2/kernel/time.c: warning: 'ret' may be used uninitialized in this function

This makes it return '0' here, matching the comment above the
function.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/nios2/kernel/time.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/nios2/kernel/time.c b/arch/nios2/kernel/time.c
index d9563dd..746bf5c 100644
--- a/arch/nios2/kernel/time.c
+++ b/arch/nios2/kernel/time.c
@@ -324,6 +324,7 @@ static int __init nios2_time_init(struct device_node *timer)
 		ret = nios2_clocksource_init(timer);
 		break;
 	default:
+		ret = 0;
 		break;
 	}
 
-- 
2.9.0

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

* [PATCH 27/28] rocker: fix maybe-uninitialized warning
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (25 preceding siblings ...)
  2016-10-17 22:16 ` [PATCH 26/28] nios2: fix timer initcall return value Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
  2016-10-18 18:21   ` David Miller
  2016-10-17 22:19 ` [PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning Arnd Bergmann
  2016-10-18  5:08 ` [PATCH 00/28] Reenable maybe-uninitialized warnings Christoph Hellwig
  28 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, David S. Miller,
	Ido Schimmel, Dan Carpenter, netdev

In some rare configurations, we get a warning about the 'index' variable
being used without an initialization:

drivers/net/ethernet/rocker/rocker_ofdpa.c: In function ‘ofdpa_port_fib_ipv4.isra.16.constprop’:
drivers/net/ethernet/rocker/rocker_ofdpa.c:2425:92: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized]

This is a false positive, the logic is just a bit too complex for gcc
to follow here. Moving the intialization of 'index' a little further
down makes it clear to gcc that the function always returns an error
if it is not initialized.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/rocker/rocker_ofdpa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index 431a608..4ca4613 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -1493,8 +1493,6 @@ static int ofdpa_port_ipv4_nh(struct ofdpa_port *ofdpa_port,
 	spin_lock_irqsave(&ofdpa->neigh_tbl_lock, lock_flags);
 
 	found = ofdpa_neigh_tbl_find(ofdpa, ip_addr);
-	if (found)
-		*index = found->index;
 
 	updating = found && adding;
 	removing = found && !adding;
@@ -1508,9 +1506,11 @@ static int ofdpa_port_ipv4_nh(struct ofdpa_port *ofdpa_port,
 		resolved = false;
 	} else if (removing) {
 		ofdpa_neigh_del(trans, found);
+		*index = found->index;
 	} else if (updating) {
 		ofdpa_neigh_update(found, trans, NULL, false);
 		resolved = !is_zero_ether_addr(found->eth_dst);
+		*index = found->index;
 	} else {
 		err = -ENOENT;
 	}
-- 
2.9.0

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

* [PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (26 preceding siblings ...)
  2016-10-17 22:16 ` [PATCH 27/28] rocker: fix maybe-uninitialized warning Arnd Bergmann
@ 2016-10-17 22:19 ` Arnd Bergmann
  2016-10-18  5:08 ` [PATCH 00/28] Reenable maybe-uninitialized warnings Christoph Hellwig
  28 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:19 UTC (permalink / raw)
  To: Linus Torvalds, Michal Marek
  Cc: linux-kernel, Arnd Bergmann, x86, linux-media,
	Mauro Carvalho Chehab, Martin Schwidefsky, linux-s390,
	Ilya Dryomov, dri-devel, linux-mtd, Herbert Xu, linux-crypto,
	David S. Miller, netdev, Greg Kroah-Hartman, ceph-devel,
	linux-f2fs-devel, linux-ext4, netfilter-devel, Vineet Gupta,
	Kees Cook, Ingo Molnar, Josh Poimboeuf, Nicolas Pitre,
	Andrew Morton, Heiko Carstens, Christian Borntraeger,
	linux-kbuild, linux-snps-arc

Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].

Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE.  This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.

With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.

However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that
I had not addressed until then. This caused a lot of actual bugs to
get merged into mainline, and I sent several dozen patches for these
during the v4.9 development cycle. Most of these are actual bugs,
some are for correct code that is safe because it is only called
under external constraints that make it impossible to run into
the case that gcc sees, and in a few cases gcc is just stupid and
finds something that can obviously never happen.

I have now done a few thousand randconfig builds on x86 and collected
all patches that I needed to address every single warning I got
(I can provide the combined patch for the other warnings if anyone
is interested), so I hope we can get the warning back and let people
catch the actual bugs earlier.

Note that the majority of the patches I created are for the third kind
of problem (stupid false-positives), for one of two reasons:
- some of them only get triggered in certain combinations of config
  options, so we don't always run into them, and
- the actual bugs tend to get addressed much quicker as they also
  lead to incorrect runtime behavior.

These 27 patches address the warnings that either occur in one of the more
common configurations (defconfig, allmodconfig, or something built by the
kbuild robot or kernelci.org), or they are about a real bug. It would be
good to get these all into v4.9 if we want to turn on the warning again.
I have tested these extensively with gcc-4.9 and gcc-6 and done a bit
of testing with gcc-5, and all of these should now be fine. gcc-4.8
is much worse about the false-positive warnings and is also fairly old
now, so I'm leaving the warning disabled with that version. gcc-4.7 and
older don't understand the -Wno-maybe-uninitialized option and are not
affected by this patch either way.

I have another (smaller) series of patches for warnings that are both
harmless and not as easy to trigger, and I will send them for inclusion
in v4.10.

Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 Makefile               | 10 ++++++----
 arch/arc/Makefile      |  4 +++-
 scripts/Makefile.ubsan |  4 ++++
 3 files changed, 13 insertions(+), 5 deletions(-)

Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org

diff --git a/Makefile b/Makefile
index 512e47a..43cd3d9 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
 LDFLAGS_vmlinux =
-CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im  -Wno-maybe-uninitialized
 CFLAGS_KCOV	:= $(call cc-option,-fsanitize-coverage=trace-pc,)
 
 
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
 include arch/$(SRCARCH)/Makefile
 
 KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS	+= $(call cc-disable-warning,maybe-uninitialized,)
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 
 ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS	+= $(call cc-option,-fdata-sections,)
 endif
 
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS	+= -Os
+KBUILD_CFLAGS	+= -Os $(call cc-disable-warning,maybe-uninitialized,)
 else
 ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS	+= -O2
+KBUILD_CFLAGS	+= -O2 $(call cc-disable-warning,maybe-uninitialized,)
 else
 KBUILD_CFLAGS   += -O2
 endif
 endif
 
+KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
+			$(call cc-disable-warning,maybe-uninitialized,))
+
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index aa82d13..19cce22 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -71,7 +71,9 @@ cflags-$(CONFIG_ARC_DW2_UNWIND)		+= -fasynchronous-unwind-tables $(cfi)
 ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
 # Generic build system uses -O2, we want -O3
 # Note: No need to add to cflags-y as that happens anyways
-ARCH_CFLAGS += -O3
+#
+# Disable the false maybe-uninitialized warings gcc spits out at -O3
+ARCH_CFLAGS += -O3 $(call cc-disable-warning,maybe-uninitialized,)
 endif
 
 # small data is default for elf32 tool-chain. If not usable, disable it
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index dd779c4..3b1b138 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -17,4 +17,8 @@ endif
 ifdef CONFIG_UBSAN_NULL
       CFLAGS_UBSAN += $(call cc-option, -fsanitize=null)
 endif
+
+      # -fsanitize=* options makes GCC less smart than usual and
+      # increase number of 'maybe-uninitialized false-positives
+      CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
 endif
-- 
2.9.0

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

* Re: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code
       [not found]   ` <CY4PR11MB1751050A7C7AF840E94DDE60CBD00@CY4PR11MB1751.namprd11.prod.outlook.com>
@ 2016-10-17 22:29     ` Arnd Bergmann
  2016-10-17 22:37       ` Linus Torvalds
  0 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:29 UTC (permalink / raw)
  To: Patrick Farrell
  Cc: Oleg Drokin, devel, Greg Kroah-Hartman, linux-kernel,
	Linus Torvalds, lustre-devel

On Monday, October 17, 2016 10:23:00 PM CEST Patrick Farrell wrote:
> Arnd,
> 
> 
> The description and the actual patch don't seem to match up.  Am I missing something?

Sorry, I pasted the wrong error message when writing the changelog.

> From: lustre-devel <lustre-devel-bounces@lists.lustre.org> on behalf of Arnd Bergmann <arnd@arndb.de>
> Sent: Monday, October 17, 2016 5:08:55 PM
> To: Oleg Drokin
> Cc: devel@driverdev.osuosl.org; Arnd Bergmann; Greg Kroah-Hartman; linux-kernel@vger.kernel.org; Linus Torvalds; lustre-devel@lists.lustre.org
> Subject: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code
> 
> A recent rework removed the initialization of the successful return
> code from lpfc_write_firmware:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
> drivers/scsi/lpfc/lpfc_init.c:10333:214: error: format '%ld' expects argument of type 'long int', but argument 9 has type 'size_t {aka const unsigned int}' [-Werror=format=]
> 
> This adds it back.


It should have been this warning:


drivers/staging/lustre/lustre/lov/lov_pack.c: In function 'lov_getstripe':
drivers/staging/lustre/lustre/lov/lov_pack.c:426:9: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  return rc;
         ^~
drivers/staging/lustre/lustre/lov/lov_pack.c:313:6: note: 'rc' was declared here


	Arnd

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

* Re: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code
  2016-10-17 22:29     ` [lustre-devel] " Arnd Bergmann
@ 2016-10-17 22:37       ` Linus Torvalds
  2016-10-17 23:00         ` Arnd Bergmann
  0 siblings, 1 reply; 74+ messages in thread
From: Linus Torvalds @ 2016-10-17 22:37 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Patrick Farrell, Oleg Drokin, devel, Greg Kroah-Hartman,
	linux-kernel, lustre-devel

On Mon, Oct 17, 2016 at 3:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> Sorry, I pasted the wrong error message when writing the changelog.

Not just the warning, the summary above it talks about the wrong
function too. And the commit it references doesn't actually exist
either. So apparently this is against something else than my tree.

               Linus

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

* [PATCH 08/28 v2] staging: lustre: restore initialization of return code
  2016-10-17 22:08 ` [PATCH 08/28] staging: lustre: restore initialization of return code Arnd Bergmann
       [not found]   ` <CY4PR11MB1751050A7C7AF840E94DDE60CBD00@CY4PR11MB1751.namprd11.prod.outlook.com>
@ 2016-10-17 22:42   ` Arnd Bergmann
  1 sibling, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:42 UTC (permalink / raw)
  To: Oleg Drokin
  Cc: Linus Torvalds, linux-kernel, John L . Hammond, Jinshan Xiong,
	James Simmons, Andreas Dilger, Greg Kroah-Hartman, lustre-devel,
	devel

A recent rework dropped the initialization of the initialization of the
successful return code in lov_getstripe:

drivers/staging/lustre/lustre/lov/lov_pack.c: In function 'lov_getstripe':
drivers/staging/lustre/lustre/lov/lov_pack.c:426:9: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/staging/lustre/lustre/lov/lov_pack.c:313:6: note: 'rc' was declared here

This adds it back.

Fixes: e10a431b3fd0 ("staging: lustre: lov: move LSM to LOV layer")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: fix embarrassing incorrect changelog

diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
index 17bceadd66f8..ccc1fae35791 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -418,6 +418,8 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm,
 	((struct lov_user_md *)lmmk)->lmm_stripe_count = lum.lmm_stripe_count;
 	if (copy_to_user(lump, lmmk, lmm_size))
 		rc = -EFAULT;
+	else
+		rc = 0;
 
 out_free:
 	kvfree(lmmk);

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

* Re: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code
  2016-10-17 22:37       ` Linus Torvalds
@ 2016-10-17 23:00         ` Arnd Bergmann
  0 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-17 23:00 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Patrick Farrell, Oleg Drokin, devel, Greg Kroah-Hartman,
	linux-kernel, lustre-devel

On Monday, October 17, 2016 3:37:11 PM CEST Linus Torvalds wrote:
> On Mon, Oct 17, 2016 at 3:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > Sorry, I pasted the wrong error message when writing the changelog.
> 
> Not just the warning, the summary above it talks about the wrong
> function too. And the commit it references doesn't actually exist
> either. So apparently this is against something else than my tree.

Right, it slipped in here together with the other lustre patch when
I rebased my longer series (based on linux-next) onto v4.9-rc1.

Both applied cleanly to v4.9-rc1 and they are required on linux-next
(not the version with the wrong changelog of course) but have no
effect in mainline so far.

I'll double-check the rest of the series tomorrow, to see if some
of the other patches also have the same problem and are only
needed on linux-next. I meant to send those as part of the
separate series for v4.10.

For now, it would be good to know if you see any remaining warnings
on your machine after applying the current series (with or without
the lustre patches, doesn't matter) to a test branch. Some other
patches in the series likely need to go through a second revision
anyway.

	Arnd

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

* Re: [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank
  2016-10-17 22:13 ` [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank Arnd Bergmann
@ 2016-10-17 23:47   ` Mario Kleiner
  2016-10-18  7:46     ` Daniel Vetter
  0 siblings, 1 reply; 74+ messages in thread
From: Mario Kleiner @ 2016-10-17 23:47 UTC (permalink / raw)
  To: Arnd Bergmann, David Airlie
  Cc: Linus Torvalds, linux-kernel, Rob Clark, Daniel Vetter,
	Dave Airlie, Ville Syrjälä,
	Alex Deucher, Gustavo Padovan, Matthew Auld, dri-devel

On 10/18/2016 12:13 AM, Arnd Bergmann wrote:
> gcc warns about the timestamp in drm_wait_vblank being possibly
> used without an initialization:
>
> drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
> drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
> drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This can happen if drm_vblank_count_and_time() returns 0 in its
> error path. To sanitize the error case, I'm changing that function
> to return a zero timestamp when it fails.
>
> Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> First submitted in January 2016, second submission in February,
> the patch is still required.
>
>  drivers/gpu/drm/drm_irq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index b969a64..48a6167 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
>  	u32 vblank_count;
>  	unsigned int seq;
>
> -	if (WARN_ON(pipe >= dev->num_crtcs))
> +	if (WARN_ON(pipe >= dev->num_crtcs)) {
> +		*vblanktime = (struct timeval) { 0 };
>  		return 0;
> +	}
>
>  	do {
>  		seq = read_seqbegin(&vblank->seqlock);
>

Looks good to me.

Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>

-mario

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

* Re: [PATCH 07/28] ceph: avoid false positive maybe-uninitialized warning
  2016-10-17 22:08 ` [PATCH 07/28] ceph: avoid false positive maybe-uninitialized warning Arnd Bergmann
@ 2016-10-18  2:07   ` Yan, Zheng
  0 siblings, 0 replies; 74+ messages in thread
From: Yan, Zheng @ 2016-10-18  2:07 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Sage Weil, Ilya Dryomov, Linus Torvalds, open list, Michal Hocko,
	Kirill A. Shutemov, Andrew Morton, Nikolay Borisov, ceph-devel


> On 18 Oct 2016, at 06:08, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> A recent rework removed the initialization of the local 'root'
> variable that is returned from ceph_real_mount:
> 
> fs/ceph/super.c: In function 'ceph_mount':
> fs/ceph/super.c:1016:38: error: 'root' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> It's not obvious to me what the correct fix is, so this just
> returns the saved root as we did before.
> 
> Fixes: ce2728aaa82b ("ceph: avoid accessing / when mounting a subpath")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: "Yan, Zheng" <zyan@redhat.com>
> ---
> fs/ceph/super.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index a29ffce..79a4be8 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -821,7 +821,8 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc)
> 	dout("mount start %p\n", fsc);
> 	mutex_lock(&fsc->client->mount_mutex);
> 
> -	if (!fsc->sb->s_root) {
> +	root = dget(fsc->sb->s_root);
> +	if (!root) {
> 		const char *path;
> 		err = __ceph_open_session(fsc->client, started);
> 		if (err < 0)
> — 

This bug has already been fixed.

Regards
Yan, Zheng

> 2.9.0
> 

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

* Re: [PATCH 00/28] Reenable maybe-uninitialized warnings
  2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
                   ` (27 preceding siblings ...)
  2016-10-17 22:19 ` [PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning Arnd Bergmann
@ 2016-10-18  5:08 ` Christoph Hellwig
  28 siblings, 0 replies; 74+ messages in thread
From: Christoph Hellwig @ 2016-10-18  5:08 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Torvalds, linux-kernel, x86, linux-media,
	Mauro Carvalho Chehab, Martin Schwidefsky, linux-s390,
	Ilya Dryomov, dri-devel, linux-mtd, Herbert Xu, linux-crypto,
	David S. Miller, netdev, Greg Kroah-Hartman, ceph-devel,
	linux-f2fs-devel, linux-ext4, netfilter-devel

On Tue, Oct 18, 2016 at 12:03:28AM +0200, Arnd Bergmann wrote:
> This is a set of patches that I hope to get into v4.9 in some form
> in order to turn on the -Wmaybe-uninitialized warnings again.

Hi Arnd,

I jsut complained to Geert that I was introducing way to many
bugs or pointless warnings for some compilers lately, but gcc didn't
warn me about them.  From a little research the lack of
-Wmaybe-uninitialized seems to be the reason for it, so I'm all
for re-enabling it.

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

* Re: [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning
  2016-10-17 22:05 ` [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning Arnd Bergmann
@ 2016-10-18  5:15   ` Christoph Hellwig
  2016-10-18  9:30     ` Jan Kara
  0 siblings, 1 reply; 74+ messages in thread
From: Christoph Hellwig @ 2016-10-18  5:15 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jan Kara, Linus Torvalds, linux-kernel, Christoph Hellwig,
	Dave Chinner, Ross Zwisler, Dave Chinner, Al Viro, Andrew Morton,
	Matthew Wilcox, Carlos Maiolino, linux-ext4

Thanks Arnd, this looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 10/28] UBI: fix uninitialized access of vid_hdr pointer
  2016-10-17 22:10 ` [PATCH 10/28] UBI: fix uninitialized access of vid_hdr pointer Arnd Bergmann
@ 2016-10-18  5:17   ` Boris Brezillon
  0 siblings, 0 replies; 74+ messages in thread
From: Boris Brezillon @ 2016-10-18  5:17 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Artem Bityutskiy, Richard Weinberger, Linus Torvalds,
	linux-kernel, linux-mtd, Brian Norris, David Woodhouse,
	Geert Uytterhoeven

Hi Arnd,

On Tue, 18 Oct 2016 00:10:13 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> A rework of UBI that just appeared in linux-next during the merge
> window introduced caused the recover_peb to use a variable that
> is never initialized as seen from this gcc warning:
> 
> drivers/mtd/ubi/eba.c: In function ‘recover_peb’:
> drivers/mtd/ubi/eba.c:744:40: error: ‘vid_hdr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> It seems clear that the change to the function arguments was missing
> the initialization that I'm now adding back to restore the
> way the function was working before.

Thanks for the fix, but Geert already sent a patch for this bug a few
days ago.

Regards,

Boris

> 
> Fixes: 3291b52f9ff0 ("UBI: introduce the VID buffer concept")
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Richard Weinberger <richard@nod.at>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/mtd/ubi/eba.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
> index 95c4048..2e152be 100644
> --- a/drivers/mtd/ubi/eba.c
> +++ b/drivers/mtd/ubi/eba.c
> @@ -719,7 +719,7 @@ static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum,
>  			   struct ubi_vid_io_buf *vidb, bool *retry)
>  {
>  	struct ubi_device *ubi = vol->ubi;
> -	struct ubi_vid_hdr *vid_hdr;
> +	struct ubi_vid_hdr *vid_hdr = ubi_get_vid_hdr(vidb);
>  	int new_pnum, err, vol_id = vol->vol_id, data_size;
>  	uint32_t crc;
>  

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

* Re: [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode
  2016-10-17 22:05 ` [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode Arnd Bergmann
@ 2016-10-18  5:19   ` Boris Brezillon
       [not found]     ` <1476785552.24626.4.camel@mtkswgap22>
  0 siblings, 1 reply; 74+ messages in thread
From: Boris Brezillon @ 2016-10-18  5:19 UTC (permalink / raw)
  To: Arnd Bergmann, Jorge Ramirez-Ortiz, RogerCC Lin
  Cc: Richard Weinberger, linux-mediatek, linux-kernel, Linus Torvalds,
	linux-mtd, Matthias Brugger, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Tue, 18 Oct 2016 00:05:31 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> warning for the mtk_ecc_encode function:
> 
> drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> The function for some reason contains a double byte swap on big-endian
> builds to get the OOB data into the correct order again, and is written
> in a slightly confusing way.
> 
> Using a simple memcpy32_fromio() to read the data simplifies it a lot
> so it becomes more readable and produces no warning. However, the
> output might not have 32-bit alignment, so we have to use another
> memcpy to avoid taking alignment faults or writing beyond the end
> of the array.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?

> ---
> v2: move temporary buffer into struct mtk_ecc instead of having it
>     on the stack, as suggested by Boris Brezillon
> ---
>  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> index d54f666..dbf2562 100644
> --- a/drivers/mtd/nand/mtk_ecc.c
> +++ b/drivers/mtd/nand/mtk_ecc.c
> @@ -86,6 +86,8 @@ struct mtk_ecc {
>  	struct completion done;
>  	struct mutex lock;
>  	u32 sectors;
> +
> +	u8 eccdata[112];
>  };
>  
>  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
>  		   u8 *data, u32 bytes)
>  {
>  	dma_addr_t addr;
> -	u8 *p;
> -	u32 len, i, val;
> -	int ret = 0;
> +	u32 len;
> +	int ret;
>  
>  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
>  	ret = dma_mapping_error(ecc->dev, addr);
> @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
>  
>  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
>  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> -	p = data + bytes;
>  
> -	/* write the parity bytes generated by the ECC back to the OOB region */
> -	for (i = 0; i < len; i++) {
> -		if ((i % 4) == 0)
> -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> -	}
> +	/* write the parity bytes generated by the ECC back to temp buffer */
> +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> +
> +	/* copy into possibly unaligned OOB region with actual length */
> +	memcpy(data + bytes, ecc->eccdata, len);
>  timeout:
>  
>  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);

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

* Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning
  2016-10-17 22:05 ` [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning Arnd Bergmann
@ 2016-10-18  6:47   ` Haggai Eran
  2016-10-18 10:18     ` Arnd Bergmann
  0 siblings, 1 reply; 74+ messages in thread
From: Haggai Eran @ 2016-10-18  6:47 UTC (permalink / raw)
  To: Arnd Bergmann, Doug Ledford
  Cc: Linus Torvalds, linux-kernel, Sean Hefty, Hal Rosenstock,
	Matan Barak, Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
	Alex Vesker, Guy Shapiro, linux-rdma

On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
> @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
>  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
>  					  const struct cma_req_info *req)
>  {
> -	struct sockaddr_storage listen_addr_storage, src_addr_storage;
> +	struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};

Doesn't this still translate to an extra initialization that Doug was
worried about?

Haggai

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

* Re: [PATCH 25/28] s390: pci: don't print uninitialized data for debugging
  2016-10-17 22:16 ` [PATCH 25/28] s390: pci: don't print uninitialized data for debugging Arnd Bergmann
@ 2016-10-18  6:48   ` Martin Schwidefsky
  2016-10-18  8:53     ` Sebastian Ott
  0 siblings, 1 reply; 74+ messages in thread
From: Martin Schwidefsky @ 2016-10-18  6:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, Linus Torvalds, linux-kernel, Sebastian Ott,
	Gerald Schaefer, Joerg Roedel, Christian Borntraeger, linux-s390

On Tue, 18 Oct 2016 00:16:13 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> gcc correctly warns about an incorrect use of the 'pa' variable
> in case we pass an empty scatterlist to __s390_dma_map_sg:
> 
> arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg':
> arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This adds a bogus initialization to the function to sanitize
> the debug output.  I would have preferred a solution without
> the initialization, but I only got the report from the
> kbuild bot after turning on the warning again, and didn't
> manage to reproduce it myself.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/pci/pci_dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> index 7350c8b..6b2f72f 100644
> --- a/arch/s390/pci/pci_dma.c
> +++ b/arch/s390/pci/pci_dma.c
> @@ -423,7 +423,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
>  	dma_addr_t dma_addr_base, dma_addr;
>  	int flags = ZPCI_PTE_VALID;
>  	struct scatterlist *s;
> -	unsigned long pa;
> +	unsigned long pa = 0;
>  	int ret;
> 
>  	size = PAGE_ALIGN(size);

The compiler is right. pa is set in the for-loop. If "dma_addr < dma_addr_base + size"
is never true and __dma_purge_tlb() returns with an error, pa *is* uninitialized.
The fix seems sensible to me.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

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

* Re: [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank
  2016-10-17 23:47   ` Mario Kleiner
@ 2016-10-18  7:46     ` Daniel Vetter
  0 siblings, 0 replies; 74+ messages in thread
From: Daniel Vetter @ 2016-10-18  7:46 UTC (permalink / raw)
  To: Mario Kleiner
  Cc: Arnd Bergmann, David Airlie, Linus Torvalds, linux-kernel,
	Rob Clark, Daniel Vetter, Dave Airlie, Ville Syrjälä,
	Alex Deucher, Gustavo Padovan, Matthew Auld, dri-devel

On Tue, Oct 18, 2016 at 01:47:24AM +0200, Mario Kleiner wrote:
> On 10/18/2016 12:13 AM, Arnd Bergmann wrote:
> > gcc warns about the timestamp in drm_wait_vblank being possibly
> > used without an initialization:
> > 
> > drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
> > drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
> > drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 
> > This can happen if drm_vblank_count_and_time() returns 0 in its
> > error path. To sanitize the error case, I'm changing that function
> > to return a zero timestamp when it fails.
> > 
> > Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
> > Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > First submitted in January 2016, second submission in February,
> > the patch is still required.

Hm, sorry I missed that.

> >  drivers/gpu/drm/drm_irq.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index b969a64..48a6167 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
> >  	u32 vblank_count;
> >  	unsigned int seq;
> > 
> > -	if (WARN_ON(pipe >= dev->num_crtcs))
> > +	if (WARN_ON(pipe >= dev->num_crtcs)) {
> > +		*vblanktime = (struct timeval) { 0 };
> >  		return 0;
> > +	}
> > 
> >  	do {
> >  		seq = read_seqbegin(&vblank->seqlock);
> > 
> 
> Looks good to me.
> 
> Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Applied to drm-misc, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH 25/28] s390: pci: don't print uninitialized data for debugging
  2016-10-18  6:48   ` Martin Schwidefsky
@ 2016-10-18  8:53     ` Sebastian Ott
  0 siblings, 0 replies; 74+ messages in thread
From: Sebastian Ott @ 2016-10-18  8:53 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Arnd Bergmann, Heiko Carstens, Linus Torvalds, linux-kernel,
	Gerald Schaefer, Joerg Roedel, Christian Borntraeger, linux-s390

On Tue, 18 Oct 2016, Martin Schwidefsky wrote:
> On Tue, 18 Oct 2016 00:16:13 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > gcc correctly warns about an incorrect use of the 'pa' variable
> > in case we pass an empty scatterlist to __s390_dma_map_sg:
> > 
> > arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg':
> > arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > 
> > This adds a bogus initialization to the function to sanitize
> > the debug output.  I would have preferred a solution without
> > the initialization, but I only got the report from the
> > kbuild bot after turning on the warning again, and didn't
> > manage to reproduce it myself.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/s390/pci/pci_dma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> > index 7350c8b..6b2f72f 100644
> > --- a/arch/s390/pci/pci_dma.c
> > +++ b/arch/s390/pci/pci_dma.c
> > @@ -423,7 +423,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
> >  	dma_addr_t dma_addr_base, dma_addr;
> >  	int flags = ZPCI_PTE_VALID;
> >  	struct scatterlist *s;
> > -	unsigned long pa;
> > +	unsigned long pa = 0;
> >  	int ret;
> > 
> >  	size = PAGE_ALIGN(size);
> 
> The compiler is right. pa is set in the for-loop. If "dma_addr < dma_addr_base + size"
> is never true and __dma_purge_tlb() returns with an error, pa *is* uninitialized.
> The fix seems sensible to me.

Although that could only happen if map_sg is called with zero length sg
list entries I'm in favor getting rid of that warning.

Acked-by: Sebastian Ott <sebott@linux.vnet.ibm.com>

Regards,
Sebastian

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

* Re: [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning
  2016-10-18  5:15   ` Christoph Hellwig
@ 2016-10-18  9:30     ` Jan Kara
  0 siblings, 0 replies; 74+ messages in thread
From: Jan Kara @ 2016-10-18  9:30 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Arnd Bergmann, Jan Kara, Linus Torvalds, linux-kernel,
	Christoph Hellwig, Dave Chinner, Ross Zwisler, Dave Chinner,
	Al Viro, Andrew Morton, Matthew Wilcox, Carlos Maiolino,
	linux-ext4

On Mon 17-10-16 22:15:29, Christoph Hellwig wrote:
> Thanks Arnd, this looks fine to me:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks! I have pulled the patch into my tree and will push it to Linus.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH 16/28] pcmcia: fix return value of soc_pcmcia_regulator_set
  2016-10-17 22:13 ` [PATCH 16/28] pcmcia: fix return value of soc_pcmcia_regulator_set Arnd Bergmann
@ 2016-10-18  9:42   ` Russell King - ARM Linux
  0 siblings, 0 replies; 74+ messages in thread
From: Russell King - ARM Linux @ 2016-10-18  9:42 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Linus Torvalds, linux-kernel, linux-pcmcia

On Tue, Oct 18, 2016 at 12:13:37AM +0200, Arnd Bergmann wrote:
> The newly introduced soc_pcmcia_regulator_set() function sometimes returns
> without setting its return code, as shown by this warning:
> 
> drivers/pcmcia/soc_common.c: In function 'soc_pcmcia_regulator_set':
> drivers/pcmcia/soc_common.c:112:5: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This changes it to propagate the regulator_disable() result instead.

I guess this is the problem with the stupid patch which silences this
warning - I don't see this warning here.

Having this warning disabled means that _real_ coding errors end up
making their way into the kernel (yes, this should be an error, this
is not a warning, because the value of 'ret' is completely undefined,
and therefore the behaviour of the following code is undefined.)

With the warning silenced, it means that such errors are undetectable.
I knew nothing about this until I received this patch, and I always
check that the code I send is warning-free on the GCC I'm using.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

* Re: [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized
  2016-10-17 22:10 ` [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized Arnd Bergmann
@ 2016-10-18  9:57   ` Ilya Dryomov
  2016-10-18 10:04     ` Arnd Bergmann
  0 siblings, 1 reply; 74+ messages in thread
From: Ilya Dryomov @ 2016-10-18  9:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Sage Weil, Linus Torvalds, linux-kernel, Alex Elder,
	Mike Christie, Ceph Development

On Tue, Oct 18, 2016 at 12:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> When building with gcc-4.9 -Wmaybe-uninitialized, we get a bogus
> warning in rbd_watch_cb, as the variable is not used at all
> in the one case in which it is not initialized first:
>
> drivers/block/rbd.c: In function ‘rbd_watch_cb’:
> drivers/block/rbd.c:3690:5: error: ‘struct_v’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/block/rbd.c:3759:5: note: ‘struct_v’ was declared here
>
> Later compiler versions fix this, but adding another initialization
> here is harmless and lets us build cleanly with 4.9 as well.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/block/rbd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index abb7162..4ab990b 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -3776,6 +3776,7 @@ static void rbd_watch_cb(void *arg, u64 notify_id, u64 cookie,
>         } else {
>                 /* legacy notification for header updates */
>                 notify_op = RBD_NOTIFY_OP_HEADER_UPDATE;
> +               struct_v = 0;
>                 len = 0;
>         }

It already got silenced by initializing at declaration in one of the
downstream trees, so I'd rather we do

@@ -3756,7 +3819,7 @@ static void rbd_watch_cb(void *arg, u64
notify_id, u64 cookie,
        struct rbd_device *rbd_dev = arg;
        void *p = data;
        void *const end = p + data_len;
-       u8 struct_v;
+       u8 struct_v = 0;
        u32 len;
        u32 notify_op;
        int ret;

to reduce the churn.

The "block" prefix is redundant and "rdb" should be "rbd" in the subject.

Thanks,

                Ilya

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

* Re: [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized
  2016-10-18  9:57   ` Ilya Dryomov
@ 2016-10-18 10:04     ` Arnd Bergmann
  0 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-18 10:04 UTC (permalink / raw)
  To: Ilya Dryomov
  Cc: Sage Weil, Linus Torvalds, linux-kernel, Alex Elder,
	Mike Christie, Ceph Development

On Tuesday, October 18, 2016 11:57:33 AM CEST Ilya Dryomov wrote:
> It already got silenced by initializing at declaration in one of the
> downstream trees, so I'd rather we do
> 
> @@ -3756,7 +3819,7 @@ static void rbd_watch_cb(void *arg, u64
> notify_id, u64 cookie,
>         struct rbd_device *rbd_dev = arg;
>         void *p = data;
>         void *const end = p + data_len;
> -       u8 struct_v;
> +       u8 struct_v = 0;
>         u32 len;
>         u32 notify_op;
>         int ret;
> 
> to reduce the churn.

Fair enough. I try to avoid adding extraneous initializations like
this, but my suggested change is not all that different here,
except if ceph_start_decoding() got changed in a way that could
lead to another uninitialized use.

> The "block" prefix is redundant and "rdb" should be "rbd" in the subject.

Oops.

	Arnd

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

* Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning
  2016-10-18  6:47   ` Haggai Eran
@ 2016-10-18 10:18     ` Arnd Bergmann
  2016-10-18 10:32       ` Haggai Eran
  0 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-18 10:18 UTC (permalink / raw)
  To: Haggai Eran
  Cc: Doug Ledford, Linus Torvalds, linux-kernel, Sean Hefty,
	Hal Rosenstock, Matan Barak, Leon Romanovsky, Sagi Grimberg,
	Bart Van Assche, Alex Vesker, Guy Shapiro, linux-rdma

On Tuesday, October 18, 2016 9:47:31 AM CEST Haggai Eran wrote:
> On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
> > @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
> >  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
> >                                         const struct cma_req_info *req)
> >  {
> > -     struct sockaddr_storage listen_addr_storage, src_addr_storage;
> > +     struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
> 
> Doesn't this still translate to an extra initialization that Doug was
> worried about?

Thanks for spotting this. I must have screwed up while rebasing the patch
at some point, this one change should not be there, the other changes by
themselves sufficiently address the warning.

	Arnd

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

* Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning
  2016-10-18 10:18     ` Arnd Bergmann
@ 2016-10-18 10:32       ` Haggai Eran
  0 siblings, 0 replies; 74+ messages in thread
From: Haggai Eran @ 2016-10-18 10:32 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Doug Ledford, Linus Torvalds, linux-kernel, Sean Hefty,
	Hal Rosenstock, Matan Barak, Leon Romanovsky, Sagi Grimberg,
	Bart Van Assche, Alex Vesker, Guy Shapiro, linux-rdma

On 10/18/2016 1:18 PM, Arnd Bergmann wrote:
> On Tuesday, October 18, 2016 9:47:31 AM CEST Haggai Eran wrote:
>> On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
>>> @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
>>>  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
>>>                                         const struct cma_req_info *req)
>>>  {
>>> -     struct sockaddr_storage listen_addr_storage, src_addr_storage;
>>> +     struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
>>
>> Doesn't this still translate to an extra initialization that Doug was
>> worried about?
> 
> Thanks for spotting this. I must have screwed up while rebasing the patch
> at some point, this one change should not be there, the other changes by
> themselves sufficiently address the warning.

Okay, other than this the patch looks good to me.

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

* Re: [PATCH 22/28] x86: apm: avoid uninitialized data
  2016-10-17 22:16 ` [PATCH 22/28] x86: apm: avoid uninitialized data Arnd Bergmann
@ 2016-10-18 13:05   ` Jiri Kosina
  2016-10-18 21:35   ` Luis R. Rodriguez
  1 sibling, 0 replies; 74+ messages in thread
From: Jiri Kosina @ 2016-10-18 13:05 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Torvalds, linux-kernel, x86, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Luis R. Rodriguez

On Tue, 18 Oct 2016, Arnd Bergmann wrote:

> apm_bios_call() can fail, and return a status in its argument
> structure. If that status however is zero during a call from
> apm_get_power_status(), we end up using data that may have
> never been set, as reported by "gcc -Wmaybe-uninitialized":
> 
> arch/x86/kernel/apm_32.c: In function ‘apm’:
> arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
> arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
> 
> This changes the function to return "APM_NO_ERROR" here, which
> makes the code more robust to broken BIOS versions, and avoids
> the warning.
> 
> Cc: x86@kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Makes sense.

Reviewed-by: Jiri Kosina <jkosina@suse.cz>

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

* Re: [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning
  2016-10-17 22:05 ` [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning Arnd Bergmann
@ 2016-10-18 15:23   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 74+ messages in thread
From: Pablo Neira Ayuso @ 2016-10-18 15:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
	Linus Torvalds, linux-kernel, netfilter-devel, coreteam, netdev

On Tue, Oct 18, 2016 at 12:05:30AM +0200, Arnd Bergmann wrote:
> The newly added nft_range_eval() function handles the two possible
> nft range operations, but as the compiler warning points out,
> any unexpected value would lead to the 'mismatch' variable being
> used without being initialized:
> 
> net/netfilter/nft_range.c: In function 'nft_range_eval':
> net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This removes the variable in question and instead moves the
> condition into the switch itself, which is potentially more
> efficient than adding a bogus 'default' clause as in my
> first approach, and is nicer than using the 'uninitialized_var'
> macro.

Applied to the nf tree, thanks Arnd.

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

* Re: [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable
  2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
@ 2016-10-18 18:21   ` David Miller
  0 siblings, 0 replies; 74+ messages in thread
From: David Miller @ 2016-10-18 18:21 UTC (permalink / raw)
  To: arnd
  Cc: torvalds, linux-kernel, tremyfr, f.fainelli,
	bcm-kernel-feedback-list, andrew, netdev, linux-arm-kernel

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:08 +0200

> gcc found a reference to an uninitialized variable in the error handling
> of bcm_enet_open, introduced by a recent cleanup:
> 
> drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
> drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This makes the use of that variable conditional, so we only reference it
> here after it has been used before. Unlike my normal patches, I have not
> build-tested this one, as I don't currently have mips test in my
> randconfig setup.
> 
> Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
> Cc: Philippe Reynes <tremyfr@gmail.com>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

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

* Re: [PATCH 21/28] net/hyperv: avoid uninitialized variable
  2016-10-17 22:16 ` [PATCH 21/28] net/hyperv: avoid " Arnd Bergmann
@ 2016-10-18 18:21   ` David Miller
  0 siblings, 0 replies; 74+ messages in thread
From: David Miller @ 2016-10-18 18:21 UTC (permalink / raw)
  To: arnd
  Cc: kys, haiyangz, torvalds, linux-kernel, vkuznets, stephen, sixiao,
	devel, netdev

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:09 +0200

> The hdr_offset variable is only if we deal with a TCP or UDP packet,
> but as the check surrounding its usage tests for skb_is_gso()
> instead, the compiler has no idea if the variable is initialized
> or not at that point:
> 
> drivers/net/hyperv/netvsc_drv.c: In function ‘netvsc_start_xmit’:
> drivers/net/hyperv/netvsc_drv.c:494:42: error: ‘hdr_offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This adds an additional check for the transport type, which
> tells the compiler that this path cannot happen. Since the
> get_net_transport_info() function should always be inlined
> here, I don't expect this to result in additional runtime
> checks.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

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

* Re: [PATCH 27/28] rocker: fix maybe-uninitialized warning
  2016-10-17 22:16 ` [PATCH 27/28] rocker: fix maybe-uninitialized warning Arnd Bergmann
@ 2016-10-18 18:21   ` David Miller
  0 siblings, 0 replies; 74+ messages in thread
From: David Miller @ 2016-10-18 18:21 UTC (permalink / raw)
  To: arnd; +Cc: jiri, torvalds, linux-kernel, idosch, dan.carpenter, netdev

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:15 +0200

> In some rare configurations, we get a warning about the 'index' variable
> being used without an initialization:
> 
> drivers/net/ethernet/rocker/rocker_ofdpa.c: In function ‘ofdpa_port_fib_ipv4.isra.16.constprop’:
> drivers/net/ethernet/rocker/rocker_ofdpa.c:2425:92: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This is a false positive, the logic is just a bit too complex for gcc
> to follow here. Moving the intialization of 'index' a little further
> down makes it clear to gcc that the function always returns an error
> if it is not initialized.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

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

* Re: [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode
       [not found]     ` <1476785552.24626.4.camel@mtkswgap22>
@ 2016-10-18 19:45       ` Boris Brezillon
  0 siblings, 0 replies; 74+ messages in thread
From: Boris Brezillon @ 2016-10-18 19:45 UTC (permalink / raw)
  To: RogerCC.Lin
  Cc: Arnd Bergmann, Jorge Ramirez-Ortiz, Richard Weinberger,
	linux-mediatek, linux-kernel, Linus Torvalds, linux-mtd,
	Matthias Brugger, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Tue, 18 Oct 2016 18:12:32 +0800
RogerCC.Lin <rogercc.lin@mediatek.com> wrote:

> On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> > On Tue, 18 Oct 2016 00:05:31 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >   
> > > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > > warning for the mtk_ecc_encode function:
> > > 
> > > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > 
> > > The function for some reason contains a double byte swap on big-endian
> > > builds to get the OOB data into the correct order again, and is written
> > > in a slightly confusing way.
> > > 
> > > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > > so it becomes more readable and produces no warning. However, the
> > > output might not have 32-bit alignment, so we have to use another
> > > memcpy to avoid taking alignment faults or writing beyond the end
> > > of the array.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>  
> > 
> > Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?  
> Tested, this patch is OK,
> Tested-by: RogerCC Lin <rogercc.lin@mediatek.com>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Brian, can you take this patch for the next -rc?

> 
> >   
> > > ---
> > > v2: move temporary buffer into struct mtk_ecc instead of having it
> > >     on the stack, as suggested by Boris Brezillon
> > > ---
> > >  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> > >  1 file changed, 9 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > > index d54f666..dbf2562 100644
> > > --- a/drivers/mtd/nand/mtk_ecc.c
> > > +++ b/drivers/mtd/nand/mtk_ecc.c
> > > @@ -86,6 +86,8 @@ struct mtk_ecc {
> > >  	struct completion done;
> > >  	struct mutex lock;
> > >  	u32 sectors;
> > > +
> > > +	u8 eccdata[112];
> > >  };
> > >  
> > >  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >  		   u8 *data, u32 bytes)
> > >  {
> > >  	dma_addr_t addr;
> > > -	u8 *p;
> > > -	u32 len, i, val;
> > > -	int ret = 0;
> > > +	u32 len;
> > > +	int ret;
> > >  
> > >  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> > >  	ret = dma_mapping_error(ecc->dev, addr);
> > > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >  
> > >  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> > >  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > > -	p = data + bytes;
> > >  
> > > -	/* write the parity bytes generated by the ECC back to the OOB region */
> > > -	for (i = 0; i < len; i++) {
> > > -		if ((i % 4) == 0)
> > > -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > > -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > > -	}
> > > +	/* write the parity bytes generated by the ECC back to temp buffer */
> > > +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > > +
> > > +	/* copy into possibly unaligned OOB region with actual length */
> > > +	memcpy(data + bytes, ecc->eccdata, len);
> > >  timeout:
> > >  
> > >  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);  
> >   
> 
> 

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

* Re: [PATCH 22/28] x86: apm: avoid uninitialized data
  2016-10-17 22:16 ` [PATCH 22/28] x86: apm: avoid uninitialized data Arnd Bergmann
  2016-10-18 13:05   ` Jiri Kosina
@ 2016-10-18 21:35   ` Luis R. Rodriguez
  1 sibling, 0 replies; 74+ messages in thread
From: Luis R. Rodriguez @ 2016-10-18 21:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jiri Kosina, Linus Torvalds, linux-kernel, x86, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, Luis R. Rodriguez, Boris Ostrovsky,
	Juergen Gross

On Tue, Oct 18, 2016 at 12:16:10AM +0200, Arnd Bergmann wrote:
> apm_bios_call() can fail, and return a status in its argument
> structure. If that status however is zero during a call from
> apm_get_power_status(), we end up using data that may have
> never been set, as reported by "gcc -Wmaybe-uninitialized":

Userspace *may* already rely on this broken behavior for broken
BIOSes which may leave the return value as 0, ignoring that,
this change makes sense to me given that handling the error
would be better than relying any possible invalid data.

> arch/x86/kernel/apm_32.c: In function ‘apm’:
> arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
> arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
> 
> This changes the function to return "APM_NO_ERROR" here, which
> makes the code more robust to broken BIOS versions, and avoids
> the warning.
> 
> Cc: x86@kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org>

  Luis

> ---
>  arch/x86/kernel/apm_32.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
> index c7364bd..51287cd 100644
> --- a/arch/x86/kernel/apm_32.c
> +++ b/arch/x86/kernel/apm_32.c
> @@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, u_short *bat, u_short *life)
>  
>  	if (apm_info.get_power_status_broken)
>  		return APM_32_UNSUPPORTED;
> -	if (apm_bios_call(&call))
> +	if (apm_bios_call(&call)) {
> +		if (!call.err)
> +			return APM_NO_ERROR;
>  		return call.err;
> +	}
>  	*status = call.ebx;
>  	*bat = call.ecx;
>  	if (apm_info.get_power_status_swabinminutes) {
> -- 
> 2.9.0
> 
> 

-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg

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

* Re: [PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable
  2016-10-17 22:13 ` [PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable Arnd Bergmann
@ 2016-10-23 21:25   ` Jonathan Cameron
  0 siblings, 0 replies; 74+ messages in thread
From: Jonathan Cameron @ 2016-10-23 21:25 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Torvalds, linux-kernel, Ico Doornekamp, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Greg Kroah-Hartman,
	Ioana Ciornei, Luis de Bethencourt, linux-iio, devel

On 17/10/16 23:13, Arnd Bergmann wrote:
> The newly added __sca3000_get_base_freq function handles all valid
> modes of the SCA3000_REG_ADDR_MODE register, but gcc notices
> that any other value (i.e. 0x00) causes the base_freq variable to
> not get initialized:
> 
> drivers/staging/iio/accel/sca3000_core.c: In function 'sca3000_write_raw':
> drivers/staging/iio/accel/sca3000_core.c:527:23: error: 'base_freq' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This adds explicit error handling for unexpected register values,
> to ensure this cannot happen.
> 
> Fixes: e0f3fc9b47e6 ("iio: accel: sca3000_core: implemented IIO_CHAN_INFO_SAMP_FREQ")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Ico Doornekamp <ico@pruts.nl>
> Cc: Jonathan Cameron <jic23@kernel.org>
> ---
> I submitted this on Sept 22, and Jonathan said he applied it to his
> 'togreg' tree, but it hasn't appeared in linux-next yet, presumably
> since this was not considered material for v4.9.
Yes, it's just gone to Greg near the start of a big pull request for
4.10.
> 
> If we enable the warning again by default, we may want to have the
> fix merged for v4.9 after all.
I have no particular objection to it being merged faster.

Given things have gone horribly wrong if the hardware returns a value
other than those handled, it is more in the category of a nice
tidy up than an true bug fix.  Mind you always nice to handle
possible broken hardware as cleanly as possible.

Still it's risk free and as I'm about to send Greg a pull request for
some fixes, I'll put this in there as well and it'll work it's way
towards mainline.

Jonathan
> 
>  drivers/staging/iio/accel/sca3000_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
> index d626125..564b36d 100644
> --- a/drivers/staging/iio/accel/sca3000_core.c
> +++ b/drivers/staging/iio/accel/sca3000_core.c
> @@ -468,6 +468,8 @@ static inline int __sca3000_get_base_freq(struct sca3000_state *st,
>  	case SCA3000_MEAS_MODE_OP_2:
>  		*base_freq = info->option_mode_2_freq;
>  		break;
> +	default:
> +		ret = -EINVAL;
>  	}
>  error_ret:
>  	return ret;
> 

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

* Re: [PATCH 26/28] nios2: fix timer initcall return value
  2016-10-17 22:16 ` [PATCH 26/28] nios2: fix timer initcall return value Arnd Bergmann
@ 2016-10-24  0:54   ` Ley Foon Tan
  0 siblings, 0 replies; 74+ messages in thread
From: Ley Foon Tan @ 2016-10-24  0:54 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Linus Torvalds, linux-kernel, Daniel Lezcano, nios2-dev

On Tue, Oct 18, 2016 at 6:16 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> When called more than twice, the nios2_time_init() function
> return an uninitialized value, as detected by gcc -Wmaybe-uninitialized
>
> arch/nios2/kernel/time.c: warning: 'ret' may be used uninitialized in this function
>
> This makes it return '0' here, matching the comment above the
> function.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/nios2/kernel/time.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/nios2/kernel/time.c b/arch/nios2/kernel/time.c
> index d9563dd..746bf5c 100644
> --- a/arch/nios2/kernel/time.c
> +++ b/arch/nios2/kernel/time.c
> @@ -324,6 +324,7 @@ static int __init nios2_time_init(struct device_node *timer)
>                 ret = nios2_clocksource_init(timer);
>                 break;
>         default:
> +               ret = 0;
>                 break;
>         }
Acked-by: Ley Foon Tan <lftan@altera.com>

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

* Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error
  2016-10-17 22:13 ` [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error Arnd Bergmann
@ 2016-10-24 17:27   ` Mark Brown
  2016-10-24 18:36     ` Heiner Kallweit
  2016-10-26 10:15   ` Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree Mark Brown
  1 sibling, 1 reply; 74+ messages in thread
From: Mark Brown @ 2016-10-24 17:27 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Torvalds, linux-kernel, Heiner Kallweit, Nobuteru Hayashi,
	linux-spi

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Tue, Oct 18, 2016 at 12:13:38AM +0200, Arnd Bergmann wrote:
> When we get a spurious interrupt in fsl_espi_irq, we end up
> processing four uninitalized bytes of data, as shown in this
> warning message:

This doesn't apply against current code, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error
  2016-10-24 17:27   ` Mark Brown
@ 2016-10-24 18:36     ` Heiner Kallweit
  2016-10-24 18:45       ` Mark Brown
  0 siblings, 1 reply; 74+ messages in thread
From: Heiner Kallweit @ 2016-10-24 18:36 UTC (permalink / raw)
  To: Mark Brown
  Cc: Arnd Bergmann, Linus Torvalds, linux-kernel, Heiner Kallweit,
	Nobuteru Hayashi, linux-spi

Am 24.10.2016 um 19:27 schrieb Mark Brown:
> On Tue, Oct 18, 2016 at 12:13:38AM +0200, Arnd Bergmann wrote:
>> When we get a spurious interrupt in fsl_espi_irq, we end up
>> processing four uninitalized bytes of data, as shown in this
>> warning message:
> 
> This doesn't apply against current code, please check and resend.
> 
The not yet reviewed part of my patch series from Oct 2nd,
namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
from RX FIFO" replaces the code in question.
There's more to fix like removing polling from the ISR.
If you prefer to apply Arnd's fix first I'd rebase the open part
of the patch series and resend it.

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

* Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error
  2016-10-24 18:36     ` Heiner Kallweit
@ 2016-10-24 18:45       ` Mark Brown
  2016-10-24 20:37         ` Arnd Bergmann
  0 siblings, 1 reply; 74+ messages in thread
From: Mark Brown @ 2016-10-24 18:45 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Arnd Bergmann, Linus Torvalds, linux-kernel, Heiner Kallweit,
	Nobuteru Hayashi, linux-spi

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On Mon, Oct 24, 2016 at 08:36:37PM +0200, Heiner Kallweit wrote:
> Am 24.10.2016 um 19:27 schrieb Mark Brown:

> > This doesn't apply against current code, please check and resend.

> The not yet reviewed part of my patch series from Oct 2nd,
> namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
> from RX FIFO" replaces the code in question.
> There's more to fix like removing polling from the ISR.
> If you prefer to apply Arnd's fix first I'd rebase the open part
> of the patch series and resend it.

If there are dependencies you should mention them when you resend (in
general you should always mention any unapplied or cross tree
dependencies when sending things).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error
  2016-10-24 18:45       ` Mark Brown
@ 2016-10-24 20:37         ` Arnd Bergmann
  2016-10-25 19:13           ` Mark Brown
  0 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-24 20:37 UTC (permalink / raw)
  To: Mark Brown
  Cc: Heiner Kallweit, Linus Torvalds, linux-kernel, Heiner Kallweit,
	Nobuteru Hayashi, linux-spi

On Monday, October 24, 2016 7:45:43 PM CEST Mark Brown wrote:
> On Mon, Oct 24, 2016 at 08:36:37PM +0200, Heiner Kallweit wrote:
> > Am 24.10.2016 um 19:27 schrieb Mark Brown:
> 
> > > This doesn't apply against current code, please check and resend.
> 
> > The not yet reviewed part of my patch series from Oct 2nd,
> > namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
> > from RX FIFO" replaces the code in question.
> > There's more to fix like removing polling from the ISR.
> > If you prefer to apply Arnd's fix first I'd rebase the open part
> > of the patch series and resend it.
> 
> If there are dependencies you should mention them when you resend (in
> general you should always mention any unapplied or cross tree
> dependencies when sending things).

I think my patch (the version I sent) should ideally make it into
v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
autobuilder with the -Wmaybe-uninitialized warning added back, and
it's one of the actual bugs I found (though rather unlikely
to hit in practice).

Merging with Heiner's patches should be trivial, and I'm pretty
sure we want the patch either way. Not sure if we need a backport,
it was introduced earlier this year in commit 6319a68011b8
("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
I now found.

	Arnd

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

* Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error
  2016-10-24 20:37         ` Arnd Bergmann
@ 2016-10-25 19:13           ` Mark Brown
  2016-10-25 20:57             ` Arnd Bergmann
  0 siblings, 1 reply; 74+ messages in thread
From: Mark Brown @ 2016-10-25 19:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiner Kallweit, Linus Torvalds, linux-kernel, Heiner Kallweit,
	Nobuteru Hayashi, linux-spi

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Mon, Oct 24, 2016 at 10:37:53PM +0200, Arnd Bergmann wrote:

> I think my patch (the version I sent) should ideally make it into
> v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
> autobuilder with the -Wmaybe-uninitialized warning added back, and
> it's one of the actual bugs I found (though rather unlikely
> to hit in practice).

> Merging with Heiner's patches should be trivial, and I'm pretty
> sure we want the patch either way. Not sure if we need a backport,
> it was introduced earlier this year in commit 6319a68011b8
> ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
> I now found.

Sorry but I've lost track of which patches are being talked about here.
If there's stuff for v4.9 can you send me a version that applies on
Linus' tree and I'll merge that up into what's applied for -next?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error
  2016-10-25 19:13           ` Mark Brown
@ 2016-10-25 20:57             ` Arnd Bergmann
  0 siblings, 0 replies; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-25 20:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: Heiner Kallweit, Linus Torvalds, linux-kernel, Heiner Kallweit,
	Nobuteru Hayashi, linux-spi

On Tuesday, October 25, 2016 8:13:09 PM CEST Mark Brown wrote:
> 
> Not enough information to check signature validity.	Show Details
> On Mon, Oct 24, 2016 at 10:37:53PM +0200, Arnd Bergmann wrote:
> 
> > I think my patch (the version I sent) should ideally make it into
> > v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
> > autobuilder with the -Wmaybe-uninitialized warning added back, and
> > it's one of the actual bugs I found (though rather unlikely
> > to hit in practice).
> 
> > Merging with Heiner's patches should be trivial, and I'm pretty
> > sure we want the patch either way. Not sure if we need a backport,
> > it was introduced earlier this year in commit 6319a68011b8
> > ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
> > I now found.
> 
> Sorry but I've lost track of which patches are being talked about here.
> If there's stuff for v4.9 can you send me a version that applies on
> Linus' tree and I'll merge that up into what's applied for -next?
> 

Done.

	Arnd

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

* Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  2016-10-17 22:13 ` [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap Arnd Bergmann
@ 2016-10-26  6:49   ` Kalle Valo
  2016-10-26  9:57     ` Arnd Bergmann
  2016-10-27 15:05   ` [19/28] " Kalle Valo
  1 sibling, 1 reply; 74+ messages in thread
From: Kalle Valo @ 2016-10-26  6:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arend van Spriel, Linus Torvalds, linux-kernel, Hante Meuleman,
	Franky Lin, Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
	Rafał Miłecki, linux-wireless, brcm80211-dev-list.pdl,
	netdev

Arnd Bergmann <arnd@arndb.de> writes:

> A bugfix added a sanity check around the assignment and use of the
> 'is_11d' variable, which looks correct to me, but as the function is
> rather complex already, this confuses the compiler to the point where
> it can no longer figure out if the variable is always initialized
> correctly:
>
> brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This adds an initialization for the newly introduced case in which
> the variable should not really be used, in order to make the warning
> go away.
>
> Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> Cc: Hante Meuleman <hante.meuleman@broadcom.com>
> Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Via which tree are you planning to submit this? Should I take it?

-- 
Kalle Valo

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

* Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  2016-10-26  6:49   ` Kalle Valo
@ 2016-10-26  9:57     ` Arnd Bergmann
  2016-10-26 11:11       ` Kalle Valo
  0 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-26  9:57 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend van Spriel, Linus Torvalds, linux-kernel, Hante Meuleman,
	Franky Lin, Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
	Rafał Miłecki, linux-wireless, brcm80211-dev-list.pdl,
	netdev

On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> 
> > A bugfix added a sanity check around the assignment and use of the
> > 'is_11d' variable, which looks correct to me, but as the function is
> > rather complex already, this confuses the compiler to the point where
> > it can no longer figure out if the variable is always initialized
> > correctly:
> >
> > brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > This adds an initialization for the newly introduced case in which
> > the variable should not really be used, in order to make the warning
> > go away.
> >
> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> > Cc: Hante Meuleman <hante.meuleman@broadcom.com>
> > Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> > Cc: Kalle Valo <kvalo@codeaurora.org>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Via which tree are you planning to submit this? Should I take it?

I'd prefer if you can take it and forward it along with your other
bugfixes. I'll try to take care of the ones that nobody else
picked up.

	Arnd

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

* Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree
  2016-10-17 22:13 ` [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error Arnd Bergmann
  2016-10-24 17:27   ` Mark Brown
@ 2016-10-26 10:15   ` Mark Brown
  2016-10-26 18:11     ` Merge problem: " Heiner Kallweit
  1 sibling, 1 reply; 74+ messages in thread
From: Mark Brown @ 2016-10-26 10:15 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mark Brown, stable, Mark Brown, Linus Torvalds, linux-kernel,
	Heiner Kallweit, Nobuteru Hayashi, linux-spi

The patch

   spi: fsl-espi: avoid processing uninitalized data on error

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 5c0ba57744b1422d528f19430dd66d6803cea86f Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 25 Oct 2016 22:57:10 +0200
Subject: [PATCH] spi: fsl-espi: avoid processing uninitalized data on error

When we get a spurious interrupt in fsl_espi_irq, we end up
processing four uninitalized bytes of data, as shown in this
warning message:

   drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
   drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds another check so we skip the data in this case.

Fixes: 6319a68011b8 ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/spi/spi-fsl-espi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7451585a080e..2c175b9495f7 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
 
 		mspi->len -= rx_nr_bytes;
 
-		if (mspi->rx)
+		if (rx_nr_bytes && mspi->rx)
 			mspi->get_rx(rx_data, mspi);
 	}
 
-- 
2.8.1

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

* Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  2016-10-26  9:57     ` Arnd Bergmann
@ 2016-10-26 11:11       ` Kalle Valo
  0 siblings, 0 replies; 74+ messages in thread
From: Kalle Valo @ 2016-10-26 11:11 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arend van Spriel, Linus Torvalds, linux-kernel, Hante Meuleman,
	Franky Lin, Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
	Rafał Miłecki, linux-wireless, brcm80211-dev-list.pdl,
	netdev

Arnd Bergmann <arnd@arndb.de> writes:

> On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
>> Arnd Bergmann <arnd@arndb.de> writes:
>> 
>> > A bugfix added a sanity check around the assignment and use of the
>> > 'is_11d' variable, which looks correct to me, but as the function is
>> > rather complex already, this confuses the compiler to the point where
>> > it can no longer figure out if the variable is always initialized
>> > correctly:
>> >
>> > brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
>> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> >
>> > This adds an initialization for the newly introduced case in which
>> > the variable should not really be used, in order to make the warning
>> > go away.
>> >
>> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
>> > Cc: Hante Meuleman <hante.meuleman@broadcom.com>
>> > Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
>> > Cc: Kalle Valo <kvalo@codeaurora.org>
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> 
>> Via which tree are you planning to submit this? Should I take it?
>
> I'd prefer if you can take it and forward it along with your other
> bugfixes. I'll try to take care of the ones that nobody else
> picked up.

Ok, I'll take it. I'm planning to push this to 4.9.

-- 
Kalle Valo

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

* Re: [f2fs-dev] [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON
  2016-10-17 22:05 ` [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON Arnd Bergmann
@ 2016-10-26 14:05   ` Chao Yu
  2016-10-26 14:57     ` Arnd Bergmann
  0 siblings, 1 reply; 74+ messages in thread
From: Chao Yu @ 2016-10-26 14:05 UTC (permalink / raw)
  To: Arnd Bergmann, Jaegeuk Kim, Chao Yu
  Cc: linux-kernel, linux-f2fs-devel, Weichao Guo, Linus Torvalds

Hi Arnd,

On 2016/10/18 6:05, Arnd Bergmann wrote:
> gcc is unsure about the use of last_ofs_in_node, which might happen
> without a prior initialization:
> 
> fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
> fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>    if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {

In each round of dnode block traverse, we will init 'prealloc' and then update
'prealloc' and 'last_ofs_in_node' together in below lines of f2fs_map_blocks:
			if (flag == F2FS_GET_BLOCK_PRE_AIO) {
				if (blkaddr == NULL_ADDR) {
					prealloc++;
					last_ofs_in_node = dn.ofs_in_node;
				}
			}

Then in below codes, it is safe to use 'last_ofs_in_node' since we will check
'prealloc' firstly, so if 'prealloc' is non-zero, 'last_ofs_in_node' must be valid.
	if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {

So I think we should not add WARN_ON there.

Thanks,

> 
> I'm not sure about it either, so to shut up the warning I initialize
> it to a known invalid -1u and later check for this, so we get a
> runtime warning if we ever hit the uninitialized case.
> 
> It would be much better to reorganize the code in some form that
> made it obvious to both the compiler and the reader that this
> variable use it ok.
> 
> Since I only see the warning with gcc-4.9 but not any later version,
> it's possible that the compiler is actually smarter than I am here
> and has learned to see the code as correct, in which case this
> patch could just be disregarded. It would certainly be helpful
> to get an opinion from the maintainers on the matter.
> 
> Fixes: 46008c6d4232 ("f2fs: support in batch multi blocks preallocation")
> Cc: Chao Yu <yuchao0@huawei.com>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/f2fs/data.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 9ae194f..1b17de2 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -696,6 +696,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  		goto out;
>  	}
>  
> +	/*
> +	 * FIXME: without this, we get "warning: ‘last_ofs_in_node’ may be
> +	 * used uninitialized". It's not clear whether that can actually
> +	 * happen, so there is now a WARN_ON() checking for this.
> +	 */
> +	last_ofs_in_node = -1u;
>  next_dnode:
>  	if (create)
>  		f2fs_lock_op(sbi);
> @@ -796,6 +802,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  		allocated = dn.node_changed;
>  
>  		map->m_len += dn.ofs_in_node - ofs_in_node;
> +		WARN_ON(last_ofs_in_node == -1u);
>  		if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
>  			err = -ENOSPC;
>  			goto sync_out;
> 

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

* Re: [f2fs-dev] [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON
  2016-10-26 14:05   ` [f2fs-dev] " Chao Yu
@ 2016-10-26 14:57     ` Arnd Bergmann
  2016-10-27 11:41       ` Chao Yu
  0 siblings, 1 reply; 74+ messages in thread
From: Arnd Bergmann @ 2016-10-26 14:57 UTC (permalink / raw)
  To: Chao Yu
  Cc: Jaegeuk Kim, Chao Yu, linux-kernel, linux-f2fs-devel,
	Weichao Guo, Linus Torvalds

On Wednesday, October 26, 2016 10:05:00 PM CEST Chao Yu wrote:
> On 2016/10/18 6:05, Arnd Bergmann wrote:
> > gcc is unsure about the use of last_ofs_in_node, which might happen
> > without a prior initialization:
> > 
> > fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
> > fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> >    if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
> 
> In each round of dnode block traverse, we will init 'prealloc' and then update
> 'prealloc' and 'last_ofs_in_node' together in below lines of f2fs_map_blocks:
>                         if (flag == F2FS_GET_BLOCK_PRE_AIO) {
>                                 if (blkaddr == NULL_ADDR) {
>                                         prealloc++;
>                                         last_ofs_in_node = dn.ofs_in_node;
>                                 }
>                         }
> 
> Then in below codes, it is safe to use 'last_ofs_in_node' since we will check
> 'prealloc' firstly, so if 'prealloc' is non-zero, 'last_ofs_in_node' must be valid.
>         if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
> 
> So I think we should not add WARN_ON there.

Ok, that make sense. Thanks for taking a closer look!

Should we just set last_ofs_in_node to the same value as ofs_in_node
before the loop?

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9ae194f..14db4b7 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -716,7 +716,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 	}
 
 	prealloc = 0;
-	ofs_in_node = dn.ofs_in_node;
+	last_ofs_in_node = ofs_in_node = dn.ofs_in_node;
 	end_offset = ADDRS_PER_PAGE(dn.node_page, inode);
 
 next_block:

	Arnd

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

* Merge problem: Re: Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree
  2016-10-26 10:15   ` Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree Mark Brown
@ 2016-10-26 18:11     ` Heiner Kallweit
  2016-10-26 21:59       ` Mark Brown
  0 siblings, 1 reply; 74+ messages in thread
From: Heiner Kallweit @ 2016-10-26 18:11 UTC (permalink / raw)
  To: Mark Brown, Arnd Bergmann
  Cc: Linus Torvalds, linux-kernel, Nobuteru Hayashi, linux-spi

Am 26.10.2016 um 12:15 schrieb Mark Brown:
> The patch
> 
>    spi: fsl-espi: avoid processing uninitalized data on error
> 
> has been applied to the spi tree at
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 
> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.  
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark
> 
>>From 5c0ba57744b1422d528f19430dd66d6803cea86f Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 25 Oct 2016 22:57:10 +0200
> Subject: [PATCH] spi: fsl-espi: avoid processing uninitalized data on error
> 
> When we get a spurious interrupt in fsl_espi_irq, we end up
> processing four uninitalized bytes of data, as shown in this
> warning message:
> 
>    drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
>    drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This adds another check so we skip the data in this case.
> 
> Fixes: 6319a68011b8 ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  drivers/spi/spi-fsl-espi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
> index 7451585a080e..2c175b9495f7 100644
> --- a/drivers/spi/spi-fsl-espi.c
> +++ b/drivers/spi/spi-fsl-espi.c
> @@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
>  
>  		mspi->len -= rx_nr_bytes;
>  
> -		if (mspi->rx)
> +		if (rx_nr_bytes && mspi->rx)
>  			mspi->get_rx(rx_data, mspi);
>  	}
>  
> 
There seems to be a merge problem. Before the relevant code was:
(changed in recent commit "spi: fsl-espi: fix handling of word
sizes other than 8 bit")

if (mspi->rx) {
	*(u32 *)mspi->rx = rx_data;
	mspi->rx += 4;
}

Now it's:

if (rx_nr_bytes && mspi->rx) {
	mspi->get_rx(rx_data, mspi);
	mspi->rx += 4;
}

Instead it should be:

if (rx_nr_bytes && mspi->rx) {
	*(u32 *)mspi->rx = rx_data;
	mspi->rx += 4;
}

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

* Re: Merge problem: Re: Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree
  2016-10-26 18:11     ` Merge problem: " Heiner Kallweit
@ 2016-10-26 21:59       ` Mark Brown
  0 siblings, 0 replies; 74+ messages in thread
From: Mark Brown @ 2016-10-26 21:59 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Arnd Bergmann, Linus Torvalds, linux-kernel, Nobuteru Hayashi, linux-spi

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

On Wed, Oct 26, 2016 at 08:11:28PM +0200, Heiner Kallweit wrote:

> Instead it should be:
> 
> if (rx_nr_bytes && mspi->rx) {
> 	*(u32 *)mspi->rx = rx_data;
> 	mspi->rx += 4;
> }

Please send a patch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [f2fs-dev] [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON
  2016-10-26 14:57     ` Arnd Bergmann
@ 2016-10-27 11:41       ` Chao Yu
  0 siblings, 0 replies; 74+ messages in thread
From: Chao Yu @ 2016-10-27 11:41 UTC (permalink / raw)
  To: Arnd Bergmann, Chao Yu
  Cc: Jaegeuk Kim, linux-kernel, linux-f2fs-devel, Weichao Guo, Linus Torvalds

On 2016/10/26 22:57, Arnd Bergmann wrote:
> On Wednesday, October 26, 2016 10:05:00 PM CEST Chao Yu wrote:
>> On 2016/10/18 6:05, Arnd Bergmann wrote:
>>> gcc is unsure about the use of last_ofs_in_node, which might happen
>>> without a prior initialization:
>>>
>>> fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
>>> fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>    if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
>>
>> In each round of dnode block traverse, we will init 'prealloc' and then update
>> 'prealloc' and 'last_ofs_in_node' together in below lines of f2fs_map_blocks:
>>                         if (flag == F2FS_GET_BLOCK_PRE_AIO) {
>>                                 if (blkaddr == NULL_ADDR) {
>>                                         prealloc++;
>>                                         last_ofs_in_node = dn.ofs_in_node;
>>                                 }
>>                         }
>>
>> Then in below codes, it is safe to use 'last_ofs_in_node' since we will check
>> 'prealloc' firstly, so if 'prealloc' is non-zero, 'last_ofs_in_node' must be valid.
>>         if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
>>
>> So I think we should not add WARN_ON there.
> 
> Ok, that make sense. Thanks for taking a closer look!
> 
> Should we just set last_ofs_in_node to the same value as ofs_in_node
> before the loop?

I think it's OK as it can remove warning compiler reports. :)

Thanks,

> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 9ae194f..14db4b7 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -716,7 +716,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  	}
>  
>  	prealloc = 0;
> -	ofs_in_node = dn.ofs_in_node;
> +	last_ofs_in_node = ofs_in_node = dn.ofs_in_node;
>  	end_offset = ADDRS_PER_PAGE(dn.node_page, inode);
>  
>  next_block:
> 
> 	Arnd
> 
> .
> 

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

* Re: [19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  2016-10-17 22:13 ` [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap Arnd Bergmann
  2016-10-26  6:49   ` Kalle Valo
@ 2016-10-27 15:05   ` Kalle Valo
  1 sibling, 0 replies; 74+ messages in thread
From: Kalle Valo @ 2016-10-27 15:05 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arend van Spriel, Linus Torvalds, linux-kernel, Arnd Bergmann,
	Hante Meuleman, Franky Lin, Pieter-Paul Giesberts,
	Franky (Zhenhui) Lin, Rafał Miłecki, linux-wireless,
	brcm80211-dev-list.pdl, netdev

Arnd Bergmann <arnd@arndb.de> wrote:
> A bugfix added a sanity check around the assignment and use of the
> 'is_11d' variable, which looks correct to me, but as the function is
> rather complex already, this confuses the compiler to the point where
> it can no longer figure out if the variable is always initialized
> correctly:
> 
> brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This adds an initialization for the newly introduced case in which
> the variable should not really be used, in order to make the warning
> go away.
> 
> Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> Cc: Hante Meuleman <hante.meuleman@broadcom.com>
> Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied to wireless-drivers.git, thanks.

d3532ea6ce4e brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

-- 
https://patchwork.kernel.org/patch/9380763/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

end of thread, other threads:[~2016-10-27 15:07 UTC | newest]

Thread overview: 74+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
2016-10-17 22:05 ` [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning Arnd Bergmann
2016-10-18 15:23   ` Pablo Neira Ayuso
2016-10-17 22:05 ` [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode Arnd Bergmann
2016-10-18  5:19   ` Boris Brezillon
     [not found]     ` <1476785552.24626.4.camel@mtkswgap22>
2016-10-18 19:45       ` Boris Brezillon
2016-10-17 22:05 ` [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning Arnd Bergmann
2016-10-18  6:47   ` Haggai Eran
2016-10-18 10:18     ` Arnd Bergmann
2016-10-18 10:32       ` Haggai Eran
2016-10-17 22:05 ` [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON Arnd Bergmann
2016-10-26 14:05   ` [f2fs-dev] " Chao Yu
2016-10-26 14:57     ` Arnd Bergmann
2016-10-27 11:41       ` Chao Yu
2016-10-17 22:05 ` [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning Arnd Bergmann
2016-10-18  5:15   ` Christoph Hellwig
2016-10-18  9:30     ` Jan Kara
2016-10-17 22:05 ` [PATCH 06/28] NFSv4.1: work around " Arnd Bergmann
2016-10-17 22:08 ` [PATCH 07/28] ceph: avoid false positive maybe-uninitialized warning Arnd Bergmann
2016-10-18  2:07   ` Yan, Zheng
2016-10-17 22:08 ` [PATCH 08/28] staging: lustre: restore initialization of return code Arnd Bergmann
     [not found]   ` <CY4PR11MB1751050A7C7AF840E94DDE60CBD00@CY4PR11MB1751.namprd11.prod.outlook.com>
2016-10-17 22:29     ` [lustre-devel] " Arnd Bergmann
2016-10-17 22:37       ` Linus Torvalds
2016-10-17 23:00         ` Arnd Bergmann
2016-10-17 22:42   ` [PATCH 08/28 v2] " Arnd Bergmann
2016-10-17 22:08 ` [PATCH 09/28] staging: lustre: remove broken dead code in cfs_cpt_table_create_pattern Arnd Bergmann
2016-10-17 22:10 ` [PATCH 10/28] UBI: fix uninitialized access of vid_hdr pointer Arnd Bergmann
2016-10-18  5:17   ` Boris Brezillon
2016-10-17 22:10 ` [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized Arnd Bergmann
2016-10-18  9:57   ` Ilya Dryomov
2016-10-18 10:04     ` Arnd Bergmann
2016-10-17 22:12 ` [PATCH 12/28] [media] rc: print correct variable for z8f0811 Arnd Bergmann
2016-10-17 22:13 ` [PATCH 13/28] [media] dib0700: fix uninitialized data on 'repeat' event Arnd Bergmann
2016-10-17 22:13 ` [PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable Arnd Bergmann
2016-10-23 21:25   ` Jonathan Cameron
2016-10-17 22:13 ` [PATCH 15/28] crypto: aesni: avoid -Wmaybe-uninitialized warning Arnd Bergmann
2016-10-17 22:13 ` [PATCH 16/28] pcmcia: fix return value of soc_pcmcia_regulator_set Arnd Bergmann
2016-10-18  9:42   ` Russell King - ARM Linux
2016-10-17 22:13 ` [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error Arnd Bergmann
2016-10-24 17:27   ` Mark Brown
2016-10-24 18:36     ` Heiner Kallweit
2016-10-24 18:45       ` Mark Brown
2016-10-24 20:37         ` Arnd Bergmann
2016-10-25 19:13           ` Mark Brown
2016-10-25 20:57             ` Arnd Bergmann
2016-10-26 10:15   ` Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree Mark Brown
2016-10-26 18:11     ` Merge problem: " Heiner Kallweit
2016-10-26 21:59       ` Mark Brown
2016-10-17 22:13 ` [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank Arnd Bergmann
2016-10-17 23:47   ` Mario Kleiner
2016-10-18  7:46     ` Daniel Vetter
2016-10-17 22:13 ` [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap Arnd Bergmann
2016-10-26  6:49   ` Kalle Valo
2016-10-26  9:57     ` Arnd Bergmann
2016-10-26 11:11       ` Kalle Valo
2016-10-27 15:05   ` [19/28] " Kalle Valo
2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
2016-10-18 18:21   ` David Miller
2016-10-17 22:16 ` [PATCH 21/28] net/hyperv: avoid " Arnd Bergmann
2016-10-18 18:21   ` David Miller
2016-10-17 22:16 ` [PATCH 22/28] x86: apm: avoid uninitialized data Arnd Bergmann
2016-10-18 13:05   ` Jiri Kosina
2016-10-18 21:35   ` Luis R. Rodriguez
2016-10-17 22:16 ` [PATCH 23/28] x86: mark target address as output in 'insb' asm Arnd Bergmann
2016-10-17 22:16 ` [PATCH 24/28] x86: math-emu: possible uninitialized variable use Arnd Bergmann
2016-10-17 22:16 ` [PATCH 25/28] s390: pci: don't print uninitialized data for debugging Arnd Bergmann
2016-10-18  6:48   ` Martin Schwidefsky
2016-10-18  8:53     ` Sebastian Ott
2016-10-17 22:16 ` [PATCH 26/28] nios2: fix timer initcall return value Arnd Bergmann
2016-10-24  0:54   ` Ley Foon Tan
2016-10-17 22:16 ` [PATCH 27/28] rocker: fix maybe-uninitialized warning Arnd Bergmann
2016-10-18 18:21   ` David Miller
2016-10-17 22:19 ` [PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning Arnd Bergmann
2016-10-18  5:08 ` [PATCH 00/28] Reenable maybe-uninitialized warnings Christoph Hellwig

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).