| Message ID | 20200603233203.1695403-1-keescook@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Remove uninitialized_var() macro | expand |
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.
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.
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 -
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
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]
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
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 -
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
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
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! :)
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!
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
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
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>