netdev - Re: [PATCH 3/3] ehea: do not dereference possible NULL port

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20120418124252.GA2455@oc1711230544.ibm.com>
Date:	Wed, 18 Apr 2012 09:42:52 -0300
From:	Thadeu Lima de Souza Cascardo <cascardo@...ux.vnet.ibm.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, joe@...ches.com
Subject: Re: [PATCH 3/3] ehea: do not dereference possible NULL port

On Tue, Apr 17, 2012 at 10:29:12PM -0400, David Miller wrote:
> From: Thadeu Lima de Souza Cascardo <cascardo@...ux.vnet.ibm.com>
> Date: Tue, 17 Apr 2012 11:41:55 -0300
> 
> > port may be NULL when we receive an interrupt too early in the probe.
> > 
> > commit 8c4877a4128e7931077b024a891a4b284d8756a3, in particular, has
> > introduced this problem in the case of port state change interrupt.
> > 
> > This causes crashes in some situations:
> 
> Never fix this kind of bug like this.  Do not make a tasklet or
> an irq handler pay the price of the probe not doing things in the
> right order.
> 
> Do not register asynchronous threads of control until all the
> state is setup properly.
> 
> That's the right way to fix this problem.
> 

Sure. That was in my plans too.

However, at first, I suspected this interrupt may have come from some
other port from ehca, for example, considering a possible firmware bug.
After more investigation, I found out that the real problem was some
physical ports in some odd states, that caused the IRQs before the ports
were enabled. Note that this is not really supposed to happen.

So, consider the possibility that we receive, for a reason like a
firmware bug, an interrupt is received and the EQE indicates a port
number we don't know about. We oops even if we have registered the IRQ
after enabling the ports.

So, I think it's better to be safe to just check for a NULL pointer in
the interrupt (which was being checked either way, just not in the right
place).

So, in fact, I just moved a pointer check because the pointer is now
being used when it wasn't before, due to the commit I referred to.

Would you apply this patch if I include the other one, registering the
IRQ later in the probe process?

Thanks for the comment.
Cascardo.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html