* [PATCH 00/10] Remove uninitialized_var() macro
@ 2020-06-03 23:31 Kees Cook
2020-06-03 23:31 ` [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage Kees Cook
` (11 more replies)
0 siblings, 12 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:31 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings
(e.g. "unused variable"). If the compiler thinks it is uninitialized,
either simply initialize the variable or make compiler changes.
As recommended[2] by[3] Linus[4], remove the macro.
Most of the 300 uses don't cause any warnings on gcc 9.3.0, so they're in
a single treewide commit in this series. A few others needed to actually
get cleaned up, and I broke those out into individual patches.
-Kees
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Kees Cook (10):
x86/mm/numa: Remove uninitialized_var() usage
drbd: Remove uninitialized_var() usage
b43: Remove uninitialized_var() usage
rtlwifi: rtl8192cu: Remove uninitialized_var() usage
ide: Remove uninitialized_var() usage
clk: st: Remove uninitialized_var() usage
spi: davinci: Remove uninitialized_var() usage
checkpatch: Remove awareness of uninitialized_var() macro
treewide: Remove uninitialized_var() usage
compiler: Remove uninitialized_var() macro
arch/arm/mach-sa1100/assabet.c | 2 +-
arch/arm/mm/alignment.c | 2 +-
arch/ia64/kernel/process.c | 2 +-
arch/ia64/mm/discontig.c | 2 +-
arch/ia64/mm/tlb.c | 2 +-
arch/mips/lib/dump_tlb.c | 2 +-
arch/mips/mm/init.c | 2 +-
arch/mips/mm/tlb-r4k.c | 6 +++---
arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
arch/powerpc/kvm/book3s_pr.c | 2 +-
arch/powerpc/kvm/powerpc.c | 2 +-
arch/powerpc/platforms/52xx/mpc52xx_pic.c | 2 +-
arch/s390/kernel/smp.c | 2 +-
arch/x86/kernel/quirks.c | 10 +++++-----
arch/x86/kvm/mmu/mmu.c | 2 +-
arch/x86/kvm/mmu/paging_tmpl.h | 2 +-
arch/x86/kvm/x86.c | 2 +-
arch/x86/mm/numa.c | 18 +++++++++---------
block/blk-merge.c | 2 +-
drivers/acpi/acpi_pad.c | 2 +-
drivers/ata/libata-scsi.c | 2 +-
drivers/atm/zatm.c | 2 +-
drivers/block/drbd/drbd_nl.c | 6 +++---
drivers/block/drbd/drbd_state.c | 2 +-
drivers/block/rbd.c | 2 +-
drivers/clk/clk-gate.c | 2 +-
drivers/clk/spear/clk-vco-pll.c | 2 +-
drivers/clk/st/clkgen-fsyn.c | 1 -
drivers/crypto/nx/nx-842-powernv.c | 2 +-
drivers/firewire/ohci.c | 14 +++++++-------
drivers/gpu/drm/bridge/sil-sii8620.c | 2 +-
drivers/gpu/drm/drm_edid.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 6 +++---
drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
drivers/gpu/drm/i915/intel_uncore.c | 2 +-
.../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 4 ++--
drivers/i2c/busses/i2c-rk3x.c | 2 +-
drivers/ide/ide-acpi.c | 2 +-
drivers/ide/ide-atapi.c | 2 +-
drivers/ide/ide-io-std.c | 4 ++--
drivers/ide/ide-io.c | 8 ++++----
drivers/ide/ide-sysfs.c | 2 +-
drivers/ide/ide-taskfile.c | 1 -
drivers/ide/umc8672.c | 2 +-
drivers/idle/intel_idle.c | 2 +-
drivers/infiniband/core/uverbs_cmd.c | 4 ++--
drivers/infiniband/hw/cxgb4/cm.c | 2 +-
drivers/infiniband/hw/cxgb4/cq.c | 2 +-
drivers/infiniband/hw/mlx4/qp.c | 6 +++---
drivers/infiniband/hw/mlx5/cq.c | 6 +++---
drivers/infiniband/hw/mlx5/devx.c | 2 +-
drivers/infiniband/hw/mlx5/qp.c | 2 +-
drivers/infiniband/hw/mthca/mthca_qp.c | 10 +++++-----
drivers/infiniband/sw/siw/siw_qp_rx.c | 2 +-
drivers/input/serio/serio_raw.c | 2 +-
drivers/input/touchscreen/sur40.c | 2 +-
drivers/iommu/intel-iommu.c | 2 +-
drivers/md/dm-io.c | 2 +-
drivers/md/dm-ioctl.c | 2 +-
drivers/md/dm-snap-persistent.c | 2 +-
drivers/md/dm-table.c | 2 +-
drivers/md/dm-writecache.c | 2 +-
drivers/md/raid5.c | 2 +-
drivers/media/dvb-frontends/rtl2832.c | 2 +-
drivers/media/tuners/qt1010.c | 4 ++--
drivers/media/usb/gspca/vicam.c | 2 +-
drivers/media/usb/uvc/uvc_video.c | 8 ++++----
drivers/memstick/host/jmb38x_ms.c | 2 +-
drivers/memstick/host/tifm_ms.c | 2 +-
drivers/mmc/host/sdhci.c | 2 +-
drivers/mtd/nand/raw/nand_ecc.c | 2 +-
drivers/mtd/nand/raw/s3c2410.c | 2 +-
drivers/mtd/parsers/afs.c | 4 ++--
drivers/mtd/ubi/eba.c | 2 +-
drivers/net/can/janz-ican3.c | 2 +-
drivers/net/ethernet/broadcom/bnx2.c | 4 ++--
.../ethernet/mellanox/mlx5/core/pagealloc.c | 4 ++--
drivers/net/ethernet/neterion/s2io.c | 2 +-
drivers/net/ethernet/qlogic/qla3xxx.c | 2 +-
drivers/net/ethernet/sun/cassini.c | 2 +-
drivers/net/ethernet/sun/niu.c | 6 +++---
drivers/net/wan/z85230.c | 2 +-
drivers/net/wireless/ath/ath10k/core.c | 2 +-
drivers/net/wireless/ath/ath6kl/init.c | 2 +-
drivers/net/wireless/ath/ath9k/init.c | 2 +-
drivers/net/wireless/broadcom/b43/debugfs.c | 2 +-
drivers/net/wireless/broadcom/b43/dma.c | 2 +-
drivers/net/wireless/broadcom/b43/lo.c | 2 +-
drivers/net/wireless/broadcom/b43/phy_n.c | 12 ++++++++----
drivers/net/wireless/broadcom/b43/xmit.c | 12 ++++++------
.../net/wireless/broadcom/b43legacy/debugfs.c | 2 +-
drivers/net/wireless/broadcom/b43legacy/main.c | 2 +-
drivers/net/wireless/intel/iwlegacy/3945.c | 2 +-
drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
.../wireless/realtek/rtlwifi/rtl8192cu/hw.c | 8 ++++----
drivers/pci/pcie/aer.c | 2 +-
drivers/platform/x86/hdaps.c | 4 ++--
drivers/scsi/dc395x.c | 2 +-
drivers/scsi/pm8001/pm8001_hwi.c | 2 +-
drivers/scsi/pm8001/pm80xx_hwi.c | 2 +-
drivers/spi/spi-davinci.c | 1 -
drivers/ssb/driver_chipcommon.c | 4 ++--
drivers/tty/cyclades.c | 2 +-
drivers/tty/isicom.c | 2 +-
drivers/usb/musb/cppi_dma.c | 2 +-
drivers/usb/storage/sddr55.c | 4 ++--
drivers/vhost/net.c | 6 +++---
drivers/video/fbdev/matrox/matroxfb_maven.c | 6 +++---
drivers/video/fbdev/pm3fb.c | 6 +++---
drivers/video/fbdev/riva/riva_hw.c | 3 +--
drivers/virtio/virtio_ring.c | 6 +++---
fs/afs/dir.c | 2 +-
fs/afs/security.c | 2 +-
fs/dlm/netlink.c | 2 +-
fs/erofs/data.c | 4 ++--
fs/erofs/zdata.c | 2 +-
fs/f2fs/data.c | 2 +-
fs/fat/dir.c | 2 +-
fs/fuse/control.c | 4 ++--
fs/fuse/cuse.c | 2 +-
fs/fuse/file.c | 2 +-
fs/gfs2/aops.c | 2 +-
fs/gfs2/bmap.c | 2 +-
fs/gfs2/lops.c | 2 +-
fs/hfsplus/unicode.c | 2 +-
fs/isofs/namei.c | 4 ++--
fs/jffs2/erase.c | 2 +-
fs/nfsd/nfsctl.c | 2 +-
fs/ocfs2/alloc.c | 4 ++--
fs/ocfs2/dir.c | 14 +++++++-------
fs/ocfs2/extent_map.c | 4 ++--
fs/ocfs2/namei.c | 2 +-
fs/ocfs2/refcounttree.c | 2 +-
fs/ocfs2/xattr.c | 2 +-
fs/omfs/file.c | 2 +-
fs/overlayfs/copy_up.c | 4 ++--
fs/ubifs/commit.c | 6 +++---
fs/ubifs/dir.c | 2 +-
fs/ubifs/file.c | 4 ++--
fs/ubifs/journal.c | 4 ++--
fs/ubifs/lpt.c | 2 +-
fs/ubifs/tnc.c | 6 +++---
fs/ubifs/tnc_misc.c | 4 ++--
fs/udf/balloc.c | 2 +-
fs/xfs/xfs_bmap_util.c | 2 +-
include/linux/compiler-clang.h | 2 --
include/linux/compiler-gcc.h | 6 ------
include/linux/page-flags-layout.h | 2 +-
include/net/flow_offload.h | 2 +-
kernel/async.c | 4 ++--
kernel/audit.c | 2 +-
kernel/debug/kdb/kdb_io.c | 2 +-
kernel/dma/debug.c | 2 +-
kernel/events/core.c | 2 +-
kernel/events/uprobes.c | 2 +-
kernel/exit.c | 2 +-
kernel/futex.c | 14 +++++++-------
kernel/locking/lockdep.c | 16 ++++++++--------
kernel/trace/ring_buffer.c | 2 +-
lib/radix-tree.c | 2 +-
lib/test_lockup.c | 2 +-
mm/frontswap.c | 2 +-
mm/ksm.c | 2 +-
mm/memcontrol.c | 2 +-
mm/memory.c | 2 +-
mm/mempolicy.c | 4 ++--
mm/page_alloc.c | 2 +-
mm/percpu.c | 2 +-
mm/slub.c | 4 ++--
mm/swap.c | 4 ++--
net/dccp/options.c | 2 +-
net/ipv4/netfilter/nf_socket_ipv4.c | 6 +++---
net/ipv6/ip6_flowlabel.c | 2 +-
net/ipv6/netfilter/nf_socket_ipv6.c | 2 +-
net/netfilter/nf_conntrack_ftp.c | 2 +-
net/netfilter/nfnetlink_log.c | 2 +-
net/netfilter/nfnetlink_queue.c | 4 ++--
net/sched/cls_flow.c | 2 +-
net/sched/sch_cake.c | 2 +-
net/sched/sch_cbq.c | 2 +-
net/sched/sch_fq_codel.c | 2 +-
net/sched/sch_fq_pie.c | 2 +-
net/sched/sch_hfsc.c | 2 +-
net/sched/sch_htb.c | 2 +-
net/sched/sch_sfq.c | 2 +-
net/sunrpc/svcsock.c | 4 ++--
net/sunrpc/xprtsock.c | 10 +++++-----
net/tls/tls_sw.c | 2 +-
scripts/checkpatch.pl | 18 ++++++------------
sound/core/control_compat.c | 2 +-
sound/isa/sb/sb16_csp.c | 2 +-
sound/usb/endpoint.c | 2 +-
tools/include/linux/compiler.h | 2 --
tools/virtio/linux/kernel.h | 2 --
195 files changed, 310 insertions(+), 328 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
@ 2020-06-03 23:31 ` Kees Cook
2020-06-04 7:58 ` Thomas Gleixner
2020-06-03 23:31 ` [PATCH 02/10] drbd: " Kees Cook
` (10 subsequent siblings)
11 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:31 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings (e.g.
"unused variable"). If the compiler thinks it is uninitialized, either
simply initialize the variable or make compiler changes. As a precursor
to removing[2] this[3] macro[4], refactor code to avoid its need.
The original reason for its use here was to work around the #ifdef
being the only place the variable was used. This is better expressed
using IS_ENABLED() and a new code block where the variable can be used
unconditionally.
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/x86/mm/numa.c | 18 +++++++++---------
include/linux/page-flags-layout.h | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 59ba008504dc..38eeb15f3b07 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -541,7 +541,6 @@ static void __init numa_clear_kernel_node_hotplug(void)
static int __init numa_register_memblks(struct numa_meminfo *mi)
{
- unsigned long uninitialized_var(pfn_align);
int i, nid;
/* Account for nodes with cpus and no memory */
@@ -569,15 +568,16 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
* If sections array is gonna be used for pfn -> nid mapping, check
* whether its granularity is fine enough.
*/
-#ifdef NODE_NOT_IN_PAGE_FLAGS
- pfn_align = node_map_pfn_alignment();
- if (pfn_align && pfn_align < PAGES_PER_SECTION) {
- printk(KERN_WARNING "Node alignment %LuMB < min %LuMB, rejecting NUMA config\n",
- PFN_PHYS(pfn_align) >> 20,
- PFN_PHYS(PAGES_PER_SECTION) >> 20);
- return -EINVAL;
+ if (IS_ENABLED(NODE_NOT_IN_PAGE_FLAGS)) {
+ unsigned long pfn_align = node_map_pfn_alignment();
+
+ if (pfn_align && pfn_align < PAGES_PER_SECTION) {
+ pr_warn("Node alignment %LuMB < min %LuMB, rejecting NUMA config\n",
+ PFN_PHYS(pfn_align) >> 20,
+ PFN_PHYS(PAGES_PER_SECTION) >> 20);
+ return -EINVAL;
+ }
}
-#endif
if (!numa_meminfo_cover_memory(mi))
return -EINVAL;
diff --git a/include/linux/page-flags-layout.h b/include/linux/page-flags-layout.h
index 71283739ffd2..1a4cdec2bd29 100644
--- a/include/linux/page-flags-layout.h
+++ b/include/linux/page-flags-layout.h
@@ -100,7 +100,7 @@
* there. This includes the case where there is no node, so it is implicit.
*/
#if !(NODES_WIDTH > 0 || NODES_SHIFT == 0)
-#define NODE_NOT_IN_PAGE_FLAGS
+#define NODE_NOT_IN_PAGE_FLAGS 1
#endif
#if defined(CONFIG_NUMA_BALANCING) && LAST_CPUPID_WIDTH == 0
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 02/10] drbd: Remove uninitialized_var() usage
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
2020-06-03 23:31 ` [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage Kees Cook
@ 2020-06-03 23:31 ` Kees Cook
2020-06-04 19:56 ` Nick Desaulniers
2020-06-03 23:31 ` [PATCH 03/10] b43: " Kees Cook
` (9 subsequent siblings)
11 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:31 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings (e.g.
"unused variable"). If the compiler thinks it is uninitialized, either
simply initialize the variable or make compiler changes. As a precursor
to removing[2] this[3] macro[4], just initialize this variable to NULL.
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/block/drbd/drbd_state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
index eeaa3b49b264..0067d328f0b5 100644
--- a/drivers/block/drbd/drbd_state.c
+++ b/drivers/block/drbd/drbd_state.c
@@ -1604,7 +1604,7 @@ static void broadcast_state_change(struct drbd_state_change *state_change)
unsigned int n_device, n_connection, n_peer_device, n_peer_devices;
void (*last_func)(struct sk_buff *, unsigned int, void *,
enum drbd_notification_type) = NULL;
- void *uninitialized_var(last_arg);
+ void *last_arg = NULL;
#define HAS_CHANGED(state) ((state)[OLD] != (state)[NEW])
#define FINAL_STATE_CHANGE(type) \
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 03/10] b43: Remove uninitialized_var() usage
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
2020-06-03 23:31 ` [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage Kees Cook
2020-06-03 23:31 ` [PATCH 02/10] drbd: " Kees Cook
@ 2020-06-03 23:31 ` Kees Cook
2020-06-04 20:08 ` Nick Desaulniers
2020-06-05 9:20 ` Kalle Valo
2020-06-03 23:31 ` [PATCH 04/10] rtlwifi: rtl8192cu: " Kees Cook
` (8 subsequent siblings)
11 siblings, 2 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:31 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings (e.g.
"unused variable"). If the compiler thinks it is uninitialized, either
simply initialize the variable or make compiler changes. As a precursor
to removing[2] this[3] macro[4], just initialize this variable to NULL,
and make the (unreachable!) code do a conditional test.
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/wireless/broadcom/b43/phy_n.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index d3c001fa8eb4..88cdcea10d61 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -4222,7 +4222,7 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
u32 rfpwr_offset;
u8 pga_gain, pad_gain;
int i;
- const s16 *uninitialized_var(rf_pwr_offset_table);
+ const s16 *rf_pwr_offset_table = NULL;
table = b43_nphy_get_tx_gain_table(dev);
if (!table)
@@ -4256,9 +4256,13 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
pga_gain = (table[i] >> 24) & 0xf;
pad_gain = (table[i] >> 19) & 0x1f;
if (b43_current_band(dev->wl) == NL80211_BAND_2GHZ)
- rfpwr_offset = rf_pwr_offset_table[pad_gain];
+ rfpwr_offset = rf_pwr_offset_table
+ ? rf_pwr_offset_table[pad_gain]
+ : 0;
else
- rfpwr_offset = rf_pwr_offset_table[pga_gain];
+ rfpwr_offset = rf_pwr_offset_table
+ ? rf_pwr_offset_table[pga_gain]
+ : 0;
} else {
pga_gain = (table[i] >> 24) & 0xF;
if (b43_current_band(dev->wl) == NL80211_BAND_2GHZ)
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 04/10] rtlwifi: rtl8192cu: Remove uninitialized_var() usage
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
` (2 preceding siblings ...)
2020-06-03 23:31 ` [PATCH 03/10] b43: " Kees Cook
@ 2020-06-03 23:31 ` Kees Cook
2020-06-04 20:16 ` Nick Desaulniers
2020-06-05 9:18 ` Kalle Valo
2020-06-03 23:31 ` [PATCH 05/10] ide: " Kees Cook
` (7 subsequent siblings)
11 siblings, 2 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:31 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings (e.g.
"unused variable"). If the compiler thinks it is uninitialized, either
simply initialize the variable or make compiler changes. As a precursor
to removing[2] this[3] macro[4], just initialize this variable to NULL,
and avoid sending garbage by returning.
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
index f070f25bb735..5b071b70bc08 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
@@ -592,7 +592,7 @@ static void _rtl92cu_init_chipn_one_out_ep_priority(struct ieee80211_hw *hw,
bool wmm_enable,
u8 queue_sel)
{
- u16 uninitialized_var(value);
+ u16 value;
switch (queue_sel) {
case TX_SELE_HQ:
@@ -606,7 +606,7 @@ static void _rtl92cu_init_chipn_one_out_ep_priority(struct ieee80211_hw *hw,
break;
default:
WARN_ON(1); /* Shall not reach here! */
- break;
+ return;
}
_rtl92c_init_chipn_reg_priority(hw, value, value, value, value,
value, value);
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 05/10] ide: Remove uninitialized_var() usage
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
` (3 preceding siblings ...)
2020-06-03 23:31 ` [PATCH 04/10] rtlwifi: rtl8192cu: " Kees Cook
@ 2020-06-03 23:31 ` Kees Cook
2020-06-04 19:29 ` Nick Desaulniers
2020-06-03 23:31 ` [PATCH 06/10] clk: st: " Kees Cook
` (6 subsequent siblings)
11 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:31 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings (e.g.
"unused variable"). If the compiler thinks it is uninitialized, either
simply initialize the variable or make compiler changes. As a precursor
to removing[2] this[3] macro[4], just remove this variable since it was
actually unused:
drivers/ide/ide-taskfile.c:232:34: warning: unused variable 'flags' [-Wunused-variable]
unsigned long uninitialized_var(flags);
^
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/ide/ide-taskfile.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index aab6a10435b6..a26f85ab58a9 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -229,7 +229,6 @@ void ide_pio_bytes(ide_drive_t *drive, struct ide_cmd *cmd,
ide_hwif_t *hwif = drive->hwif;
struct scatterlist *sg = hwif->sg_table;
struct scatterlist *cursg = cmd->cursg;
- unsigned long uninitialized_var(flags);
struct page *page;
unsigned int offset;
u8 *buf;
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 06/10] clk: st: Remove uninitialized_var() usage
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
` (4 preceding siblings ...)
2020-06-03 23:31 ` [PATCH 05/10] ide: " Kees Cook
@ 2020-06-03 23:31 ` Kees Cook
2020-06-04 4:38 ` Stephen Boyd
2020-06-03 23:32 ` [PATCH 07/10] spi: davinci: " Kees Cook
` (5 subsequent siblings)
11 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:31 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings (e.g.
"unused variable"). If the compiler thinks it is uninitialized, either
simply initialize the variable or make compiler changes. As a precursor
to removing[2] this[3] macro[4], just remove this variable since it was
actually unused:
drivers/clk/st/clkgen-fsyn.c: In function ‘quadfs_set_rate’:
drivers/clk/st/clkgen-fsyn.c:793:6: warning: unused variable ‘i’ [-Wunused-variable]
793 | int i;
| ^
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/clk/st/clkgen-fsyn.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/clk/st/clkgen-fsyn.c b/drivers/clk/st/clkgen-fsyn.c
index a156bd0c6af7..f1adc858b590 100644
--- a/drivers/clk/st/clkgen-fsyn.c
+++ b/drivers/clk/st/clkgen-fsyn.c
@@ -790,7 +790,6 @@ static int quadfs_set_rate(struct clk_hw *hw, unsigned long rate,
struct st_clk_quadfs_fsynth *fs = to_quadfs_fsynth(hw);
struct stm_fs params;
long hwrate;
- int uninitialized_var(i);
if (!rate || !parent_rate)
return -EINVAL;
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 07/10] spi: davinci: Remove uninitialized_var() usage
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
` (5 preceding siblings ...)
2020-06-03 23:31 ` [PATCH 06/10] clk: st: " Kees Cook
@ 2020-06-03 23:32 ` Kees Cook
2020-06-04 19:40 ` Nick Desaulniers
2020-06-03 23:32 ` [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro Kees Cook
` (4 subsequent siblings)
11 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:32 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings (e.g.
"unused variable"). If the compiler thinks it is uninitialized, either
simply initialize the variable or make compiler changes. As a precursor
to removing[2] this[3] macro[4], just remove this variable since it was
actually unused:
drivers/spi/spi-davinci.c: In function ‘davinci_spi_bufs’:
drivers/spi/spi-davinci.c:579:11: warning: unused variable ‘rx_buf_count’ [-Wunused-variable]
579 | unsigned rx_buf_count;
| ^~~~~~~~~~~~
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/spi/spi-davinci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index f71c497393a6..f50c0c79cbdf 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -576,7 +576,6 @@ static int davinci_spi_bufs(struct spi_device *spi, struct spi_transfer *t)
u32 errors = 0;
struct davinci_spi_config *spicfg;
struct davinci_spi_platform_data *pdata;
- unsigned uninitialized_var(rx_buf_count);
dspi = spi_master_get_devdata(spi->master);
pdata = &dspi->pdata;
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
` (6 preceding siblings ...)
2020-06-03 23:32 ` [PATCH 07/10] spi: davinci: " Kees Cook
@ 2020-06-03 23:32 ` Kees Cook
2020-06-04 0:02 ` Joe Perches
2020-06-03 23:32 ` [PATCH 10/10] compiler: Remove " Kees Cook
` (3 subsequent siblings)
11 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:32 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings
(e.g. "unused variable"). If the compiler thinks it is uninitialized,
either simply initialize the variable or make compiler changes.
In preparation for removing[2] the[3] macro[4], effectively revert
commit 16b7f3c89907 ("checkpatch: avoid warning about uninitialized_var()")
and remove all remaining mentions of uninitialized_var().
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
scripts/checkpatch.pl | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b83be177edf0..e9f8146600d0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -838,7 +838,6 @@ our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)};
our $declaration_macros = qr{(?x:
(?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(|
(?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(|
- (?:$Storage\s+)?${Type}\s+uninitialized_var\s*\(|
(?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\(
)};
@@ -4075,7 +4074,7 @@ sub process {
}
# check for function declarations without arguments like "int foo()"
- if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
+ if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
if (ERROR("FUNCTION_WITHOUT_ARGS",
"Bad function definition - $1() should probably be $1(void)\n" . $herecurr) &&
$fix) {
@@ -6271,8 +6270,7 @@ sub process {
if (defined $cond) {
substr($s, 0, length($cond), '');
}
- if ($s =~ /^\s*;/ &&
- $function_name ne 'uninitialized_var')
+ if ($s =~ /^\s*;/)
{
WARN("AVOID_EXTERNS",
"externs should be avoided in .c files\n" . $herecurr);
@@ -6291,17 +6289,13 @@ sub process {
}
# check for function declarations that have arguments without identifier names
-# while avoiding uninitialized_var(x)
if (defined $stat &&
- $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:($Ident)|\(\s*\*\s*$Ident\s*\))\s*\(\s*([^{]+)\s*\)\s*;/s &&
- (!defined($1) ||
- (defined($1) && $1 ne "uninitialized_var")) &&
- $2 ne "void") {
- my $args = trim($2);
+ $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*\(\s*([^{]+)\s*\)\s*;/s &&
+ $1 ne "void") {
+ my $args = trim($1);
while ($args =~ m/\s*($Type\s*(?:$Ident|\(\s*\*\s*$Ident?\s*\)\s*$balanced_parens)?)/g) {
my $arg = trim($1);
- if ($arg =~ /^$Type$/ &&
- $arg !~ /enum\s+$Ident$/) {
+ if ($arg =~ /^$Type$/ && $arg !~ /enum\s+$Ident$/) {
WARN("FUNCTION_ARGUMENTS",
"function definition argument '$arg' should also have an identifier name\n" . $herecurr);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 10/10] compiler: Remove uninitialized_var() macro
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
` (7 preceding siblings ...)
2020-06-03 23:32 ` [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro Kees Cook
@ 2020-06-03 23:32 ` Kees Cook
2020-06-04 0:00 ` Bart Van Assche
2020-06-04 0:50 ` Miguel Ojeda
2020-06-04 1:23 ` [PATCH 00/10] " Sedat Dilek
` (2 subsequent siblings)
11 siblings, 2 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-03 23:32 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Using uninitialized_var() is dangerous as it papers over real bugs[1]
(or can in the future), and suppresses unrelated compiler warnings
(e.g. "unused variable"). If the compiler thinks it is uninitialized,
either simply initialize the variable or make compiler changes.
As recommended[2] by[3] Linus[4], remove the macro.
[1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
[2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
[3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
[4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/compiler-clang.h | 2 --
include/linux/compiler-gcc.h | 6 ------
tools/include/linux/compiler.h | 2 --
tools/virtio/linux/kernel.h | 2 --
4 files changed, 12 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 790c0c6b8552..019f444b500b 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -5,8 +5,6 @@
/* Compiler specific definitions for Clang compiler */
-#define uninitialized_var(x) x = *(&(x))
-
/* same as gcc, this was present in clang-2.6 so we can assume it works
* with any version that can compile the kernel
*/
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index d7ee4c6bad48..ac3e29ae32de 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -58,12 +58,6 @@
(typeof(ptr)) (__ptr + (off)); \
})
-/*
- * A trick to suppress uninitialized variable warning without generating any
- * code
- */
-#define uninitialized_var(x) x = x
-
#ifdef CONFIG_RETPOLINE
#define __noretpoline __attribute__((__indirect_branch__("keep")))
#endif
diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h
index 180f7714a5f1..29cbb73f2ae0 100644
--- a/tools/include/linux/compiler.h
+++ b/tools/include/linux/compiler.h
@@ -108,8 +108,6 @@
# define noinline
#endif
-#define uninitialized_var(x) x = *(&(x))
-
#include <linux/types.h>
/*
diff --git a/tools/virtio/linux/kernel.h b/tools/virtio/linux/kernel.h
index 6683b4a70b05..1e14ab967c11 100644
--- a/tools/virtio/linux/kernel.h
+++ b/tools/virtio/linux/kernel.h
@@ -109,8 +109,6 @@ static inline void free_page(unsigned long addr)
const typeof( ((type *)0)->member ) *__mptr = (ptr); \
(type *)( (char *)__mptr - offsetof(type,member) );})
-#define uninitialized_var(x) x = x
-
# ifndef likely
# define likely(x) (__builtin_expect(!!(x), 1))
# endif
--
2.25.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 10/10] compiler: Remove uninitialized_var() macro
2020-06-03 23:32 ` [PATCH 10/10] compiler: Remove " Kees Cook
@ 2020-06-04 0:00 ` Bart Van Assche
2020-06-04 0:50 ` Miguel Ojeda
1 sibling, 0 replies; 53+ messages in thread
From: Bart Van Assche @ 2020-06-04 0:00 UTC (permalink / raw)
To: Kees Cook, linux-kernel
Cc: Linus Torvalds, Miguel Ojeda, Alexander Potapenko, Joe Perches,
Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev, netdev,
linux-wireless, linux-ide, linux-clk, linux-spi, linux-mm,
clang-built-linux
On 2020-06-03 16:32, Kees Cook wrote:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
Thank you for having done this work!
Reviewed-by: Bart van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro
2020-06-03 23:32 ` [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro Kees Cook
@ 2020-06-04 0:02 ` Joe Perches
2020-06-04 1:40 ` Kees Cook
0 siblings, 1 reply; 53+ messages in thread
From: Joe Perches @ 2020-06-04 0:02 UTC (permalink / raw)
To: Kees Cook, linux-kernel
Cc: Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev, netdev,
linux-wireless, linux-ide, linux-clk, linux-spi, linux-mm,
clang-built-linux
On Wed, 2020-06-03 at 16:32 -0700, Kees Cook wrote:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
>
> In preparation for removing[2] the[3] macro[4], effectively revert
> commit 16b7f3c89907 ("checkpatch: avoid warning about uninitialized_var()")
> and remove all remaining mentions of uninitialized_var().
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
nack. see below.
I'd prefer a simple revert, but it shouldn't
be done here.
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -4075,7 +4074,7 @@ sub process {
> }
>
> # check for function declarations without arguments like "int foo()"
> - if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
> + if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
This isn't right because $Type includes a possible trailing *
where there isn't a space between $Type and $Ident
e.g.: int *bar(void);
Other than that, fine by me...
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 10/10] compiler: Remove uninitialized_var() macro
2020-06-03 23:32 ` [PATCH 10/10] compiler: Remove " Kees Cook
2020-06-04 0:00 ` Bart Van Assche
@ 2020-06-04 0:50 ` Miguel Ojeda
1 sibling, 0 replies; 53+ messages in thread
From: Miguel Ojeda @ 2020-06-04 0:50 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Alexander Potapenko, Joe Perches,
Andy Whitcroft, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi, Linux-MM,
clang-built-linux
Hi Kees,
On Thu, Jun 4, 2020 at 1:32 AM Kees Cook <keescook@chromium.org> wrote:
>
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
>
> As recommended[2] by[3] Linus[4], remove the macro.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
+1, one less trick split between `compiler*` files.
Reviewed-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cheers,
Miguel
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 00/10] Remove uninitialized_var() macro
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
` (8 preceding siblings ...)
2020-06-03 23:32 ` [PATCH 10/10] compiler: Remove " Kees Cook
@ 2020-06-04 1:23 ` Sedat Dilek
2020-06-04 1:44 ` Kees Cook
2020-06-04 3:33 ` Nathan Chancellor
[not found] ` <20200603233203.1695403-10-keescook@chromium.org>
11 siblings, 1 reply; 53+ messages in thread
From: Sedat Dilek @ 2020-06-04 1:23 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, Clang-Built-Linux ML
On Thu, Jun 4, 2020 at 1:32 AM Kees Cook <keescook@chromium.org> wrote:
>
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
>
> As recommended[2] by[3] Linus[4], remove the macro.
>
> Most of the 300 uses don't cause any warnings on gcc 9.3.0, so they're in
> a single treewide commit in this series. A few others needed to actually
> get cleaned up, and I broke those out into individual patches.
>
> -Kees
>
Hi Kees,
what is the base for your patchset?
I would like to test on top of Linux v5.7.
Can you place the series in your Git tree for easy fetching, please?
Thanks.
Regards,
- Sedat -
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Kees Cook (10):
> x86/mm/numa: Remove uninitialized_var() usage
> drbd: Remove uninitialized_var() usage
> b43: Remove uninitialized_var() usage
> rtlwifi: rtl8192cu: Remove uninitialized_var() usage
> ide: Remove uninitialized_var() usage
> clk: st: Remove uninitialized_var() usage
> spi: davinci: Remove uninitialized_var() usage
> checkpatch: Remove awareness of uninitialized_var() macro
> treewide: Remove uninitialized_var() usage
> compiler: Remove uninitialized_var() macro
>
> arch/arm/mach-sa1100/assabet.c | 2 +-
> arch/arm/mm/alignment.c | 2 +-
> arch/ia64/kernel/process.c | 2 +-
> arch/ia64/mm/discontig.c | 2 +-
> arch/ia64/mm/tlb.c | 2 +-
> arch/mips/lib/dump_tlb.c | 2 +-
> arch/mips/mm/init.c | 2 +-
> arch/mips/mm/tlb-r4k.c | 6 +++---
> arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
> arch/powerpc/kvm/book3s_pr.c | 2 +-
> arch/powerpc/kvm/powerpc.c | 2 +-
> arch/powerpc/platforms/52xx/mpc52xx_pic.c | 2 +-
> arch/s390/kernel/smp.c | 2 +-
> arch/x86/kernel/quirks.c | 10 +++++-----
> arch/x86/kvm/mmu/mmu.c | 2 +-
> arch/x86/kvm/mmu/paging_tmpl.h | 2 +-
> arch/x86/kvm/x86.c | 2 +-
> arch/x86/mm/numa.c | 18 +++++++++---------
> block/blk-merge.c | 2 +-
> drivers/acpi/acpi_pad.c | 2 +-
> drivers/ata/libata-scsi.c | 2 +-
> drivers/atm/zatm.c | 2 +-
> drivers/block/drbd/drbd_nl.c | 6 +++---
> drivers/block/drbd/drbd_state.c | 2 +-
> drivers/block/rbd.c | 2 +-
> drivers/clk/clk-gate.c | 2 +-
> drivers/clk/spear/clk-vco-pll.c | 2 +-
> drivers/clk/st/clkgen-fsyn.c | 1 -
> drivers/crypto/nx/nx-842-powernv.c | 2 +-
> drivers/firewire/ohci.c | 14 +++++++-------
> drivers/gpu/drm/bridge/sil-sii8620.c | 2 +-
> drivers/gpu/drm/drm_edid.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 6 +++---
> drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
> drivers/gpu/drm/i915/intel_uncore.c | 2 +-
> .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 4 ++--
> drivers/i2c/busses/i2c-rk3x.c | 2 +-
> drivers/ide/ide-acpi.c | 2 +-
> drivers/ide/ide-atapi.c | 2 +-
> drivers/ide/ide-io-std.c | 4 ++--
> drivers/ide/ide-io.c | 8 ++++----
> drivers/ide/ide-sysfs.c | 2 +-
> drivers/ide/ide-taskfile.c | 1 -
> drivers/ide/umc8672.c | 2 +-
> drivers/idle/intel_idle.c | 2 +-
> drivers/infiniband/core/uverbs_cmd.c | 4 ++--
> drivers/infiniband/hw/cxgb4/cm.c | 2 +-
> drivers/infiniband/hw/cxgb4/cq.c | 2 +-
> drivers/infiniband/hw/mlx4/qp.c | 6 +++---
> drivers/infiniband/hw/mlx5/cq.c | 6 +++---
> drivers/infiniband/hw/mlx5/devx.c | 2 +-
> drivers/infiniband/hw/mlx5/qp.c | 2 +-
> drivers/infiniband/hw/mthca/mthca_qp.c | 10 +++++-----
> drivers/infiniband/sw/siw/siw_qp_rx.c | 2 +-
> drivers/input/serio/serio_raw.c | 2 +-
> drivers/input/touchscreen/sur40.c | 2 +-
> drivers/iommu/intel-iommu.c | 2 +-
> drivers/md/dm-io.c | 2 +-
> drivers/md/dm-ioctl.c | 2 +-
> drivers/md/dm-snap-persistent.c | 2 +-
> drivers/md/dm-table.c | 2 +-
> drivers/md/dm-writecache.c | 2 +-
> drivers/md/raid5.c | 2 +-
> drivers/media/dvb-frontends/rtl2832.c | 2 +-
> drivers/media/tuners/qt1010.c | 4 ++--
> drivers/media/usb/gspca/vicam.c | 2 +-
> drivers/media/usb/uvc/uvc_video.c | 8 ++++----
> drivers/memstick/host/jmb38x_ms.c | 2 +-
> drivers/memstick/host/tifm_ms.c | 2 +-
> drivers/mmc/host/sdhci.c | 2 +-
> drivers/mtd/nand/raw/nand_ecc.c | 2 +-
> drivers/mtd/nand/raw/s3c2410.c | 2 +-
> drivers/mtd/parsers/afs.c | 4 ++--
> drivers/mtd/ubi/eba.c | 2 +-
> drivers/net/can/janz-ican3.c | 2 +-
> drivers/net/ethernet/broadcom/bnx2.c | 4 ++--
> .../ethernet/mellanox/mlx5/core/pagealloc.c | 4 ++--
> drivers/net/ethernet/neterion/s2io.c | 2 +-
> drivers/net/ethernet/qlogic/qla3xxx.c | 2 +-
> drivers/net/ethernet/sun/cassini.c | 2 +-
> drivers/net/ethernet/sun/niu.c | 6 +++---
> drivers/net/wan/z85230.c | 2 +-
> drivers/net/wireless/ath/ath10k/core.c | 2 +-
> drivers/net/wireless/ath/ath6kl/init.c | 2 +-
> drivers/net/wireless/ath/ath9k/init.c | 2 +-
> drivers/net/wireless/broadcom/b43/debugfs.c | 2 +-
> drivers/net/wireless/broadcom/b43/dma.c | 2 +-
> drivers/net/wireless/broadcom/b43/lo.c | 2 +-
> drivers/net/wireless/broadcom/b43/phy_n.c | 12 ++++++++----
> drivers/net/wireless/broadcom/b43/xmit.c | 12 ++++++------
> .../net/wireless/broadcom/b43legacy/debugfs.c | 2 +-
> drivers/net/wireless/broadcom/b43legacy/main.c | 2 +-
> drivers/net/wireless/intel/iwlegacy/3945.c | 2 +-
> drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
> .../wireless/realtek/rtlwifi/rtl8192cu/hw.c | 8 ++++----
> drivers/pci/pcie/aer.c | 2 +-
> drivers/platform/x86/hdaps.c | 4 ++--
> drivers/scsi/dc395x.c | 2 +-
> drivers/scsi/pm8001/pm8001_hwi.c | 2 +-
> drivers/scsi/pm8001/pm80xx_hwi.c | 2 +-
> drivers/spi/spi-davinci.c | 1 -
> drivers/ssb/driver_chipcommon.c | 4 ++--
> drivers/tty/cyclades.c | 2 +-
> drivers/tty/isicom.c | 2 +-
> drivers/usb/musb/cppi_dma.c | 2 +-
> drivers/usb/storage/sddr55.c | 4 ++--
> drivers/vhost/net.c | 6 +++---
> drivers/video/fbdev/matrox/matroxfb_maven.c | 6 +++---
> drivers/video/fbdev/pm3fb.c | 6 +++---
> drivers/video/fbdev/riva/riva_hw.c | 3 +--
> drivers/virtio/virtio_ring.c | 6 +++---
> fs/afs/dir.c | 2 +-
> fs/afs/security.c | 2 +-
> fs/dlm/netlink.c | 2 +-
> fs/erofs/data.c | 4 ++--
> fs/erofs/zdata.c | 2 +-
> fs/f2fs/data.c | 2 +-
> fs/fat/dir.c | 2 +-
> fs/fuse/control.c | 4 ++--
> fs/fuse/cuse.c | 2 +-
> fs/fuse/file.c | 2 +-
> fs/gfs2/aops.c | 2 +-
> fs/gfs2/bmap.c | 2 +-
> fs/gfs2/lops.c | 2 +-
> fs/hfsplus/unicode.c | 2 +-
> fs/isofs/namei.c | 4 ++--
> fs/jffs2/erase.c | 2 +-
> fs/nfsd/nfsctl.c | 2 +-
> fs/ocfs2/alloc.c | 4 ++--
> fs/ocfs2/dir.c | 14 +++++++-------
> fs/ocfs2/extent_map.c | 4 ++--
> fs/ocfs2/namei.c | 2 +-
> fs/ocfs2/refcounttree.c | 2 +-
> fs/ocfs2/xattr.c | 2 +-
> fs/omfs/file.c | 2 +-
> fs/overlayfs/copy_up.c | 4 ++--
> fs/ubifs/commit.c | 6 +++---
> fs/ubifs/dir.c | 2 +-
> fs/ubifs/file.c | 4 ++--
> fs/ubifs/journal.c | 4 ++--
> fs/ubifs/lpt.c | 2 +-
> fs/ubifs/tnc.c | 6 +++---
> fs/ubifs/tnc_misc.c | 4 ++--
> fs/udf/balloc.c | 2 +-
> fs/xfs/xfs_bmap_util.c | 2 +-
> include/linux/compiler-clang.h | 2 --
> include/linux/compiler-gcc.h | 6 ------
> include/linux/page-flags-layout.h | 2 +-
> include/net/flow_offload.h | 2 +-
> kernel/async.c | 4 ++--
> kernel/audit.c | 2 +-
> kernel/debug/kdb/kdb_io.c | 2 +-
> kernel/dma/debug.c | 2 +-
> kernel/events/core.c | 2 +-
> kernel/events/uprobes.c | 2 +-
> kernel/exit.c | 2 +-
> kernel/futex.c | 14 +++++++-------
> kernel/locking/lockdep.c | 16 ++++++++--------
> kernel/trace/ring_buffer.c | 2 +-
> lib/radix-tree.c | 2 +-
> lib/test_lockup.c | 2 +-
> mm/frontswap.c | 2 +-
> mm/ksm.c | 2 +-
> mm/memcontrol.c | 2 +-
> mm/memory.c | 2 +-
> mm/mempolicy.c | 4 ++--
> mm/page_alloc.c | 2 +-
> mm/percpu.c | 2 +-
> mm/slub.c | 4 ++--
> mm/swap.c | 4 ++--
> net/dccp/options.c | 2 +-
> net/ipv4/netfilter/nf_socket_ipv4.c | 6 +++---
> net/ipv6/ip6_flowlabel.c | 2 +-
> net/ipv6/netfilter/nf_socket_ipv6.c | 2 +-
> net/netfilter/nf_conntrack_ftp.c | 2 +-
> net/netfilter/nfnetlink_log.c | 2 +-
> net/netfilter/nfnetlink_queue.c | 4 ++--
> net/sched/cls_flow.c | 2 +-
> net/sched/sch_cake.c | 2 +-
> net/sched/sch_cbq.c | 2 +-
> net/sched/sch_fq_codel.c | 2 +-
> net/sched/sch_fq_pie.c | 2 +-
> net/sched/sch_hfsc.c | 2 +-
> net/sched/sch_htb.c | 2 +-
> net/sched/sch_sfq.c | 2 +-
> net/sunrpc/svcsock.c | 4 ++--
> net/sunrpc/xprtsock.c | 10 +++++-----
> net/tls/tls_sw.c | 2 +-
> scripts/checkpatch.pl | 18 ++++++------------
> sound/core/control_compat.c | 2 +-
> sound/isa/sb/sb16_csp.c | 2 +-
> sound/usb/endpoint.c | 2 +-
> tools/include/linux/compiler.h | 2 --
> tools/virtio/linux/kernel.h | 2 --
> 195 files changed, 310 insertions(+), 328 deletions(-)
>
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200603233203.1695403-1-keescook%40chromium.org.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro
2020-06-04 0:02 ` Joe Perches
@ 2020-06-04 1:40 ` Kees Cook
2020-06-04 1:47 ` Joe Perches
0 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-04 1:40 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev, netdev,
linux-wireless, linux-ide, linux-clk, linux-spi, linux-mm,
clang-built-linux
On Wed, Jun 03, 2020 at 05:02:29PM -0700, Joe Perches wrote:
> On Wed, 2020-06-03 at 16:32 -0700, Kees Cook wrote:
> > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > (or can in the future), and suppresses unrelated compiler warnings
> > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > either simply initialize the variable or make compiler changes.
> >
> > In preparation for removing[2] the[3] macro[4], effectively revert
> > commit 16b7f3c89907 ("checkpatch: avoid warning about uninitialized_var()")
> > and remove all remaining mentions of uninitialized_var().
> >
> > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> nack. see below.
>
> I'd prefer a simple revert, but it shouldn't
> be done here.
What do you mean? (I can't understand this and "fine by me" below?)
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -4075,7 +4074,7 @@ sub process {
> > }
> >
> > # check for function declarations without arguments like "int foo()"
> > - if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
> > + if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
>
> This isn't right because $Type includes a possible trailing *
> where there isn't a space between $Type and $Ident
Ah, hm, that was changed in the mentioned commit:
- if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
+ if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
>
> e.g.: int *bar(void);
>
> Other than that, fine by me...
Thanks for looking it over! I'll adjust it however you'd like. :)
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 00/10] Remove uninitialized_var() macro
2020-06-04 1:23 ` [PATCH 00/10] " Sedat Dilek
@ 2020-06-04 1:44 ` Kees Cook
2020-06-04 1:46 ` Sedat Dilek
0 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-04 1:44 UTC (permalink / raw)
To: Sedat Dilek
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, Clang-Built-Linux ML
On Thu, Jun 04, 2020 at 03:23:28AM +0200, Sedat Dilek wrote:
> what is the base for your patchset?
Hi! This was actually on Linus's latest tree (which is basically -next),
mostly because I figured this might be a bit of an RFC but if it was
clean enough, it might actually make the merge window (I can dream).
> I would like to test on top of Linux v5.7.
>
> Can you place the series in your Git tree for easy fetching, please?
Sure! https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git in
the kspp/uninit/v5.7/macro branch. There were three small differences.
I'm doing the "all my cross compilers allmodconfig" build run now, but
figured I'd push it for you now so you didn't have to wait.
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 00/10] Remove uninitialized_var() macro
2020-06-04 1:44 ` Kees Cook
@ 2020-06-04 1:46 ` Sedat Dilek
0 siblings, 0 replies; 53+ messages in thread
From: Sedat Dilek @ 2020-06-04 1:46 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, Clang-Built-Linux ML
On Thu, Jun 4, 2020 at 3:44 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Jun 04, 2020 at 03:23:28AM +0200, Sedat Dilek wrote:
> > what is the base for your patchset?
>
> Hi! This was actually on Linus's latest tree (which is basically -next),
> mostly because I figured this might be a bit of an RFC but if it was
> clean enough, it might actually make the merge window (I can dream).
>
> > I would like to test on top of Linux v5.7.
> >
> > Can you place the series in your Git tree for easy fetching, please?
>
> Sure! https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git in
> the kspp/uninit/v5.7/macro branch. There were three small differences.
> I'm doing the "all my cross compilers allmodconfig" build run now, but
> figured I'd push it for you now so you didn't have to wait.
>
Hi Kees!
Thanks :-).
Regards,
- Sedat -
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro
2020-06-04 1:40 ` Kees Cook
@ 2020-06-04 1:47 ` Joe Perches
2020-06-04 2:44 ` Kees Cook
0 siblings, 1 reply; 53+ messages in thread
From: Joe Perches @ 2020-06-04 1:47 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev, netdev,
linux-wireless, linux-ide, linux-clk, linux-spi, linux-mm,
clang-built-linux
On Wed, 2020-06-03 at 18:40 -0700, Kees Cook wrote:
> On Wed, Jun 03, 2020 at 05:02:29PM -0700, Joe Perches wrote:
> > On Wed, 2020-06-03 at 16:32 -0700, Kees Cook wrote:
> > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > (or can in the future), and suppresses unrelated compiler warnings
> > > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > > either simply initialize the variable or make compiler changes.
> > >
> > > In preparation for removing[2] the[3] macro[4], effectively revert
> > > commit 16b7f3c89907 ("checkpatch: avoid warning about uninitialized_var()")
> > > and remove all remaining mentions of uninitialized_var().
> > >
> > > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> >
> > nack. see below.
> >
> > I'd prefer a simple revert, but it shouldn't
> > be done here.
>
> What do you mean? (I can't understand this and "fine by me" below?)
I did write "other than that"...
I mean that the original commit fixed 2 issues,
one with the uninitialized_var addition, and
another with the missing void function declaration.
I think I found the missing void function bit because
the uninitialized_var use looked like a function so I
fixed both things at the same time.
If you change it, please just remove the bit that
checks for uninitialized_var.
Thanks, Joe
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -4075,7 +4074,7 @@ sub process {
> > > }
> > >
> > > # check for function declarations without arguments like "int foo()"
> > > - if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
> > > + if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
> >
> > This isn't right because $Type includes a possible trailing *
> > where there isn't a space between $Type and $Ident
>
> Ah, hm, that was changed in the mentioned commit:
>
> - if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
> + if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
>
> > e.g.: int *bar(void);
> >
> > Other than that, fine by me...
>
> Thanks for looking it over! I'll adjust it however you'd like. :)
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro
2020-06-04 1:47 ` Joe Perches
@ 2020-06-04 2:44 ` Kees Cook
2020-06-04 2:53 ` Sedat Dilek
0 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-04 2:44 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev, netdev,
linux-wireless, linux-ide, linux-clk, linux-spi, linux-mm,
clang-built-linux
On Wed, Jun 03, 2020 at 06:47:13PM -0700, Joe Perches wrote:
> On Wed, 2020-06-03 at 18:40 -0700, Kees Cook wrote:
> > On Wed, Jun 03, 2020 at 05:02:29PM -0700, Joe Perches wrote:
> > > On Wed, 2020-06-03 at 16:32 -0700, Kees Cook wrote:
> > > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > > (or can in the future), and suppresses unrelated compiler warnings
> > > > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > > > either simply initialize the variable or make compiler changes.
> > > >
> > > > In preparation for removing[2] the[3] macro[4], effectively revert
> > > > commit 16b7f3c89907 ("checkpatch: avoid warning about uninitialized_var()")
> > > > and remove all remaining mentions of uninitialized_var().
> > > >
> > > > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > > > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > > > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > > > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> > >
> > > nack. see below.
> > >
> > > I'd prefer a simple revert, but it shouldn't
> > > be done here.
> >
> > What do you mean? (I can't understand this and "fine by me" below?)
>
> I did write "other than that"...
>
> I mean that the original commit fixed 2 issues,
> one with the uninitialized_var addition, and
> another with the missing void function declaration.
>
> I think I found the missing void function bit because
> the uninitialized_var use looked like a function so I
> fixed both things at the same time.
>
> If you change it, please just remove the bit that
> checks for uninitialized_var.
Ah! Gotcha. Thanks; I will update it.
-Kees
>
> Thanks, Joe
>
> > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > []
> > > > @@ -4075,7 +4074,7 @@ sub process {
> > > > }
> > > >
> > > > # check for function declarations without arguments like "int foo()"
> > > > - if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
> > > > + if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
> > >
> > > This isn't right because $Type includes a possible trailing *
> > > where there isn't a space between $Type and $Ident
> >
> > Ah, hm, that was changed in the mentioned commit:
> >
> > - if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
> > + if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
> >
> > > e.g.: int *bar(void);
> > >
> > > Other than that, fine by me...
> >
> > Thanks for looking it over! I'll adjust it however you'd like. :)
> >
>
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro
2020-06-04 2:44 ` Kees Cook
@ 2020-06-04 2:53 ` Sedat Dilek
2020-06-04 3:46 ` Kees Cook
0 siblings, 1 reply; 53+ messages in thread
From: Sedat Dilek @ 2020-06-04 2:53 UTC (permalink / raw)
To: Kees Cook
Cc: Joe Perches, linux-kernel, Linus Torvalds, Miguel Ojeda,
Alexander Potapenko, Andy Whitcroft, x86, drbd-dev, linux-block,
b43-dev, netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, Clang-Built-Linux ML
Hi Kees,
can you push that change also to kees/linux.git#kspp/uninit/v5.7/macro ?
Thanks in advance.
Regards,
- Sedat -
[1] https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/uninit/v5.7/macro
On Thu, Jun 4, 2020 at 4:44 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, Jun 03, 2020 at 06:47:13PM -0700, Joe Perches wrote:
> > On Wed, 2020-06-03 at 18:40 -0700, Kees Cook wrote:
> > > On Wed, Jun 03, 2020 at 05:02:29PM -0700, Joe Perches wrote:
> > > > On Wed, 2020-06-03 at 16:32 -0700, Kees Cook wrote:
> > > > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > > > (or can in the future), and suppresses unrelated compiler warnings
> > > > > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > > > > either simply initialize the variable or make compiler changes.
> > > > >
> > > > > In preparation for removing[2] the[3] macro[4], effectively revert
> > > > > commit 16b7f3c89907 ("checkpatch: avoid warning about uninitialized_var()")
> > > > > and remove all remaining mentions of uninitialized_var().
> > > > >
> > > > > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > > > > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > > > > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > > > > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> > > >
> > > > nack. see below.
> > > >
> > > > I'd prefer a simple revert, but it shouldn't
> > > > be done here.
> > >
> > > What do you mean? (I can't understand this and "fine by me" below?)
> >
> > I did write "other than that"...
> >
> > I mean that the original commit fixed 2 issues,
> > one with the uninitialized_var addition, and
> > another with the missing void function declaration.
> >
> > I think I found the missing void function bit because
> > the uninitialized_var use looked like a function so I
> > fixed both things at the same time.
> >
> > If you change it, please just remove the bit that
> > checks for uninitialized_var.
>
> Ah! Gotcha. Thanks; I will update it.
>
> -Kees
>
> >
> > Thanks, Joe
> >
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > []
> > > > > @@ -4075,7 +4074,7 @@ sub process {
> > > > > }
> > > > >
> > > > > # check for function declarations without arguments like "int foo()"
> > > > > - if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
> > > > > + if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
> > > >
> > > > This isn't right because $Type includes a possible trailing *
> > > > where there isn't a space between $Type and $Ident
> > >
> > > Ah, hm, that was changed in the mentioned commit:
> > >
> > > - if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) {
> > > + if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) {
> > >
> > > > e.g.: int *bar(void);
> > > >
> > > > Other than that, fine by me...
> > >
> > > Thanks for looking it over! I'll adjust it however you'd like. :)
> > >
> >
>
> --
> Kees Cook
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/202006031944.9551FAA68E%40keescook.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/10] treewide: Remove uninitialized_var() usage
[not found] ` <20200603233203.1695403-10-keescook@chromium.org>
@ 2020-06-04 3:33 ` Nathan Chancellor
2020-06-04 4:02 ` Kees Cook
2020-06-04 10:45 ` Leon Romanovsky
` (2 subsequent siblings)
3 siblings, 1 reply; 53+ messages in thread
From: Nathan Chancellor @ 2020-06-04 3:33 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Wed, Jun 03, 2020 at 04:32:02PM -0700, Kees Cook wrote:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
>
> I preparation for removing[2] the[3] macro[4], remove all remaining
> needless uses with the following script:
>
> git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
> xargs perl -pi -e \
> 's/\buninitialized_var\(([^\)]+)\)/\1/g;
> s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
>
> drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
> pathological white-space.
>
> No outstanding warnings were found building allmodconfig with GCC 9.3.0
> for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
> alpha, and m68k.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
<snip>
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index a0f6813f4560..a71fa7204882 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -1829,7 +1829,7 @@ static int kvmppc_vcpu_run_pr(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
> {
> int ret;
> #ifdef CONFIG_ALTIVEC
> - unsigned long uninitialized_var(vrsave);
> + unsigned long vrsave;
> #endif
This variable is actually unused:
../arch/powerpc/kvm/book3s_pr.c:1832:16: warning: unused variable 'vrsave' [-Wunused-variable]
unsigned long vrsave;
^
1 warning generated.
It has been unused since commit 99dae3bad28d ("KVM: PPC: Load/save
FP/VMX/VSX state directly to/from vcpu struct").
$ git grep vrsave 99dae3bad28d8fdd32b7bfdd5e2ec7bb2d4d019d arch/powerpc/kvm/book3s_pr.c
99dae3bad28d8fdd32b7bfdd5e2ec7bb2d4d019d:arch/powerpc/kvm/book3s_pr.c: unsigned long uninitialized_var(vrsave);
I would nuke the whole '#ifdef' block.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 00/10] Remove uninitialized_var() macro
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
` (9 preceding siblings ...)
2020-06-04 1:23 ` [PATCH 00/10] " Sedat Dilek
@ 2020-06-04 3:33 ` Nathan Chancellor
2020-06-04 7:26 ` Sedat Dilek
[not found] ` <20200603233203.1695403-10-keescook@chromium.org>
11 siblings, 1 reply; 53+ messages in thread
From: Nathan Chancellor @ 2020-06-04 3:33 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Wed, Jun 03, 2020 at 04:31:53PM -0700, Kees Cook wrote:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
>
> As recommended[2] by[3] Linus[4], remove the macro.
>
> Most of the 300 uses don't cause any warnings on gcc 9.3.0, so they're in
> a single treewide commit in this series. A few others needed to actually
> get cleaned up, and I broke those out into individual patches.
>
> -Kees
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Kees Cook (10):
> x86/mm/numa: Remove uninitialized_var() usage
> drbd: Remove uninitialized_var() usage
> b43: Remove uninitialized_var() usage
> rtlwifi: rtl8192cu: Remove uninitialized_var() usage
> ide: Remove uninitialized_var() usage
> clk: st: Remove uninitialized_var() usage
> spi: davinci: Remove uninitialized_var() usage
> checkpatch: Remove awareness of uninitialized_var() macro
> treewide: Remove uninitialized_var() usage
> compiler: Remove uninitialized_var() macro
I applied all of these on top of cb8e59cc8720 and ran a variety of
builds with clang for arm32, arm64, mips, powerpc, s390, and x86_64 [1]
and only saw one warning pop up (which was about a variable being
unused, commented on patch 9 about it). No warnings about uninitialized
variables came up; clang's -Wuninitialized was not impacted by
78a5255ffb6a ("Stop the ad-hoc games with -Wno-maybe-initialized") so it
should have caught anything egregious.
[1]: https://github.com/nathanchance/llvm-kernel-testing
For the series, consider it:
Tested-by: Nathan Chancellor <natechancellor@gmail.com> [build]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro
2020-06-04 2:53 ` Sedat Dilek
@ 2020-06-04 3:46 ` Kees Cook
0 siblings, 0 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-04 3:46 UTC (permalink / raw)
To: Sedat Dilek
Cc: Joe Perches, linux-kernel, Linus Torvalds, Miguel Ojeda,
Alexander Potapenko, Andy Whitcroft, x86, drbd-dev, linux-block,
b43-dev, netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, Clang-Built-Linux ML
On Thu, Jun 04, 2020 at 04:53:34AM +0200, Sedat Dilek wrote:
> can you push that change also to kees/linux.git#kspp/uninit/v5.7/macro ?
Done! :)
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/10] treewide: Remove uninitialized_var() usage
2020-06-04 3:33 ` [PATCH 09/10] treewide: Remove uninitialized_var() usage Nathan Chancellor
@ 2020-06-04 4:02 ` Kees Cook
0 siblings, 0 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-04 4:02 UTC (permalink / raw)
To: Nathan Chancellor
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Wed, Jun 03, 2020 at 08:33:15PM -0700, Nathan Chancellor wrote:
> On Wed, Jun 03, 2020 at 04:32:02PM -0700, Kees Cook wrote:
> > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > (or can in the future), and suppresses unrelated compiler warnings
> > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > either simply initialize the variable or make compiler changes.
> >
> > I preparation for removing[2] the[3] macro[4], remove all remaining
> > needless uses with the following script:
> >
> > git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
> > xargs perl -pi -e \
> > 's/\buninitialized_var\(([^\)]+)\)/\1/g;
> > s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
> >
> > drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
> > pathological white-space.
> >
> > No outstanding warnings were found building allmodconfig with GCC 9.3.0
> > for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
> > alpha, and m68k.
> >
> > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> <snip>
>
> > diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> > index a0f6813f4560..a71fa7204882 100644
> > --- a/arch/powerpc/kvm/book3s_pr.c
> > +++ b/arch/powerpc/kvm/book3s_pr.c
> > @@ -1829,7 +1829,7 @@ static int kvmppc_vcpu_run_pr(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
> > {
> > int ret;
> > #ifdef CONFIG_ALTIVEC
> > - unsigned long uninitialized_var(vrsave);
> > + unsigned long vrsave;
> > #endif
>
> This variable is actually unused:
>
> ../arch/powerpc/kvm/book3s_pr.c:1832:16: warning: unused variable 'vrsave' [-Wunused-variable]
> unsigned long vrsave;
> ^
> 1 warning generated.
>
> It has been unused since commit 99dae3bad28d ("KVM: PPC: Load/save
> FP/VMX/VSX state directly to/from vcpu struct").
>
> $ git grep vrsave 99dae3bad28d8fdd32b7bfdd5e2ec7bb2d4d019d arch/powerpc/kvm/book3s_pr.c
> 99dae3bad28d8fdd32b7bfdd5e2ec7bb2d4d019d:arch/powerpc/kvm/book3s_pr.c: unsigned long uninitialized_var(vrsave);
>
> I would nuke the whole '#ifdef' block.
Ah, thanks! I wonder why I don't have CONFIG_ALTIVEC in any of my ppc
builds. Hmmm.
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 06/10] clk: st: Remove uninitialized_var() usage
2020-06-03 23:31 ` [PATCH 06/10] clk: st: " Kees Cook
@ 2020-06-04 4:38 ` Stephen Boyd
0 siblings, 0 replies; 53+ messages in thread
From: Stephen Boyd @ 2020-06-04 4:38 UTC (permalink / raw)
To: Kees Cook, linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Quoting Kees Cook (2020-06-03 16:31:59)
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just remove this variable since it was
> actually unused:
>
> drivers/clk/st/clkgen-fsyn.c: In function \u2018quadfs_set_rate\u2019:
> drivers/clk/st/clkgen-fsyn.c:793:6: warning: unused variable \u2018i\u2019 [-Wunused-variable]
> 793 | int i;
> | ^
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
Acked-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 00/10] Remove uninitialized_var() macro
2020-06-04 3:33 ` Nathan Chancellor
@ 2020-06-04 7:26 ` Sedat Dilek
2020-06-04 14:27 ` Kees Cook
0 siblings, 1 reply; 53+ messages in thread
From: Sedat Dilek @ 2020-06-04 7:26 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Kees Cook, linux-kernel, Linus Torvalds, Miguel Ojeda,
Alexander Potapenko, Joe Perches, Andy Whitcroft, x86, drbd-dev,
linux-block, b43-dev, netdev, linux-wireless, linux-ide,
linux-clk, linux-spi, linux-mm, Clang-Built-Linux ML
On Thu, Jun 4, 2020 at 5:33 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Wed, Jun 03, 2020 at 04:31:53PM -0700, Kees Cook wrote:
> > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > (or can in the future), and suppresses unrelated compiler warnings
> > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > either simply initialize the variable or make compiler changes.
> >
> > As recommended[2] by[3] Linus[4], remove the macro.
> >
> > Most of the 300 uses don't cause any warnings on gcc 9.3.0, so they're in
> > a single treewide commit in this series. A few others needed to actually
> > get cleaned up, and I broke those out into individual patches.
> >
> > -Kees
> >
> > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> >
> > Kees Cook (10):
> > x86/mm/numa: Remove uninitialized_var() usage
> > drbd: Remove uninitialized_var() usage
> > b43: Remove uninitialized_var() usage
> > rtlwifi: rtl8192cu: Remove uninitialized_var() usage
> > ide: Remove uninitialized_var() usage
> > clk: st: Remove uninitialized_var() usage
> > spi: davinci: Remove uninitialized_var() usage
> > checkpatch: Remove awareness of uninitialized_var() macro
> > treewide: Remove uninitialized_var() usage
> > compiler: Remove uninitialized_var() macro
>
> I applied all of these on top of cb8e59cc8720 and ran a variety of
> builds with clang for arm32, arm64, mips, powerpc, s390, and x86_64 [1]
> and only saw one warning pop up (which was about a variable being
> unused, commented on patch 9 about it). No warnings about uninitialized
> variables came up; clang's -Wuninitialized was not impacted by
> 78a5255ffb6a ("Stop the ad-hoc games with -Wno-maybe-initialized") so it
> should have caught anything egregious.
>
> [1]: https://github.com/nathanchance/llvm-kernel-testing
>
> For the series, consider it:
>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com> [build]
>
Hi Kees,
I tried with updated version (checkpatch) of your tree and see no
(new) warnings in my build-log.
Feel free to add my...
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Thanks.
Regards,
- Sedat -
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage
2020-06-03 23:31 ` [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage Kees Cook
@ 2020-06-04 7:58 ` Thomas Gleixner
2020-06-04 11:41 ` Miguel Ojeda
2020-06-04 14:34 ` Kees Cook
0 siblings, 2 replies; 53+ messages in thread
From: Thomas Gleixner @ 2020-06-04 7:58 UTC (permalink / raw)
To: Kees Cook, linux-kernel
Cc: Kees Cook, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Kees Cook <keescook@chromium.org> writes:
> -#ifdef NODE_NOT_IN_PAGE_FLAGS
> - pfn_align = node_map_pfn_alignment();
> - if (pfn_align && pfn_align < PAGES_PER_SECTION) {
> - printk(KERN_WARNING "Node alignment %LuMB < min %LuMB, rejecting NUMA config\n",
> - PFN_PHYS(pfn_align) >> 20,
> - PFN_PHYS(PAGES_PER_SECTION) >> 20);
> - return -EINVAL;
> + if (IS_ENABLED(NODE_NOT_IN_PAGE_FLAGS)) {
Hrm, clever ...
> + unsigned long pfn_align = node_map_pfn_alignment();
> +
> + if (pfn_align && pfn_align < PAGES_PER_SECTION) {
> + pr_warn("Node alignment %LuMB < min %LuMB, rejecting NUMA config\n",
> + PFN_PHYS(pfn_align) >> 20,
> + PFN_PHYS(PAGES_PER_SECTION) >> 20);
> + return -EINVAL;
> + }
> }
> -#endif
> if (!numa_meminfo_cover_memory(mi))
> return -EINVAL;
>
> diff --git a/include/linux/page-flags-layout.h b/include/linux/page-flags-layout.h
> index 71283739ffd2..1a4cdec2bd29 100644
> --- a/include/linux/page-flags-layout.h
> +++ b/include/linux/page-flags-layout.h
> @@ -100,7 +100,7 @@
> * there. This includes the case where there is no node, so it is implicit.
> */
> #if !(NODES_WIDTH > 0 || NODES_SHIFT == 0)
> -#define NODE_NOT_IN_PAGE_FLAGS
> +#define NODE_NOT_IN_PAGE_FLAGS 1
but if we ever lose the 1 then the above will silently compile the code
within the IS_ENABLED() section out.
Thanks,
tglx
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/10] treewide: Remove uninitialized_var() usage
[not found] ` <20200603233203.1695403-10-keescook@chromium.org>
2020-06-04 3:33 ` [PATCH 09/10] treewide: Remove uninitialized_var() usage Nathan Chancellor
@ 2020-06-04 10:45 ` Leon Romanovsky
2020-06-04 13:23 ` Jason Gunthorpe
2020-06-05 9:25 ` Kalle Valo
3 siblings, 0 replies; 53+ messages in thread
From: Leon Romanovsky @ 2020-06-04 10:45 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Wed, Jun 03, 2020 at 04:32:02PM -0700, Kees Cook wrote:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
>
> I preparation for removing[2] the[3] macro[4], remove all remaining
> needless uses with the following script:
>
> git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
> xargs perl -pi -e \
> 's/\buninitialized_var\(([^\)]+)\)/\1/g;
> s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
>
> drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
> pathological white-space.
>
> No outstanding warnings were found building allmodconfig with GCC 9.3.0
> for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
> alpha, and m68k.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/arm/mach-sa1100/assabet.c | 2 +-
> arch/arm/mm/alignment.c | 2 +-
> arch/ia64/kernel/process.c | 2 +-
> arch/ia64/mm/discontig.c | 2 +-
> arch/ia64/mm/tlb.c | 2 +-
> arch/mips/lib/dump_tlb.c | 2 +-
> arch/mips/mm/init.c | 2 +-
> arch/mips/mm/tlb-r4k.c | 6 +++---
> arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
> arch/powerpc/kvm/book3s_pr.c | 2 +-
> arch/powerpc/kvm/powerpc.c | 2 +-
> arch/powerpc/platforms/52xx/mpc52xx_pic.c | 2 +-
> arch/s390/kernel/smp.c | 2 +-
> arch/x86/kernel/quirks.c | 10 +++++-----
> arch/x86/kvm/mmu/mmu.c | 2 +-
> arch/x86/kvm/mmu/paging_tmpl.h | 2 +-
> arch/x86/kvm/x86.c | 2 +-
> block/blk-merge.c | 2 +-
> drivers/acpi/acpi_pad.c | 2 +-
> drivers/ata/libata-scsi.c | 2 +-
> drivers/atm/zatm.c | 2 +-
> drivers/block/drbd/drbd_nl.c | 6 +++---
> drivers/block/rbd.c | 2 +-
> drivers/clk/clk-gate.c | 2 +-
> drivers/clk/spear/clk-vco-pll.c | 2 +-
> drivers/crypto/nx/nx-842-powernv.c | 2 +-
> drivers/firewire/ohci.c | 14 +++++++-------
> drivers/gpu/drm/bridge/sil-sii8620.c | 2 +-
> drivers/gpu/drm/drm_edid.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 6 +++---
> drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
> drivers/gpu/drm/i915/intel_uncore.c | 2 +-
> drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 4 ++--
> drivers/i2c/busses/i2c-rk3x.c | 2 +-
> drivers/ide/ide-acpi.c | 2 +-
> drivers/ide/ide-atapi.c | 2 +-
> drivers/ide/ide-io-std.c | 4 ++--
> drivers/ide/ide-io.c | 8 ++++----
> drivers/ide/ide-sysfs.c | 2 +-
> drivers/ide/umc8672.c | 2 +-
> drivers/idle/intel_idle.c | 2 +-
> drivers/infiniband/core/uverbs_cmd.c | 4 ++--
> drivers/infiniband/hw/cxgb4/cm.c | 2 +-
> drivers/infiniband/hw/cxgb4/cq.c | 2 +-
> drivers/infiniband/hw/mlx4/qp.c | 6 +++---
> drivers/infiniband/hw/mlx5/cq.c | 6 +++---
> drivers/infiniband/hw/mlx5/devx.c | 2 +-
> drivers/infiniband/hw/mlx5/qp.c | 2 +-
> drivers/infiniband/hw/mthca/mthca_qp.c | 10 +++++-----
> drivers/infiniband/sw/siw/siw_qp_rx.c | 2 +-
> drivers/input/serio/serio_raw.c | 2 +-
> drivers/input/touchscreen/sur40.c | 2 +-
> drivers/iommu/intel-iommu.c | 2 +-
> drivers/md/dm-io.c | 2 +-
> drivers/md/dm-ioctl.c | 2 +-
> drivers/md/dm-snap-persistent.c | 2 +-
> drivers/md/dm-table.c | 2 +-
> drivers/md/dm-writecache.c | 2 +-
> drivers/md/raid5.c | 2 +-
> drivers/media/dvb-frontends/rtl2832.c | 2 +-
> drivers/media/tuners/qt1010.c | 4 ++--
> drivers/media/usb/gspca/vicam.c | 2 +-
> drivers/media/usb/uvc/uvc_video.c | 8 ++++----
> drivers/memstick/host/jmb38x_ms.c | 2 +-
> drivers/memstick/host/tifm_ms.c | 2 +-
> drivers/mmc/host/sdhci.c | 2 +-
> drivers/mtd/nand/raw/nand_ecc.c | 2 +-
> drivers/mtd/nand/raw/s3c2410.c | 2 +-
> drivers/mtd/parsers/afs.c | 4 ++--
> drivers/mtd/ubi/eba.c | 2 +-
> drivers/net/can/janz-ican3.c | 2 +-
> drivers/net/ethernet/broadcom/bnx2.c | 4 ++--
> .../net/ethernet/mellanox/mlx5/core/pagealloc.c | 4 ++--
> drivers/net/ethernet/neterion/s2io.c | 2 +-
> drivers/net/ethernet/qlogic/qla3xxx.c | 2 +-
> drivers/net/ethernet/sun/cassini.c | 2 +-
> drivers/net/ethernet/sun/niu.c | 6 +++---
> drivers/net/wan/z85230.c | 2 +-
> drivers/net/wireless/ath/ath10k/core.c | 2 +-
> drivers/net/wireless/ath/ath6kl/init.c | 2 +-
> drivers/net/wireless/ath/ath9k/init.c | 2 +-
> drivers/net/wireless/broadcom/b43/debugfs.c | 2 +-
> drivers/net/wireless/broadcom/b43/dma.c | 2 +-
> drivers/net/wireless/broadcom/b43/lo.c | 2 +-
> drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
> drivers/net/wireless/broadcom/b43/xmit.c | 12 ++++++------
> .../net/wireless/broadcom/b43legacy/debugfs.c | 2 +-
> drivers/net/wireless/broadcom/b43legacy/main.c | 2 +-
> drivers/net/wireless/intel/iwlegacy/3945.c | 2 +-
> drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
> .../net/wireless/realtek/rtlwifi/rtl8192cu/hw.c | 4 ++--
> drivers/pci/pcie/aer.c | 2 +-
> drivers/platform/x86/hdaps.c | 4 ++--
> drivers/scsi/dc395x.c | 2 +-
> drivers/scsi/pm8001/pm8001_hwi.c | 2 +-
> drivers/scsi/pm8001/pm80xx_hwi.c | 2 +-
> drivers/ssb/driver_chipcommon.c | 4 ++--
> drivers/tty/cyclades.c | 2 +-
> drivers/tty/isicom.c | 2 +-
> drivers/usb/musb/cppi_dma.c | 2 +-
> drivers/usb/storage/sddr55.c | 4 ++--
> drivers/vhost/net.c | 6 +++---
> drivers/video/fbdev/matrox/matroxfb_maven.c | 6 +++---
> drivers/video/fbdev/pm3fb.c | 6 +++---
> drivers/video/fbdev/riva/riva_hw.c | 3 +--
> drivers/virtio/virtio_ring.c | 6 +++---
> fs/afs/dir.c | 2 +-
> fs/afs/security.c | 2 +-
> fs/dlm/netlink.c | 2 +-
> fs/erofs/data.c | 4 ++--
> fs/erofs/zdata.c | 2 +-
> fs/f2fs/data.c | 2 +-
> fs/fat/dir.c | 2 +-
> fs/fuse/control.c | 4 ++--
> fs/fuse/cuse.c | 2 +-
> fs/fuse/file.c | 2 +-
> fs/gfs2/aops.c | 2 +-
> fs/gfs2/bmap.c | 2 +-
> fs/gfs2/lops.c | 2 +-
> fs/hfsplus/unicode.c | 2 +-
> fs/isofs/namei.c | 4 ++--
> fs/jffs2/erase.c | 2 +-
> fs/nfsd/nfsctl.c | 2 +-
> fs/ocfs2/alloc.c | 4 ++--
> fs/ocfs2/dir.c | 14 +++++++-------
> fs/ocfs2/extent_map.c | 4 ++--
> fs/ocfs2/namei.c | 2 +-
> fs/ocfs2/refcounttree.c | 2 +-
> fs/ocfs2/xattr.c | 2 +-
> fs/omfs/file.c | 2 +-
> fs/overlayfs/copy_up.c | 4 ++--
> fs/ubifs/commit.c | 6 +++---
> fs/ubifs/dir.c | 2 +-
> fs/ubifs/file.c | 4 ++--
> fs/ubifs/journal.c | 4 ++--
> fs/ubifs/lpt.c | 2 +-
> fs/ubifs/tnc.c | 6 +++---
> fs/ubifs/tnc_misc.c | 4 ++--
> fs/udf/balloc.c | 2 +-
> fs/xfs/xfs_bmap_util.c | 2 +-
> include/net/flow_offload.h | 2 +-
> kernel/async.c | 4 ++--
> kernel/audit.c | 2 +-
> kernel/debug/kdb/kdb_io.c | 2 +-
> kernel/dma/debug.c | 2 +-
> kernel/events/core.c | 2 +-
> kernel/events/uprobes.c | 2 +-
> kernel/exit.c | 2 +-
> kernel/futex.c | 14 +++++++-------
> kernel/locking/lockdep.c | 16 ++++++++--------
> kernel/trace/ring_buffer.c | 2 +-
> lib/radix-tree.c | 2 +-
> lib/test_lockup.c | 2 +-
> mm/frontswap.c | 2 +-
> mm/ksm.c | 2 +-
> mm/memcontrol.c | 2 +-
> mm/memory.c | 2 +-
> mm/mempolicy.c | 4 ++--
> mm/page_alloc.c | 2 +-
> mm/percpu.c | 2 +-
> mm/slub.c | 4 ++--
> mm/swap.c | 4 ++--
> net/dccp/options.c | 2 +-
> net/ipv4/netfilter/nf_socket_ipv4.c | 6 +++---
> net/ipv6/ip6_flowlabel.c | 2 +-
> net/ipv6/netfilter/nf_socket_ipv6.c | 2 +-
> net/netfilter/nf_conntrack_ftp.c | 2 +-
> net/netfilter/nfnetlink_log.c | 2 +-
> net/netfilter/nfnetlink_queue.c | 4 ++--
> net/sched/cls_flow.c | 2 +-
> net/sched/sch_cake.c | 2 +-
> net/sched/sch_cbq.c | 2 +-
> net/sched/sch_fq_codel.c | 2 +-
> net/sched/sch_fq_pie.c | 2 +-
> net/sched/sch_hfsc.c | 2 +-
> net/sched/sch_htb.c | 2 +-
> net/sched/sch_sfq.c | 2 +-
> net/sunrpc/svcsock.c | 4 ++--
> net/sunrpc/xprtsock.c | 10 +++++-----
> net/tls/tls_sw.c | 2 +-
> sound/core/control_compat.c | 2 +-
> sound/isa/sb/sb16_csp.c | 2 +-
> sound/usb/endpoint.c | 2 +-
> 184 files changed, 284 insertions(+), 285 deletions(-)
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com> # drivers/infiniband and mlx4/mlx5
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage
2020-06-04 7:58 ` Thomas Gleixner
@ 2020-06-04 11:41 ` Miguel Ojeda
2020-06-04 14:56 ` Kees Cook
2020-06-04 14:34 ` Kees Cook
1 sibling, 1 reply; 53+ messages in thread
From: Miguel Ojeda @ 2020-06-04 11:41 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, linux-kernel, Linus Torvalds, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi, Linux-MM,
clang-built-linux
On Thu, Jun 4, 2020 at 9:58 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> but if we ever lose the 1 then the above will silently compile the code
> within the IS_ENABLED() section out.
Yeah, I believe `IS_ENABLED()` is only meant for Kconfig symbols, not
macro defs in general. A better option would be `__is_defined()` which
works for defined-to-nothing too.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/10] treewide: Remove uninitialized_var() usage
[not found] ` <20200603233203.1695403-10-keescook@chromium.org>
2020-06-04 3:33 ` [PATCH 09/10] treewide: Remove uninitialized_var() usage Nathan Chancellor
2020-06-04 10:45 ` Leon Romanovsky
@ 2020-06-04 13:23 ` Jason Gunthorpe
2020-06-04 14:59 ` Kees Cook
2020-06-05 9:25 ` Kalle Valo
3 siblings, 1 reply; 53+ messages in thread
From: Jason Gunthorpe @ 2020-06-04 13:23 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Wed, Jun 03, 2020 at 04:32:02PM -0700, Kees Cook wrote:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
>
> I preparation for removing[2] the[3] macro[4], remove all remaining
> needless uses with the following script:
>
> git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
> xargs perl -pi -e \
> 's/\buninitialized_var\(([^\)]+)\)/\1/g;
> s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
>
> drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
> pathological white-space.
>
> No outstanding warnings were found building allmodconfig with GCC 9.3.0
> for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
> alpha, and m68k.
At least in the infiniband part I'm confident that old gcc versions
will print warnings after this patch.
As the warnings are wrong, do we care? Should old gcc maybe just -Wno-
the warning?
Otherwise the IB bits look ok to me
Acked-by: Jason Gunthorpe <jgg@mellanox.com>
Jason
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 00/10] Remove uninitialized_var() macro
2020-06-04 7:26 ` Sedat Dilek
@ 2020-06-04 14:27 ` Kees Cook
0 siblings, 0 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-04 14:27 UTC (permalink / raw)
To: Sedat Dilek
Cc: Nathan Chancellor, linux-kernel, Linus Torvalds, Miguel Ojeda,
Alexander Potapenko, Joe Perches, Andy Whitcroft, x86, drbd-dev,
linux-block, b43-dev, netdev, linux-wireless, linux-ide,
linux-clk, linux-spi, linux-mm, Clang-Built-Linux ML
On Thu, Jun 04, 2020 at 09:26:58AM +0200, Sedat Dilek wrote:
> On Thu, Jun 4, 2020 at 5:33 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Wed, Jun 03, 2020 at 04:31:53PM -0700, Kees Cook wrote:
> > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > (or can in the future), and suppresses unrelated compiler warnings
> > > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > > either simply initialize the variable or make compiler changes.
> > >
> > > As recommended[2] by[3] Linus[4], remove the macro.
> > [...]
> > For the series, consider it:
> >
> > Tested-by: Nathan Chancellor <natechancellor@gmail.com> [build]
> [...]
>
> I tried with updated version (checkpatch) of your tree and see no
> (new) warnings in my build-log.
>
> Feel free to add my...
>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Awesome! Thank you both! :)
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage
2020-06-04 7:58 ` Thomas Gleixner
2020-06-04 11:41 ` Miguel Ojeda
@ 2020-06-04 14:34 ` Kees Cook
2020-06-04 21:39 ` Thomas Gleixner
1 sibling, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-04 14:34 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Thu, Jun 04, 2020 at 09:58:07AM +0200, Thomas Gleixner wrote:
> Kees Cook <keescook@chromium.org> writes:
> > -#ifdef NODE_NOT_IN_PAGE_FLAGS
> > - pfn_align = node_map_pfn_alignment();
> > - if (pfn_align && pfn_align < PAGES_PER_SECTION) {
> > - printk(KERN_WARNING "Node alignment %LuMB < min %LuMB, rejecting NUMA config\n",
> > - PFN_PHYS(pfn_align) >> 20,
> > - PFN_PHYS(PAGES_PER_SECTION) >> 20);
> > - return -EINVAL;
> > + if (IS_ENABLED(NODE_NOT_IN_PAGE_FLAGS)) {
>
> Hrm, clever ...
>
> > + unsigned long pfn_align = node_map_pfn_alignment();
> > +
> > + if (pfn_align && pfn_align < PAGES_PER_SECTION) {
> > + pr_warn("Node alignment %LuMB < min %LuMB, rejecting NUMA config\n",
> > + PFN_PHYS(pfn_align) >> 20,
> > + PFN_PHYS(PAGES_PER_SECTION) >> 20);
> > + return -EINVAL;
> > + }
> > }
> > -#endif
> > if (!numa_meminfo_cover_memory(mi))
> > return -EINVAL;
> >
> > diff --git a/include/linux/page-flags-layout.h b/include/linux/page-flags-layout.h
> > index 71283739ffd2..1a4cdec2bd29 100644
> > --- a/include/linux/page-flags-layout.h
> > +++ b/include/linux/page-flags-layout.h
> > @@ -100,7 +100,7 @@
> > * there. This includes the case where there is no node, so it is implicit.
> > */
> > #if !(NODES_WIDTH > 0 || NODES_SHIFT == 0)
> > -#define NODE_NOT_IN_PAGE_FLAGS
> > +#define NODE_NOT_IN_PAGE_FLAGS 1
>
> but if we ever lose the 1 then the above will silently compile the code
> within the IS_ENABLED() section out.
That's true, yes. I considered two other ways to do this:
1) smallest patch, but more #ifdef:
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 59ba008504dc..fbf5231a3d35 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -541,7 +541,9 @@ static void __init numa_clear_kernel_node_hotplug(void)
static int __init numa_register_memblks(struct numa_meminfo *mi)
{
- unsigned long uninitialized_var(pfn_align);
+#ifdef NODE_NOT_IN_PAGE_FLAGS
+ unsigned long pfn_align;
+#endif
int i, nid;
/* Account for nodes with cpus and no memory */
2) medium size, weird style:
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 59ba008504dc..0df7ba9b21b2 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -541,7 +541,6 @@ static void __init numa_clear_kernel_node_hotplug(void)
static int __init numa_register_memblks(struct numa_meminfo *mi)
{
- unsigned long uninitialized_var(pfn_align);
int i, nid;
/* Account for nodes with cpus and no memory */
@@ -570,12 +569,15 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
* whether its granularity is fine enough.
*/
#ifdef NODE_NOT_IN_PAGE_FLAGS
- pfn_align = node_map_pfn_alignment();
- if (pfn_align && pfn_align < PAGES_PER_SECTION) {
- printk(KERN_WARNING "Node alignment %LuMB < min %LuMB, rejecting NUMA config\n",
- PFN_PHYS(pfn_align) >> 20,
- PFN_PHYS(PAGES_PER_SECTION) >> 20);
- return -EINVAL;
+ {
+ unsigned long pfn_align = node_map_pfn_alignment();
+
+ if (pfn_align && pfn_align < PAGES_PER_SECTION) {
+ pr_warn("Node alignment %LuMB < min %LuMB, rejecting NUMA config\n",
+ PFN_PHYS(pfn_align) >> 20,
+ PFN_PHYS(PAGES_PER_SECTION) >> 20);
+ return -EINVAL;
+ }
}
#endif
if (!numa_meminfo_cover_memory(mi))
and 3 is what I sent: biggest, but removes #ifdef
Any preference?
Thanks!
--
Kees Cook
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage
2020-06-04 11:41 ` Miguel Ojeda
@ 2020-06-04 14:56 ` Kees Cook
2020-06-04 15:22 ` Miguel Ojeda
0 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-04 14:56 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Thomas Gleixner, linux-kernel, Linus Torvalds,
Alexander Potapenko, Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi, Linux-MM,
clang-built-linux
On Thu, Jun 04, 2020 at 01:41:07PM +0200, Miguel Ojeda wrote:
> On Thu, Jun 4, 2020 at 9:58 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > but if we ever lose the 1 then the above will silently compile the code
> > within the IS_ENABLED() section out.
>
> Yeah, I believe `IS_ENABLED()` is only meant for Kconfig symbols, not
> macro defs in general. A better option would be `__is_defined()` which
> works for defined-to-nothing too.
Er? That's not what it looked like to me:
#define IS_BUILTIN(option) __is_defined(option)
#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
But just to be sure, I just tested in with a real build:
[ 3.242160] IS_ENABLED(TEST_UNDEF) false
[ 3.242691] __is_defined(TEST_UNDEF) false
[ 3.243240] IS_ENABLED(TEST_VALUE_EMPTY) false
[ 3.243794] __is_defined(TEST_VALUE_EMPTY) false
[ 3.244353] IS_ENABLED(TEST_VALUE_1) true
[ 3.244848] __is_defined(TEST_VALUE_1) true
and nope, it only works with a defined value present.
diff --git a/init/main.c b/init/main.c
index 03371976d387..378a9e54b6dc 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1406,6 +1406,34 @@ static int __ref kernel_init(void *unused)
*/
pti_finalize();
+#undef TEST_UNDEF
+ if (IS_ENABLED(TEST_UNDEF))
+ pr_info("IS_ENABLED(TEST_UNDEF) true\n");
+ else
+ pr_info("IS_ENABLED(TEST_UNDEF) false\n");
+ if (__is_defined(TEST_UNDEF))
+ pr_info("__is_defined(TEST_UNDEF) true\n");
+ else
+ pr_info("__is_defined(TEST_UNDEF) false\n");
+#define TEST_VALUE_EMPTY
+ if (IS_ENABLED(TEST_VALUE_EMPTY))
+ pr_info("IS_ENABLED(TEST_VALUE_EMPTY) true\n");
+ else
+ pr_info("IS_ENABLED(TEST_VALUE_EMPTY) false\n");
+ if (__is_defined(TEST_VALUE_EMPTY))
+ pr_info("__is_defined(TEST_VALUE_EMPTY) true\n");
+ else
+ pr_info("__is_defined(TEST_VALUE_EMPTY) false\n");
+#define TEST_VALUE_1 1
+ if (IS_ENABLED(TEST_VALUE_1))
+ pr_info("IS_ENABLED(TEST_VALUE_1) true\n");
+ else
+ pr_info("IS_ENABLED(TEST_VALUE_1) false\n");
+ if (__is_defined(TEST_VALUE_1))
+ pr_info("__is_defined(TEST_VALUE_1) true\n");
+ else
+ pr_info("__is_defined(TEST_VALUE_1) false\n");
+
system_state = SYSTEM_RUNNING;
numa_default_policy();
which means a few other __is_defined() users are not correct too...
--
Kees Cook
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 09/10] treewide: Remove uninitialized_var() usage
2020-06-04 13:23 ` Jason Gunthorpe
@ 2020-06-04 14:59 ` Kees Cook
2020-06-04 17:57 ` Jason Gunthorpe
2020-06-04 19:09 ` Geert Uytterhoeven
0 siblings, 2 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-04 14:59 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Thu, Jun 04, 2020 at 10:23:06AM -0300, Jason Gunthorpe wrote:
> On Wed, Jun 03, 2020 at 04:32:02PM -0700, Kees Cook wrote:
> > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > (or can in the future), and suppresses unrelated compiler warnings
> > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > either simply initialize the variable or make compiler changes.
> >
> > I preparation for removing[2] the[3] macro[4], remove all remaining
> > needless uses with the following script:
> >
> > git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
> > xargs perl -pi -e \
> > 's/\buninitialized_var\(([^\)]+)\)/\1/g;
> > s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
> >
> > drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
> > pathological white-space.
> >
> > No outstanding warnings were found building allmodconfig with GCC 9.3.0
> > for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
> > alpha, and m68k.
>
> At least in the infiniband part I'm confident that old gcc versions
> will print warnings after this patch.
>
> As the warnings are wrong, do we care? Should old gcc maybe just -Wno-
> the warning?
I *think* a lot of those are from -Wmaybe-uninitialized, but Linus just
turned that off unconditionally in v5.7:
78a5255ffb6a ("Stop the ad-hoc games with -Wno-maybe-initialized")
I'll try to double-check with some older gcc versions. My compiler
collection is mostly single-axis: lots of arches, not lots of versions. ;)
> Otherwise the IB bits look ok to me
>
> Acked-by: Jason Gunthorpe <jgg@mellanox.com>
Thanks!
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage
2020-06-04 14:56 ` Kees Cook
@ 2020-06-04 15:22 ` Miguel Ojeda
0 siblings, 0 replies; 53+ messages in thread
From: Miguel Ojeda @ 2020-06-04 15:22 UTC (permalink / raw)
To: Kees Cook
Cc: Thomas Gleixner, linux-kernel, Linus Torvalds,
Alexander Potapenko, Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi, Linux-MM,
clang-built-linux, Masahiro Yamada
On Thu, Jun 4, 2020 at 4:56 PM Kees Cook <keescook@chromium.org> wrote:
>
> Er? That's not what it looked like to me:
>
> #define IS_BUILTIN(option) __is_defined(option)
> #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
>
> But just to be sure, I just tested in with a real build:
>
> [ 3.242160] IS_ENABLED(TEST_UNDEF) false
> [ 3.242691] __is_defined(TEST_UNDEF) false
> [ 3.243240] IS_ENABLED(TEST_VALUE_EMPTY) false
> [ 3.243794] __is_defined(TEST_VALUE_EMPTY) false
> [ 3.244353] IS_ENABLED(TEST_VALUE_1) true
> [ 3.244848] __is_defined(TEST_VALUE_1) true
>
> and nope, it only works with a defined value present.
You are right, it follows the Kconfig logic, returning false for
defined-but-to-0 too.
We should probably add an `IS_DEFINED()` macro kernel-wide for this
(and add it to the `coding-guidelines.rst` since `IS_ENABLED()` is
mentioned there, with a warning not to mix it with `__is_defined()`
which looks it was only intended as an implementation detail for
`include/linux/kconfig.h`).
CC'ing Masahiro by the way.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/10] treewide: Remove uninitialized_var() usage
2020-06-04 14:59 ` Kees Cook
@ 2020-06-04 17:57 ` Jason Gunthorpe
2020-06-04 19:09 ` Geert Uytterhoeven
1 sibling, 0 replies; 53+ messages in thread
From: Jason Gunthorpe @ 2020-06-04 17:57 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Thu, Jun 04, 2020 at 07:59:40AM -0700, Kees Cook wrote:
> On Thu, Jun 04, 2020 at 10:23:06AM -0300, Jason Gunthorpe wrote:
> > On Wed, Jun 03, 2020 at 04:32:02PM -0700, Kees Cook wrote:
> > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > (or can in the future), and suppresses unrelated compiler warnings
> > > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > > either simply initialize the variable or make compiler changes.
> > >
> > > I preparation for removing[2] the[3] macro[4], remove all remaining
> > > needless uses with the following script:
> > >
> > > git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
> > > xargs perl -pi -e \
> > > 's/\buninitialized_var\(([^\)]+)\)/\1/g;
> > > s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
> > >
> > > drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
> > > pathological white-space.
> > >
> > > No outstanding warnings were found building allmodconfig with GCC 9.3.0
> > > for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
> > > alpha, and m68k.
> >
> > At least in the infiniband part I'm confident that old gcc versions
> > will print warnings after this patch.
> >
> > As the warnings are wrong, do we care? Should old gcc maybe just -Wno-
> > the warning?
>
> I *think* a lot of those are from -Wmaybe-uninitialized, but Linus just
> turned that off unconditionally in v5.7:
> 78a5255ffb6a ("Stop the ad-hoc games with -Wno-maybe-initialized")
Yah, that alone is justification enough to do this purge.
Jason
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/10] treewide: Remove uninitialized_var() usage
2020-06-04 14:59 ` Kees Cook
2020-06-04 17:57 ` Jason Gunthorpe
@ 2020-06-04 19:09 ` Geert Uytterhoeven
1 sibling, 0 replies; 53+ messages in thread
From: Geert Uytterhoeven @ 2020-06-04 19:09 UTC (permalink / raw)
To: Kees Cook
Cc: Jason Gunthorpe, Linux Kernel Mailing List, Linus Torvalds,
Miguel Ojeda, Alexander Potapenko, Joe Perches, Andy Whitcroft,
the arch/x86 maintainers, Lars Ellenberg, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
Linux MM, clang-built-linux
Hi Kees,
On Thu, Jun 4, 2020 at 5:01 PM Kees Cook <keescook@chromium.org> wrote:
> On Thu, Jun 04, 2020 at 10:23:06AM -0300, Jason Gunthorpe wrote:
> > On Wed, Jun 03, 2020 at 04:32:02PM -0700, Kees Cook wrote:
> > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > (or can in the future), and suppresses unrelated compiler warnings
> > > (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> > > either simply initialize the variable or make compiler changes.
> > >
> > > I preparation for removing[2] the[3] macro[4], remove all remaining
> > > needless uses with the following script:
> > >
> > > git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
> > > xargs perl -pi -e \
> > > 's/\buninitialized_var\(([^\)]+)\)/\1/g;
> > > s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
> > >
> > > drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
> > > pathological white-space.
> > >
> > > No outstanding warnings were found building allmodconfig with GCC 9.3.0
> > > for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
> > > alpha, and m68k.
> >
> > At least in the infiniband part I'm confident that old gcc versions
> > will print warnings after this patch.
> >
> > As the warnings are wrong, do we care? Should old gcc maybe just -Wno-
> > the warning?
>
> I *think* a lot of those are from -Wmaybe-uninitialized, but Linus just
> turned that off unconditionally in v5.7:
> 78a5255ffb6a ("Stop the ad-hoc games with -Wno-maybe-initialized")
>
> I'll try to double-check with some older gcc versions. My compiler
> collection is mostly single-axis: lots of arches, not lots of versions. ;)
Nope, support for the good old gcc 4.1 was removed a while ago.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 05/10] ide: Remove uninitialized_var() usage
2020-06-03 23:31 ` [PATCH 05/10] ide: " Kees Cook
@ 2020-06-04 19:29 ` Nick Desaulniers
2020-06-04 20:20 ` Kees Cook
0 siblings, 1 reply; 53+ messages in thread
From: Nick Desaulniers @ 2020-06-04 19:29 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux,
Sebastian Andrzej Siewior
On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
>
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just remove this variable since it was
> actually unused:
>
> drivers/ide/ide-taskfile.c:232:34: warning: unused variable 'flags' [-Wunused-variable]
> unsigned long uninitialized_var(flags);
> ^
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Fixes ce1e518190ea ("ide: don't disable interrupts during kmap_atomic()")
> ---
> drivers/ide/ide-taskfile.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
> index aab6a10435b6..a26f85ab58a9 100644
> --- a/drivers/ide/ide-taskfile.c
> +++ b/drivers/ide/ide-taskfile.c
> @@ -229,7 +229,6 @@ void ide_pio_bytes(ide_drive_t *drive, struct ide_cmd *cmd,
> ide_hwif_t *hwif = drive->hwif;
> struct scatterlist *sg = hwif->sg_table;
> struct scatterlist *cursg = cmd->cursg;
> - unsigned long uninitialized_var(flags);
> struct page *page;
> unsigned int offset;
> u8 *buf;
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200603233203.1695403-6-keescook%40chromium.org.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 07/10] spi: davinci: Remove uninitialized_var() usage
2020-06-03 23:32 ` [PATCH 07/10] spi: davinci: " Kees Cook
@ 2020-06-04 19:40 ` Nick Desaulniers
0 siblings, 0 replies; 53+ messages in thread
From: Nick Desaulniers @ 2020-06-04 19:40 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux, mporter
On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
>
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just remove this variable since it was
> actually unused:
>
> drivers/spi/spi-davinci.c: In function ‘davinci_spi_bufs’:
> drivers/spi/spi-davinci.c:579:11: warning: unused variable ‘rx_buf_count’ [-Wunused-variable]
> 579 | unsigned rx_buf_count;
> | ^~~~~~~~~~~~
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Fixes 048177ce3b39 ("spi: spi-davinci: convert to DMA engine API")
> ---
> drivers/spi/spi-davinci.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
> index f71c497393a6..f50c0c79cbdf 100644
> --- a/drivers/spi/spi-davinci.c
> +++ b/drivers/spi/spi-davinci.c
> @@ -576,7 +576,6 @@ static int davinci_spi_bufs(struct spi_device *spi, struct spi_transfer *t)
> u32 errors = 0;
> struct davinci_spi_config *spicfg;
> struct davinci_spi_platform_data *pdata;
> - unsigned uninitialized_var(rx_buf_count);
>
> dspi = spi_master_get_devdata(spi->master);
> pdata = &dspi->pdata;
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200603233203.1695403-8-keescook%40chromium.org.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 02/10] drbd: Remove uninitialized_var() usage
2020-06-03 23:31 ` [PATCH 02/10] drbd: " Kees Cook
@ 2020-06-04 19:56 ` Nick Desaulniers
0 siblings, 0 replies; 53+ messages in thread
From: Nick Desaulniers @ 2020-06-04 19:56 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux
On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
>
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just initialize this variable to NULL.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Fixes: a29728463b25 ("drbd: Backport the "events2" command")
> ---
> drivers/block/drbd/drbd_state.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
> index eeaa3b49b264..0067d328f0b5 100644
> --- a/drivers/block/drbd/drbd_state.c
> +++ b/drivers/block/drbd/drbd_state.c
> @@ -1604,7 +1604,7 @@ static void broadcast_state_change(struct drbd_state_change *state_change)
> unsigned int n_device, n_connection, n_peer_device, n_peer_devices;
> void (*last_func)(struct sk_buff *, unsigned int, void *,
> enum drbd_notification_type) = NULL;
> - void *uninitialized_var(last_arg);
> + void *last_arg = NULL;
>
> #define HAS_CHANGED(state) ((state)[OLD] != (state)[NEW])
> #define FINAL_STATE_CHANGE(type) \
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200603233203.1695403-3-keescook%40chromium.org.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 03/10] b43: Remove uninitialized_var() usage
2020-06-03 23:31 ` [PATCH 03/10] b43: " Kees Cook
@ 2020-06-04 20:08 ` Nick Desaulniers
2020-06-04 20:18 ` Kees Cook
2020-06-05 9:20 ` Kalle Valo
1 sibling, 1 reply; 53+ messages in thread
From: Nick Desaulniers @ 2020-06-04 20:08 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux
On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
>
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just initialize this variable to NULL,
> and make the (unreachable!) code do a conditional test.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/net/wireless/broadcom/b43/phy_n.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
> index d3c001fa8eb4..88cdcea10d61 100644
> --- a/drivers/net/wireless/broadcom/b43/phy_n.c
> +++ b/drivers/net/wireless/broadcom/b43/phy_n.c
> @@ -4222,7 +4222,7 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
The TODOs and `#if 0` in this function are concerning. It looks like
`rf_pwr_offset_table` is only used when `phy->rev` is >=7 && < 19.
Further, the loop has a case for `phy->rev >= 19` but we would have
returned earlier if that was the case.
> u32 rfpwr_offset;
> u8 pga_gain, pad_gain;
> int i;
> - const s16 *uninitialized_var(rf_pwr_offset_table);
> + const s16 *rf_pwr_offset_table = NULL;
>
> table = b43_nphy_get_tx_gain_table(dev);
> if (!table)
> @@ -4256,9 +4256,13 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
> pga_gain = (table[i] >> 24) & 0xf;
> pad_gain = (table[i] >> 19) & 0x1f;
> if (b43_current_band(dev->wl) == NL80211_BAND_2GHZ)
> - rfpwr_offset = rf_pwr_offset_table[pad_gain];
> + rfpwr_offset = rf_pwr_offset_table
> + ? rf_pwr_offset_table[pad_gain]
> + : 0;
> else
> - rfpwr_offset = rf_pwr_offset_table[pga_gain];
> + rfpwr_offset = rf_pwr_offset_table
> + ? rf_pwr_offset_table[pga_gain]
> + : 0;
The code is trying to check `phy->rev >= 7 && phy->rev < 19` once
before the loop, then set `rf_pwr_offset_table`, so having another
conditional on `rf_pwr_offset_table` in the loop is unnecessary. I'm
ok with initializing it to `NULL`, but I'm not sure the conditional
check is necessary. Do you get a compiler warning otherwise?
> } else {
> pga_gain = (table[i] >> 24) & 0xF;
> if (b43_current_band(dev->wl) == NL80211_BAND_2GHZ)
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200603233203.1695403-4-keescook%40chromium.org.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 04/10] rtlwifi: rtl8192cu: Remove uninitialized_var() usage
2020-06-03 23:31 ` [PATCH 04/10] rtlwifi: rtl8192cu: " Kees Cook
@ 2020-06-04 20:16 ` Nick Desaulniers
2020-06-05 9:18 ` Kalle Valo
1 sibling, 0 replies; 53+ messages in thread
From: Nick Desaulniers @ 2020-06-04 20:16 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux
On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
>
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just initialize this variable to NULL,
> and avoid sending garbage by returning.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Fixes: dc0313f46664 ("rtlwifi: rtl8192cu: Add routine hw")
> ---
> drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
> index f070f25bb735..5b071b70bc08 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c
> @@ -592,7 +592,7 @@ static void _rtl92cu_init_chipn_one_out_ep_priority(struct ieee80211_hw *hw,
> bool wmm_enable,
> u8 queue_sel)
> {
> - u16 uninitialized_var(value);
> + u16 value;
>
> switch (queue_sel) {
> case TX_SELE_HQ:
> @@ -606,7 +606,7 @@ static void _rtl92cu_init_chipn_one_out_ep_priority(struct ieee80211_hw *hw,
> break;
> default:
> WARN_ON(1); /* Shall not reach here! */
> - break;
> + return;
> }
> _rtl92c_init_chipn_reg_priority(hw, value, value, value, value,
> value, value);
Whew! Nothing like passing the same value 6 times! (Other callers do
use distinct values though, just curious seeing this instance.)
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 03/10] b43: Remove uninitialized_var() usage
2020-06-04 20:08 ` Nick Desaulniers
@ 2020-06-04 20:18 ` Kees Cook
2020-06-04 20:25 ` Nick Desaulniers
0 siblings, 1 reply; 53+ messages in thread
From: Kees Cook @ 2020-06-04 20:18 UTC (permalink / raw)
To: Nick Desaulniers
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux
On Thu, Jun 04, 2020 at 01:08:44PM -0700, Nick Desaulniers wrote:
> On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > (or can in the future), and suppresses unrelated compiler warnings (e.g.
> > "unused variable"). If the compiler thinks it is uninitialized, either
> > simply initialize the variable or make compiler changes. As a precursor
> > to removing[2] this[3] macro[4], just initialize this variable to NULL,
> > and make the (unreachable!) code do a conditional test.
> >
> > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > drivers/net/wireless/broadcom/b43/phy_n.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
> > index d3c001fa8eb4..88cdcea10d61 100644
> > --- a/drivers/net/wireless/broadcom/b43/phy_n.c
> > +++ b/drivers/net/wireless/broadcom/b43/phy_n.c
> > @@ -4222,7 +4222,7 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
>
> The TODOs and `#if 0` in this function are concerning. It looks like
> `rf_pwr_offset_table` is only used when `phy->rev` is >=7 && < 19.
>
> Further, the loop has a case for `phy->rev >= 19` but we would have
> returned earlier if that was the case.
Yeah, that's why I put the "(unreachable!)" note in the commit log. ;)
>
> > u32 rfpwr_offset;
> > u8 pga_gain, pad_gain;
> > int i;
> > - const s16 *uninitialized_var(rf_pwr_offset_table);
> > + const s16 *rf_pwr_offset_table = NULL;
> >
> > table = b43_nphy_get_tx_gain_table(dev);
> > if (!table)
> > @@ -4256,9 +4256,13 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
> > pga_gain = (table[i] >> 24) & 0xf;
> > pad_gain = (table[i] >> 19) & 0x1f;
> > if (b43_current_band(dev->wl) == NL80211_BAND_2GHZ)
> > - rfpwr_offset = rf_pwr_offset_table[pad_gain];
> > + rfpwr_offset = rf_pwr_offset_table
> > + ? rf_pwr_offset_table[pad_gain]
> > + : 0;
> > else
> > - rfpwr_offset = rf_pwr_offset_table[pga_gain];
> > + rfpwr_offset = rf_pwr_offset_table
> > + ? rf_pwr_offset_table[pga_gain]
> > + : 0;
>
>
> The code is trying to check `phy->rev >= 7 && phy->rev < 19` once
> before the loop, then set `rf_pwr_offset_table`, so having another
> conditional on `rf_pwr_offset_table` in the loop is unnecessary. I'm
> ok with initializing it to `NULL`, but I'm not sure the conditional
> check is necessary. Do you get a compiler warning otherwise?
I mean, sort of the best thing to do is just remove nearly everything
here since it's actually unreachable. But it is commented as "when
supported ..." etc, so I figured I'd leave it. As part of that I didn't
want to leave any chance of a NULL deref, so I added the explicit tests
just for robustness.
*shrug*
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 05/10] ide: Remove uninitialized_var() usage
2020-06-04 19:29 ` Nick Desaulniers
@ 2020-06-04 20:20 ` Kees Cook
2020-06-04 20:29 ` Nick Desaulniers
2020-06-04 20:58 ` Sedat Dilek
0 siblings, 2 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-04 20:20 UTC (permalink / raw)
To: Nick Desaulniers
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux,
Sebastian Andrzej Siewior
On Thu, Jun 04, 2020 at 12:29:17PM -0700, Nick Desaulniers wrote:
> On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > (or can in the future), and suppresses unrelated compiler warnings (e.g.
> > "unused variable"). If the compiler thinks it is uninitialized, either
> > simply initialize the variable or make compiler changes. As a precursor
> > to removing[2] this[3] macro[4], just remove this variable since it was
> > actually unused:
> >
> > drivers/ide/ide-taskfile.c:232:34: warning: unused variable 'flags' [-Wunused-variable]
> > unsigned long uninitialized_var(flags);
> > ^
> >
> > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Thanks for the reviews!
> Fixes ce1e518190ea ("ide: don't disable interrupts during kmap_atomic()")
I originally avoided adding Fixes tags because I didn't want these
changes backported into a -stable without -Wmaybe-uninitialized
disabled, but in these cases (variable removal), that actually does make
sense. Thanks!
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 03/10] b43: Remove uninitialized_var() usage
2020-06-04 20:18 ` Kees Cook
@ 2020-06-04 20:25 ` Nick Desaulniers
0 siblings, 0 replies; 53+ messages in thread
From: Nick Desaulniers @ 2020-06-04 20:25 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux
On Thu, Jun 4, 2020 at 1:18 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Jun 04, 2020 at 01:08:44PM -0700, Nick Desaulniers wrote:
> > On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > (or can in the future), and suppresses unrelated compiler warnings (e.g.
> > > "unused variable"). If the compiler thinks it is uninitialized, either
> > > simply initialize the variable or make compiler changes. As a precursor
> > > to removing[2] this[3] macro[4], just initialize this variable to NULL,
> > > and make the (unreachable!) code do a conditional test.
> > >
> > > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > ---
> > > drivers/net/wireless/broadcom/b43/phy_n.c | 10 +++++++---
> > > 1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
> > > index d3c001fa8eb4..88cdcea10d61 100644
> > > --- a/drivers/net/wireless/broadcom/b43/phy_n.c
> > > +++ b/drivers/net/wireless/broadcom/b43/phy_n.c
> > > @@ -4222,7 +4222,7 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
> >
> > The TODOs and `#if 0` in this function are concerning. It looks like
> > `rf_pwr_offset_table` is only used when `phy->rev` is >=7 && < 19.
> >
> > Further, the loop has a case for `phy->rev >= 19` but we would have
> > returned earlier if that was the case.
oh, and there's an early return for `phy->rev < 3` I just noticed.
>
> Yeah, that's why I put the "(unreachable!)" note in the commit log. ;)
I don't think that note is correct.
>
> >
> > > u32 rfpwr_offset;
> > > u8 pga_gain, pad_gain;
> > > int i;
> > > - const s16 *uninitialized_var(rf_pwr_offset_table);
> > > + const s16 *rf_pwr_offset_table = NULL;
> > >
> > > table = b43_nphy_get_tx_gain_table(dev);
> > > if (!table)
> > > @@ -4256,9 +4256,13 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
> > > pga_gain = (table[i] >> 24) & 0xf;
> > > pad_gain = (table[i] >> 19) & 0x1f;
> > > if (b43_current_band(dev->wl) == NL80211_BAND_2GHZ)
> > > - rfpwr_offset = rf_pwr_offset_table[pad_gain];
> > > + rfpwr_offset = rf_pwr_offset_table
> > > + ? rf_pwr_offset_table[pad_gain]
> > > + : 0;
> > > else
> > > - rfpwr_offset = rf_pwr_offset_table[pga_gain];
> > > + rfpwr_offset = rf_pwr_offset_table
> > > + ? rf_pwr_offset_table[pga_gain]
> > > + : 0;
> >
> >
> > The code is trying to check `phy->rev >= 7 && phy->rev < 19` once
> > before the loop, then set `rf_pwr_offset_table`, so having another
> > conditional on `rf_pwr_offset_table` in the loop is unnecessary. I'm
> > ok with initializing it to `NULL`, but I'm not sure the conditional
> > check is necessary. Do you get a compiler warning otherwise?
>
> I mean, sort of the best thing to do is just remove nearly everything
> here since it's actually unreachable. But it is commented as "when
This code is reachable. Consider `phy->rev >= 7 && phy->rev < 19`. If
`rf_pwr_offset_table` was NULL, it would have returned early on L4246,
so the checks added in this patch are unnecessary. Forgive me if
there's some other control flow I'm not considering.
> supported ..." etc, so I figured I'd leave it. As part of that I didn't
> want to leave any chance of a NULL deref, so I added the explicit tests
> just for robustness.
>
> *shrug*
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 05/10] ide: Remove uninitialized_var() usage
2020-06-04 20:20 ` Kees Cook
@ 2020-06-04 20:29 ` Nick Desaulniers
2020-06-15 19:32 ` Kees Cook
2020-06-04 20:58 ` Sedat Dilek
1 sibling, 1 reply; 53+ messages in thread
From: Nick Desaulniers @ 2020-06-04 20:29 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux,
Sebastian Andrzej Siewior, Saravana Kannan
On Thu, Jun 4, 2020 at 1:20 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Jun 04, 2020 at 12:29:17PM -0700, Nick Desaulniers wrote:
> > On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > (or can in the future), and suppresses unrelated compiler warnings (e.g.
> > > "unused variable"). If the compiler thinks it is uninitialized, either
> > > simply initialize the variable or make compiler changes. As a precursor
> > > to removing[2] this[3] macro[4], just remove this variable since it was
> > > actually unused:
> > >
> > > drivers/ide/ide-taskfile.c:232:34: warning: unused variable 'flags' [-Wunused-variable]
> > > unsigned long uninitialized_var(flags);
> > > ^
> > >
> > > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> >
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Thanks for the reviews!
>
> > Fixes ce1e518190ea ("ide: don't disable interrupts during kmap_atomic()")
>
> I originally avoided adding Fixes tags because I didn't want these
> changes backported into a -stable without -Wmaybe-uninitialized
> disabled, but in these cases (variable removal), that actually does make
> sense. Thanks!
Saravana showed me a cool trick for quickly finding commits that
removed a particular identifier that I find faster than `git blame` or
vim-fugitive for the purpose of Fixes tags:
$ git log -S <string> <file>
I've added it to our wiki:
https://github.com/ClangBuiltLinux/linux/wiki/Command-line-tips-and-tricks#for-finding-which-commit-may-have-removed-a-string-try.
I should update the first tip; what was your suggestion for
constraining the search to the current remote?
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 05/10] ide: Remove uninitialized_var() usage
2020-06-04 20:20 ` Kees Cook
2020-06-04 20:29 ` Nick Desaulniers
@ 2020-06-04 20:58 ` Sedat Dilek
1 sibling, 0 replies; 53+ messages in thread
From: Sedat Dilek @ 2020-06-04 20:58 UTC (permalink / raw)
To: Kees Cook
Cc: Nick Desaulniers, LKML, Linus Torvalds, Miguel Ojeda,
Alexander Potapenko, Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux,
Sebastian Andrzej Siewior
On Thu, Jun 4, 2020 at 10:20 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Jun 04, 2020 at 12:29:17PM -0700, Nick Desaulniers wrote:
> > On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > (or can in the future), and suppresses unrelated compiler warnings (e.g.
> > > "unused variable"). If the compiler thinks it is uninitialized, either
> > > simply initialize the variable or make compiler changes. As a precursor
> > > to removing[2] this[3] macro[4], just remove this variable since it was
> > > actually unused:
> > >
> > > drivers/ide/ide-taskfile.c:232:34: warning: unused variable 'flags' [-Wunused-variable]
> > > unsigned long uninitialized_var(flags);
> > > ^
> > >
> > > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> >
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Thanks for the reviews!
>
> > Fixes ce1e518190ea ("ide: don't disable interrupts during kmap_atomic()")
>
> I originally avoided adding Fixes tags because I didn't want these
> changes backported into a -stable without -Wmaybe-uninitialized
> disabled, but in these cases (variable removal), that actually does make
> sense. Thanks!
>
Fixes tag does not automatically mean it is "for-stable".
[1] says:
> Patches that fix a severe bug in a released kernel should be directed
> toward the stable maintainers by putting a line like this::
>
> Cc: stable@vger.kernel.org
- Sedat -
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n299
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage
2020-06-04 14:34 ` Kees Cook
@ 2020-06-04 21:39 ` Thomas Gleixner
2020-06-04 22:39 ` Kees Cook
0 siblings, 1 reply; 53+ messages in thread
From: Thomas Gleixner @ 2020-06-04 21:39 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Kees Cook <keescook@chromium.org> writes:
>> > -#define NODE_NOT_IN_PAGE_FLAGS
>> > +#define NODE_NOT_IN_PAGE_FLAGS 1
>>
>> but if we ever lose the 1 then the above will silently compile the code
>> within the IS_ENABLED() section out.
>
> That's true, yes. I considered two other ways to do this:
>
> 1) smallest patch, but more #ifdef:
> 2) medium size, weird style:
>
> and 3 is what I sent: biggest, but removes #ifdef
>
> Any preference?
From a readbility POV I surely prefer #3. i"m just wary. Add a big fat
comment to the define might mitigate that, hmm?
Thanks,
tglx
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage
2020-06-04 21:39 ` Thomas Gleixner
@ 2020-06-04 22:39 ` Kees Cook
0 siblings, 0 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-04 22:39 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
On Thu, Jun 04, 2020 at 11:39:05PM +0200, Thomas Gleixner wrote:
> Kees Cook <keescook@chromium.org> writes:
> >> > -#define NODE_NOT_IN_PAGE_FLAGS
> >> > +#define NODE_NOT_IN_PAGE_FLAGS 1
> >>
> >> but if we ever lose the 1 then the above will silently compile the code
> >> within the IS_ENABLED() section out.
> >
> > That's true, yes. I considered two other ways to do this:
> >
> > 1) smallest patch, but more #ifdef:
> > 2) medium size, weird style:
> >
> > and 3 is what I sent: biggest, but removes #ifdef
> >
> > Any preference?
>
> From a readbility POV I surely prefer #3. i"m just wary. Add a big fat
> comment to the define might mitigate that, hmm?
Sure! I'll add it.
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 04/10] rtlwifi: rtl8192cu: Remove uninitialized_var() usage
2020-06-03 23:31 ` [PATCH 04/10] rtlwifi: rtl8192cu: " Kees Cook
2020-06-04 20:16 ` Nick Desaulniers
@ 2020-06-05 9:18 ` Kalle Valo
1 sibling, 0 replies; 53+ messages in thread
From: Kalle Valo @ 2020-06-05 9:18 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Kees Cook <keescook@chromium.org> writes:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just initialize this variable to NULL,
> and avoid sending garbage by returning.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
To which tree should this go? If something else than wireless-drivers
tree:
Acked-by: Kalle Valo <kvalo@codeaurora.org>
But let me know if you want me to take this.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 03/10] b43: Remove uninitialized_var() usage
2020-06-03 23:31 ` [PATCH 03/10] b43: " Kees Cook
2020-06-04 20:08 ` Nick Desaulniers
@ 2020-06-05 9:20 ` Kalle Valo
1 sibling, 0 replies; 53+ messages in thread
From: Kalle Valo @ 2020-06-05 9:20 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Kees Cook <keescook@chromium.org> writes:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just initialize this variable to NULL,
> and make the (unreachable!) code do a conditional test.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
[...]
> @@ -4256,9 +4256,13 @@ static void b43_nphy_tx_gain_table_upload(struct b43_wldev *dev)
> pga_gain = (table[i] >> 24) & 0xf;
> pad_gain = (table[i] >> 19) & 0x1f;
> if (b43_current_band(dev->wl) == NL80211_BAND_2GHZ)
> - rfpwr_offset = rf_pwr_offset_table[pad_gain];
> + rfpwr_offset = rf_pwr_offset_table
> + ? rf_pwr_offset_table[pad_gain]
> + : 0;
> else
> - rfpwr_offset = rf_pwr_offset_table[pga_gain];
> + rfpwr_offset = rf_pwr_offset_table
> + ? rf_pwr_offset_table[pga_gain]
> + : 0;
To me this is ugly, isn't there a better way to fix this?
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 09/10] treewide: Remove uninitialized_var() usage
[not found] ` <20200603233203.1695403-10-keescook@chromium.org>
` (2 preceding siblings ...)
2020-06-04 13:23 ` Jason Gunthorpe
@ 2020-06-05 9:25 ` Kalle Valo
3 siblings, 0 replies; 53+ messages in thread
From: Kalle Valo @ 2020-06-05 9:25 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft, x86, drbd-dev, linux-block, b43-dev,
netdev, linux-wireless, linux-ide, linux-clk, linux-spi,
linux-mm, clang-built-linux
Kees Cook <keescook@chromium.org> writes:
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings
> (e.g. "unused variable"). If the compiler thinks it is uninitialized,
> either simply initialize the variable or make compiler changes.
>
> I preparation for removing[2] the[3] macro[4], remove all remaining
> needless uses with the following script:
>
> git grep '\buninitialized_var\b' | cut -d: -f1 | sort -u | \
> xargs perl -pi -e \
> 's/\buninitialized_var\(([^\)]+)\)/\1/g;
> s:\s*/\* (GCC be quiet|to make compiler happy) \*/$::g;'
>
> drivers/video/fbdev/riva/riva_hw.c was manually tweaked to avoid
> pathological white-space.
>
> No outstanding warnings were found building allmodconfig with GCC 9.3.0
> for x86_64, i386, arm64, arm, powerpc, powerpc64le, s390x, mips, sparc64,
> alpha, and m68k.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
[...]
> drivers/net/wireless/ath/ath10k/core.c | 2 +-
> drivers/net/wireless/ath/ath6kl/init.c | 2 +-
> drivers/net/wireless/ath/ath9k/init.c | 2 +-
> drivers/net/wireless/broadcom/b43/debugfs.c | 2 +-
> drivers/net/wireless/broadcom/b43/dma.c | 2 +-
> drivers/net/wireless/broadcom/b43/lo.c | 2 +-
> drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
> drivers/net/wireless/broadcom/b43/xmit.c | 12 ++++++------
> .../net/wireless/broadcom/b43legacy/debugfs.c | 2 +-
> drivers/net/wireless/broadcom/b43legacy/main.c | 2 +-
> drivers/net/wireless/intel/iwlegacy/3945.c | 2 +-
> drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
> .../net/wireless/realtek/rtlwifi/rtl8192cu/hw.c | 4 ++--
For wireless drivers:
Acked-by: Kalle Valo <kvalo@codeaurora.org>
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 05/10] ide: Remove uninitialized_var() usage
2020-06-04 20:29 ` Nick Desaulniers
@ 2020-06-15 19:32 ` Kees Cook
0 siblings, 0 replies; 53+ messages in thread
From: Kees Cook @ 2020-06-15 19:32 UTC (permalink / raw)
To: Nick Desaulniers
Cc: LKML, Linus Torvalds, Miguel Ojeda, Alexander Potapenko,
Joe Perches, Andy Whitcroft,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
drbd-dev, linux-block, b43-dev, Network Development,
linux-wireless, linux-ide, linux-clk, linux-spi,
Linux Memory Management List, clang-built-linux,
Sebastian Andrzej Siewior, Saravana Kannan
On Thu, Jun 04, 2020 at 01:29:44PM -0700, Nick Desaulniers wrote:
> On Thu, Jun 4, 2020 at 1:20 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Thu, Jun 04, 2020 at 12:29:17PM -0700, Nick Desaulniers wrote:
> > > On Wed, Jun 3, 2020 at 4:32 PM Kees Cook <keescook@chromium.org> wrote:
> > > >
> > > > Using uninitialized_var() is dangerous as it papers over real bugs[1]
> > > > (or can in the future), and suppresses unrelated compiler warnings (e.g.
> > > > "unused variable"). If the compiler thinks it is uninitialized, either
> > > > simply initialize the variable or make compiler changes. As a precursor
> > > > to removing[2] this[3] macro[4], just remove this variable since it was
> > > > actually unused:
> > > >
> > > > drivers/ide/ide-taskfile.c:232:34: warning: unused variable 'flags' [-Wunused-variable]
> > > > unsigned long uninitialized_var(flags);
> > > > ^
> > > >
> > > > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-glider@google.com/
> > > > [2] https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1TGqCR5vQkCzWJ0QxK6CernOU6eedsudAixw@mail.gmail.com/
> > > > [3] https://lore.kernel.org/lkml/CA+55aFwgbgqhbp1fkxvRKEpzyR5J8n1vKT1VZdz9knmPuXhOeg@mail.gmail.com/
> > > > [4] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/
> > > >
> > > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > >
> > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> >
> > Thanks for the reviews!
> >
> > > Fixes ce1e518190ea ("ide: don't disable interrupts during kmap_atomic()")
> >
> > I originally avoided adding Fixes tags because I didn't want these
> > changes backported into a -stable without -Wmaybe-uninitialized
> > disabled, but in these cases (variable removal), that actually does make
> > sense. Thanks!
>
> Saravana showed me a cool trick for quickly finding commits that
> removed a particular identifier that I find faster than `git blame` or
> vim-fugitive for the purpose of Fixes tags:
> $ git log -S <string> <file>
Ah yes, I always have to look up "-S". Good reminder!
> I've added it to our wiki:
> https://github.com/ClangBuiltLinux/linux/wiki/Command-line-tips-and-tricks#for-finding-which-commit-may-have-removed-a-string-try.
> I should update the first tip; what was your suggestion for
> constraining the search to the current remote?
Ah cool. I've updated it now. It was really to narrow to a "known set of
tags", and Linus's tree's tags always start with "v".
--
Kees Cook
^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2020-06-15 19:33 UTC | newest]
Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 23:31 [PATCH 00/10] Remove uninitialized_var() macro Kees Cook
2020-06-03 23:31 ` [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage Kees Cook
2020-06-04 7:58 ` Thomas Gleixner
2020-06-04 11:41 ` Miguel Ojeda
2020-06-04 14:56 ` Kees Cook
2020-06-04 15:22 ` Miguel Ojeda
2020-06-04 14:34 ` Kees Cook
2020-06-04 21:39 ` Thomas Gleixner
2020-06-04 22:39 ` Kees Cook
2020-06-03 23:31 ` [PATCH 02/10] drbd: " Kees Cook
2020-06-04 19:56 ` Nick Desaulniers
2020-06-03 23:31 ` [PATCH 03/10] b43: " Kees Cook
2020-06-04 20:08 ` Nick Desaulniers
2020-06-04 20:18 ` Kees Cook
2020-06-04 20:25 ` Nick Desaulniers
2020-06-05 9:20 ` Kalle Valo
2020-06-03 23:31 ` [PATCH 04/10] rtlwifi: rtl8192cu: " Kees Cook
2020-06-04 20:16 ` Nick Desaulniers
2020-06-05 9:18 ` Kalle Valo
2020-06-03 23:31 ` [PATCH 05/10] ide: " Kees Cook
2020-06-04 19:29 ` Nick Desaulniers
2020-06-04 20:20 ` Kees Cook
2020-06-04 20:29 ` Nick Desaulniers
2020-06-15 19:32 ` Kees Cook
2020-06-04 20:58 ` Sedat Dilek
2020-06-03 23:31 ` [PATCH 06/10] clk: st: " Kees Cook
2020-06-04 4:38 ` Stephen Boyd
2020-06-03 23:32 ` [PATCH 07/10] spi: davinci: " Kees Cook
2020-06-04 19:40 ` Nick Desaulniers
2020-06-03 23:32 ` [PATCH 08/10] checkpatch: Remove awareness of uninitialized_var() macro Kees Cook
2020-06-04 0:02 ` Joe Perches
2020-06-04 1:40 ` Kees Cook
2020-06-04 1:47 ` Joe Perches
2020-06-04 2:44 ` Kees Cook
2020-06-04 2:53 ` Sedat Dilek
2020-06-04 3:46 ` Kees Cook
2020-06-03 23:32 ` [PATCH 10/10] compiler: Remove " Kees Cook
2020-06-04 0:00 ` Bart Van Assche
2020-06-04 0:50 ` Miguel Ojeda
2020-06-04 1:23 ` [PATCH 00/10] " Sedat Dilek
2020-06-04 1:44 ` Kees Cook
2020-06-04 1:46 ` Sedat Dilek
2020-06-04 3:33 ` Nathan Chancellor
2020-06-04 7:26 ` Sedat Dilek
2020-06-04 14:27 ` Kees Cook
[not found] ` <20200603233203.1695403-10-keescook@chromium.org>
2020-06-04 3:33 ` [PATCH 09/10] treewide: Remove uninitialized_var() usage Nathan Chancellor
2020-06-04 4:02 ` Kees Cook
2020-06-04 10:45 ` Leon Romanovsky
2020-06-04 13:23 ` Jason Gunthorpe
2020-06-04 14:59 ` Kees Cook
2020-06-04 17:57 ` Jason Gunthorpe
2020-06-04 19:09 ` Geert Uytterhoeven
2020-06-05 9:25 ` Kalle Valo
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).