On Fri, 2022-02-11 at 16:37 -0800, Kees Cook wrote: [] > Well, I'm for #1, though perhaps with a more narrow view: some semantics > are just weird/surprising. ;) Until I first encountered this warning a > few years ago when working on GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, I didn't > even know putting declarations there was valid C. ;) > > Whack-a-mole is part of the work to make these kinds of treewide > changes, but the hope is to find as much of it ahead of time as > possible. And, no, I have no interest in security theater. (Not > everything has equal levels of effectiveness, of course, but I don't > think that's what you're saying.) > > > In fairness I'd have no objection to that patch if it came with a convincing > > justification, but that is so far very much lacking. My aim here is not to > > be a change-averse Luddite, but to try to find a compromise where I can > > actually have some confidence in such changes being made. Let's not start > > pretending that 3 100ml bottles of shampoo are somehow "safer" than a 300ml > > bottle of shampoo... > > Sure. I think I am trying to take a pragmatic approach here, which is > that gaining auto-var-init is a big deal (killing entire classes of > vulnerabilities), but it comes with an annoying compiler bug (that we do > get a warning about) for an uncommon code pattern that is easy to fix. > So rather than delaying the defense until the sharp edge on the compiler > gets fixed, I'd like to get the rest rolling while the edge is filed. coccinelle would probably find most all of them. $ cat switch_define.cocci @@ expression e; type t; identifier i; @@ switch (e) { * t i; } $ spatch --very-quiet -sp-file switch_define.cocci . 13831 files match diff -u -p ./arch/arc/kernel/unwind.c /tmp/nothing/arch/arc/kernel/unwind.c --- ./arch/arc/kernel/unwind.c +++ /tmp/nothing/arch/arc/kernel/unwind.c @@ -718,7 +718,6 @@ static int processCFI(const u8 *start, c } for (ptr.p8 = start; result && ptr.p8 < end;) { switch (*ptr.p8 >> 6) { - uleb128_t value; case 0: opcode = *ptr.p8++; diff -u -p ./arch/arm/kernel/module-plts.c /tmp/nothing/arch/arm/kernel/module-plts.c --- ./arch/arm/kernel/module-plts.c +++ /tmp/nothing/arch/arm/kernel/module-plts.c @@ -124,7 +124,6 @@ static bool is_zero_addend_relocation(El * PC bias into account, i.e., -8 for ARM and -4 for Thumb2. */ switch (ELF32_R_TYPE(rel->r_info)) { - u16 upper, lower; case R_ARM_THM_CALL: case R_ARM_THM_JUMP24: diff -u -p ./arch/mips/mti-malta/malta-init.c /tmp/nothing/arch/mips/mti-malta/malta-init.c --- ./arch/mips/mti-malta/malta-init.c +++ /tmp/nothing/arch/mips/mti-malta/malta-init.c @@ -168,7 +168,6 @@ void __init prom_init(void) } switch (mips_revision_sconid) { - u32 start, map, mask, data; case MIPS_REVISION_SCON_GT64120: /* diff -u -p ./arch/parisc/kernel/inventory.c /tmp/nothing/arch/parisc/kernel/inventory.c --- ./arch/parisc/kernel/inventory.c +++ /tmp/nothing/arch/parisc/kernel/inventory.c @@ -235,7 +235,6 @@ pat_query_module(ulong pcell_loc, ulong #ifdef DEBUG_PAT /* dump what we see so far... */ switch (PAT_GET_ENTITY(dev->mod_info)) { - pdc_pat_cell_mod_maddr_block_t io_pdc_cell; unsigned long i; case PAT_ENTITY_PROC: diff -u -p ./arch/powerpc/xmon/xmon.c /tmp/nothing/arch/powerpc/xmon/xmon.c --- ./arch/powerpc/xmon/xmon.c +++ /tmp/nothing/arch/powerpc/xmon/xmon.c @@ -1528,7 +1528,6 @@ bpt_cmds(void) cmd = inchar(); switch (cmd) { - static const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n"; int mode; case 'd': /* bd - hardware data breakpoint */ if (xmon_is_ro) { diff -u -p ./arch/um/drivers/ubd_kern.c /tmp/nothing/arch/um/drivers/ubd_kern.c --- ./arch/um/drivers/ubd_kern.c +++ /tmp/nothing/arch/um/drivers/ubd_kern.c @@ -1407,7 +1407,6 @@ static int ubd_ioctl(struct block_device u16 ubd_id[ATA_ID_WORDS]; switch (cmd) { - struct cdrom_volctrl volume; case HDIO_GET_IDENTITY: memset(&ubd_id, 0, ATA_ID_WORDS * 2); ubd_id[ATA_ID_CYLS] = ubd_dev->size / (128 * 32 * 512); diff -u -p ./arch/x86/kernel/cpu/cyrix.c /tmp/nothing/arch/x86/kernel/cpu/cyrix.c --- ./arch/x86/kernel/cpu/cyrix.c +++ /tmp/nothing/arch/x86/kernel/cpu/cyrix.c @@ -224,7 +224,6 @@ static void init_cyrix(struct cpuinfo_x8 */ switch (dir0_msn) { - unsigned char tmp; case 0: /* Cx486SLC/DLC/SRx/DRx */ p = Cx486_name[dir0_lsn & 7]; diff -u -p ./arch/x86/pci/irq.c /tmp/nothing/arch/x86/pci/irq.c --- ./arch/x86/pci/irq.c +++ /tmp/nothing/arch/x86/pci/irq.c @@ -959,7 +959,6 @@ static __init int intel_router_probe(str return 0; switch (device) { - u8 rid; case PCI_DEVICE_ID_INTEL_82375: r->name = "PCEB/ESC"; r->get = pirq_esc_get; diff -u -p ./drivers/acpi/arm64/iort.c /tmp/nothing/drivers/acpi/arm64/iort.c --- ./drivers/acpi/arm64/iort.c +++ /tmp/nothing/drivers/acpi/arm64/iort.c @@ -1667,7 +1667,6 @@ phys_addr_t __init acpi_iort_dma_get_max break; switch (node->type) { - struct acpi_iort_named_component *ncomp; struct acpi_iort_root_complex *rc; phys_addr_t local_limit; diff -u -p ./drivers/infiniband/hw/irdma/hw.c /tmp/nothing/drivers/infiniband/hw/irdma/hw.c --- ./drivers/infiniband/hw/irdma/hw.c +++ /tmp/nothing/drivers/infiniband/hw/irdma/hw.c @@ -267,7 +267,6 @@ static void irdma_process_aeq(struct ird } switch (info->ae_id) { - struct irdma_cm_node *cm_node; case IRDMA_AE_LLP_CONNECTION_ESTABLISHED: cm_node = iwqp->cm_node; if (cm_node->accept_pend) { diff -u -p ./drivers/infiniband/hw/irdma/utils.c /tmp/nothing/drivers/infiniband/hw/irdma/utils.c --- ./drivers/infiniband/hw/irdma/utils.c +++ /tmp/nothing/drivers/infiniband/hw/irdma/utils.c @@ -1212,7 +1212,6 @@ enum irdma_status_code irdma_hw_modify_q return status; switch (m_info->next_iwarp_state) { - struct irdma_gen_ae_info ae_info; case IRDMA_QP_STATE_RTS: case IRDMA_QP_STATE_IDLE: diff -u -p ./drivers/input/serio/hil_mlc.c /tmp/nothing/drivers/input/serio/hil_mlc.c --- ./drivers/input/serio/hil_mlc.c +++ /tmp/nothing/drivers/input/serio/hil_mlc.c @@ -633,7 +633,6 @@ static int hilse_donode(hil_mlc *mlc) node = hil_mlc_se + mlc->seidx; switch (node->act) { - int rc; hil_packet pack; case HILSE_FUNC: diff -u -p ./drivers/rtc/rtc-pcf8523.c /tmp/nothing/drivers/rtc/rtc-pcf8523.c --- ./drivers/rtc/rtc-pcf8523.c +++ /tmp/nothing/drivers/rtc/rtc-pcf8523.c @@ -242,7 +242,6 @@ static int pcf8523_param_get(struct devi int ret; switch(param->param) { - u32 value; case RTC_PARAM_BACKUP_SWITCH_MODE: ret = regmap_read(pcf8523->regmap, PCF8523_REG_CONTROL3, &value); @@ -281,7 +280,6 @@ static int pcf8523_param_set(struct devi struct pcf8523 *pcf8523 = dev_get_drvdata(dev); switch(param->param) { - u8 mode; case RTC_PARAM_BACKUP_SWITCH_MODE: switch (param->uvalue) { case RTC_BSM_DISABLED: diff -u -p ./drivers/rtc/rtc-rv3028.c /tmp/nothing/drivers/rtc/rtc-rv3028.c --- ./drivers/rtc/rtc-rv3028.c +++ /tmp/nothing/drivers/rtc/rtc-rv3028.c @@ -523,7 +523,6 @@ static int rv3028_param_get(struct devic int ret; switch(param->param) { - u32 value; case RTC_PARAM_BACKUP_SWITCH_MODE: ret = regmap_read(rv3028->regmap, RV3028_BACKUP, &value); @@ -556,7 +555,6 @@ static int rv3028_param_set(struct devic struct rv3028_data *rv3028 = dev_get_drvdata(dev); switch(param->param) { - u8 mode; case RTC_PARAM_BACKUP_SWITCH_MODE: switch (param->uvalue) { case RTC_BSM_DISABLED: diff -u -p ./drivers/rtc/rtc-rv3032.c /tmp/nothing/drivers/rtc/rtc-rv3032.c --- ./drivers/rtc/rtc-rv3032.c +++ /tmp/nothing/drivers/rtc/rtc-rv3032.c @@ -401,7 +401,6 @@ static int rv3032_param_get(struct devic int ret; switch(param->param) { - u32 value; case RTC_PARAM_BACKUP_SWITCH_MODE: ret = regmap_read(rv3032->regmap, RV3032_PMU, &value); @@ -435,7 +434,6 @@ static int rv3032_param_set(struct devic struct rv3032_data *rv3032 = dev_get_drvdata(dev); switch(param->param) { - u8 mode; case RTC_PARAM_BACKUP_SWITCH_MODE: if (rv3032->trickle_charger_set) return -EINVAL; diff -u -p ./drivers/s390/net/ctcm_fsms.c /tmp/nothing/drivers/s390/net/ctcm_fsms.c --- ./drivers/s390/net/ctcm_fsms.c +++ /tmp/nothing/drivers/s390/net/ctcm_fsms.c @@ -1432,7 +1432,6 @@ static void ctcmpc_chx_rx(fsm_instance * again: switch (fsm_getstate(grp->fsm)) { - int rc, dolock; case MPCG_STATE_FLOWC: case MPCG_STATE_READY: if (ctcm_checkalloc_buffer(ch)) diff -u -p ./lib/test_stackinit.c /tmp/nothing/lib/test_stackinit.c --- ./lib/test_stackinit.c +++ /tmp/nothing/lib/test_stackinit.c @@ -398,7 +398,6 @@ static int noinline __leaf_switch_none(i * This is intentionally unreachable. To silence the * warning, build with -Wno-switch-unreachable */ - uint64_t var; case 1: target_start = &var; diff -u -p ./net/mpls/af_mpls.c /tmp/nothing/net/mpls/af_mpls.c --- ./net/mpls/af_mpls.c +++ /tmp/nothing/net/mpls/af_mpls.c @@ -1621,7 +1621,6 @@ static int mpls_dev_notify(struct notifi return NOTIFY_OK; switch (event) { - int err; case NETDEV_DOWN: err = mpls_ifdown(dev, event); diff -u -p ./scripts/recordmcount.c /tmp/nothing/scripts/recordmcount.c --- ./scripts/recordmcount.c +++ /tmp/nothing/scripts/recordmcount.c @@ -488,7 +488,6 @@ static int do_file(char const *const fna w2 = w2nat; w8 = w8nat; switch (ehdr->e_ident[EI_DATA]) { - static unsigned int const endian = 1; default: fprintf(stderr, "unrecognized ELF data encoding %d: %s\n", ehdr->e_ident[EI_DATA], fname); diff -u -p ./tools/testing/selftests/powerpc/nx-gzip/gunz_test.c /tmp/nothing/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c --- ./tools/testing/selftests/powerpc/nx-gzip/gunz_test.c +++ /tmp/nothing/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c @@ -805,7 +805,6 @@ ok_cc3: */ switch (sfbt) { - int dhtlen; case 0x0: /* Deflate final EOB received */ diff -u -p ./tools/testing/selftests/proc/read.c /tmp/nothing/tools/testing/selftests/proc/read.c --- ./tools/testing/selftests/proc/read.c +++ /tmp/nothing/tools/testing/selftests/proc/read.c @@ -91,7 +91,6 @@ static void f(DIR *d, unsigned int level assert(!streq(de->d_name, "..")); switch (de->d_type) { - DIR *dd; int fd; case DT_REG: $