linux-kernel - Re: Regression: Failed boots bisected to 4cd13c21b207 "softirq: Let ksoftirqd do its job"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 Nov 2016 08:09:58 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     Brian Starkey <brian.starkey@....com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Potapenko <glider@...gle.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: Regression: Failed boots bisected to 4cd13c21b207 "softirq: Let
 ksoftirqd do its job"

>
> Looks like there's a few similarly named devices and drivers. Mine is
> an SMSC LAN91C111 using the smc91x driver in
> drivers/net/ethernet/smsc/smc91x.c, rather than smc911x.c. So the
> interrupt handler is smc_interrupt()
>
> CONFIG_ARCH_PXA is not set, nor is SMC_USE_PXA_DMA or SMC_USE_DMA.

Oh right.

static irqreturn_t smc_interrupt(int irq, void *dev_id)
{
...
mask = SMC_GET_INT_MASK(lp);
SMC_SET_INT_MASK(lp, 0);

loop()

...
} else if (status & IM_ALLOC_INT) {
    DBG(3, dev, "Allocation irq\n");
    tasklet_hi_schedule(&lp->tx_task);
    mask &= ~IM_ALLOC_INT;
}
...

SMC_SET_INT_MASK(lp, mask);
spin_unlock(&lp->lock);
        /*
         * We return IRQ_HANDLED unconditionally here even if there was
         * nothing to do.  There is a possibility that a packet might
         * get enqueued into the chip right after TX_EMPTY_INT is raised
         * but just before the CPU acknowledges the IRQ.
         * Better take an unneeded IRQ in some occasions than complexifying
         * the code for all cases.
         */
        return IRQ_HANDLED;
}

Could you trace mask value, it looks we loop and never acknowledge
some interrupt status.

Maybe driver depends on tasklet_hi_schedule() triggers
smc_hardware_send_pkt() really really soon.

Oh, but smc_hardware_send_pkt() uses a spin_trylock() at its
beginning, then later a spin_lock() hidden in SMC_ENABLE_INT(),
this looks racy.


diff --git a/drivers/net/ethernet/smsc/smc91x.c
b/drivers/net/ethernet/smsc/smc91x.c
index 73212590d04a..32abb0a084ec 100644
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -617,14 +617,13 @@ static void smc_hardware_send_pkt(unsigned long data)

        /* queue the packet for TX */
        SMC_SET_MMU_CMD(lp, MC_ENQUEUE);
+       SMC_SET_INT_MASK(lp, SMC_GET_INT_MASK(lp) | IM_TX_INT |
IM_TX_EMPTY_INT);
        smc_special_unlock(&lp->lock, flags);

        netif_trans_update(dev);
        dev->stats.tx_packets++;
        dev->stats.tx_bytes += len;

-       SMC_ENABLE_INT(lp, IM_TX_INT | IM_TX_EMPTY_INT);
-
 done:  if (!THROTTLE_TX_PKTS)
                netif_wake_queue(dev);