linux-kernel - Re: [PATCH] irq/irq

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]

Message-ID: <alpine.DEB.2.21.1811091108001.1519@nanos.tec.linutronix.de>
Date:   Fri, 9 Nov 2018 11:19:16 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
cc:     Bartosz Golaszewski <brgl@...ev.pl>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] irq/irq_sim: add locking

On Thu, 8 Nov 2018, Uwe Kleine-König wrote:
> On Thu, Nov 08, 2018 at 05:47:48PM +0100, Bartosz Golaszewski wrote:
> > @@ -74,6 +74,7 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)
> >  	}
> >  
> >  	init_irq_work(&sim->work_ctx.work, irq_sim_handle_irq);
> > +	mutex_init(&sim->lock);
> >  	sim->irq_count = num_irqs;
> >  
> >  	return sim->irq_base;
> > @@ -142,10 +143,14 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init);
> >   */
> >  void irq_sim_fire(struct irq_sim *sim, unsigned int offset)
> >  {
> > +	mutex_lock(&sim->lock);
> > +
> >  	if (sim->irqs[offset].enabled) {
> >  		sim->work_ctx.irq = irq_sim_irqnum(sim, offset);
> >  		irq_work_queue(&sim->work_ctx.work);
> >  	}
> > +
> > +	mutex_unlock(&sim->lock);
> 
> This doesn't fix the issue I think. irq_work_queue() only schedules the
> work function. If after irq_sim_fire() returned but before the worker
> runs another irq_sim_fire() is issued the value is still overwritten.

Right. So the obvious solution is to avoid the irq number store and use a
bitfield instead.

struct irq_sim_work_ctx {
       ...
       unsigned long	pending;
};

fire(sim, offset)
{
	if (!sim->irqs[offset].enabled)
		return;

	set_bit(offset, &sim->work_ctx.pending);
	....

and in the work handler do:

handle(work)
{
	struct irq_sim_work_ctx *ctx = container_of(work,....);

	while (ctx->pending) {
		offs = ffs(ctx->pending);
		clr_bit(offs, &ctx->pending);
		handle_simple_irq(offs);
	}
}

Or something like that.

Thanks,

	tglx