Commit cbe6c3a8 authored by Manuel Ullmann's avatar Manuel Ullmann Committed by David S. Miller

net: atlantic: invert deep par in pm functions, preventing null derefs

This will reset deeply on freeze and thaw instead of suspend and
resume and prevent null pointer dereferences of the uninitialized ring
0 buffer while thawing.

The impact is an indefinitely hanging kernel. You can't switch
consoles after this and the only possible user interaction is SysRq.

BUG: kernel NULL pointer dereference
RIP: 0010:aq_ring_rx_fill+0xcf/0x210 [atlantic]
aq_vec_init+0x85/0xe0 [atlantic]
aq_nic_init+0xf7/0x1d0 [atlantic]
atl_resume_common+0x4f/0x100 [atlantic]
pci_pm_thaw+0x42/0xa0

resolves in aq_ring.o to

```
0000000000000ae0 <aq_ring_rx_fill>:
{
/* ... */
 baf:	48 8b 43 08          	mov    0x8(%rbx),%rax
 		buff->flags = 0U; /* buff is NULL */
```

The bug has been present since the introduction of the new pm code in
8aaa112a ("net: atlantic: refactoring pm logic") and was hidden
until 8ce84271 ("net: atlantic: changes for multi-TC support"),
which refactored the aq_vec_{free,alloc} functions into
aq_vec_{,ring}_{free,alloc}, but is technically not wrong. The
original functions just always reinitialized the buffers on S3/S4. If
the interface is down before freezing, the bug does not occur. It does
not matter, whether the initrd contains and loads the module before
thawing.

So the fix is to invert the boolean parameter deep in all pm function
calls, which was clearly intended to be set like that.

First report was on Github [1], which you have to guess from the
resume logs in the posted dmesg snippet. Recently I posted one on
Bugzilla [2], since I did not have an AQC device so far.

#regzbot introduced: 8ce84271
#regzbot from: koo5 <kolman.jindrich@gmail.com>
#regzbot monitor: https://github.com/Aquantia/AQtion/issues/32

Fixes: 8aaa112a ("net: atlantic: refactoring pm logic")
Link: https://github.com/Aquantia/AQtion/issues/32 [1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215798 [2]
Cc: stable@vger.kernel.org
Reported-by: default avatarkoo5 <kolman.jindrich@gmail.com>
Signed-off-by: default avatarManuel Ullmann <labre@posteo.de>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent d94ef51d
...@@ -444,22 +444,22 @@ static int atl_resume_common(struct device *dev, bool deep) ...@@ -444,22 +444,22 @@ static int atl_resume_common(struct device *dev, bool deep)
static int aq_pm_freeze(struct device *dev) static int aq_pm_freeze(struct device *dev)
{ {
return aq_suspend_common(dev, false); return aq_suspend_common(dev, true);
} }
static int aq_pm_suspend_poweroff(struct device *dev) static int aq_pm_suspend_poweroff(struct device *dev)
{ {
return aq_suspend_common(dev, true); return aq_suspend_common(dev, false);
} }
static int aq_pm_thaw(struct device *dev) static int aq_pm_thaw(struct device *dev)
{ {
return atl_resume_common(dev, false); return atl_resume_common(dev, true);
} }
static int aq_pm_resume_restore(struct device *dev) static int aq_pm_resume_restore(struct device *dev)
{ {
return atl_resume_common(dev, true); return atl_resume_common(dev, false);
} }
static const struct dev_pm_ops aq_pm_ops = { static const struct dev_pm_ops aq_pm_ops = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment