2018-08-27 19:44:30

by Tomas Winkler

[permalink] [raw]
Subject: [char-misc for 4.19 0/2] mei: fix suspend/hibernation regression

A kernel panic during suspend/hibernation was reported on kernel 4.17.

RIP: mei_cl_set_disconnected+0x5/0x260[mei]
Call trace:
mei_cl_all_disconnect+0x22/0x30
mei_reset+0x194/0x250
__synchronize_hardirq+0x43/0x50
_cond_resched+0x15/0x30
mei_me_intr_clear+0x20/0x100
mei_stop+0x76/0xb0
mei_me_shutdown+0x3f/0x80
pci_device_shutdown+0x34/0x60
kernel_restart+0x0e/0x30

Bugzilla entries:
https://bugzilla.redhat.com/show_bug.cgi?id=1597481
https://bugzilla.kernel.org/show_bug.cgi?id=200455

The regression is caused by a combination of two coding errors
in two separate patches. In addition, this currently only happens
on platforms with AMT watchdog enabled that systemd holds.

Both patches bellow should be applied.

Tomas Winkler (2):
mei: bus: fix hw module get/put balance
mei: bus: need to unlink client before freeing

drivers/misc/mei/bus.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

--
2.14.4



2018-08-27 19:44:33

by Tomas Winkler

[permalink] [raw]
Subject: [char-misc for 4.9 2/2] mei: bus: need to unlink client before freeing

In case a client fails to connect in mei_cldev_enable(), the
caller won't call the mei_cldev_disable leaving the client
in a linked stated. Upon driver unload the client structure
will be freed in mei_cl_bus_dev_release(), leaving a stale pointer
on a fail_list. This will eventually end up in crash
during power down flow in mei_cl_set_disonnected().

RIP: mei_cl_set_disconnected+0x5/0x260[mei]
Call trace:
mei_cl_all_disconnect+0x22/0x30
mei_reset+0x194/0x250
__synchronize_hardirq+0x43/0x50
_cond_resched+0x15/0x30
mei_me_intr_clear+0x20/0x100
mei_stop+0x76/0xb0
mei_me_shutdown+0x3f/0x80
pci_device_shutdown+0x34/0x60
kernel_restart+0x0e/0x30

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200455
Fixes: 'c110cdb17148 ("mei: bus: make a client pointer always available")'
Cc: <[email protected]> 4.10+
Tested-by: Georg Müller <[email protected]>
Signed-off-by: Tomas Winkler <[email protected]>
---
drivers/misc/mei/bus.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 13c6c9a2248a..fc3872fe7b25 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -521,17 +521,15 @@ int mei_cldev_enable(struct mei_cl_device *cldev)

cl = cldev->cl;

+ mutex_lock(&bus->device_lock);
if (cl->state == MEI_FILE_UNINITIALIZED) {
- mutex_lock(&bus->device_lock);
ret = mei_cl_link(cl);
- mutex_unlock(&bus->device_lock);
if (ret)
- return ret;
+ goto out;
/* update pointers */
cl->cldev = cldev;
}

- mutex_lock(&bus->device_lock);
if (mei_cl_is_connected(cl)) {
ret = 0;
goto out;
@@ -875,12 +873,13 @@ static void mei_cl_bus_dev_release(struct device *dev)

mei_me_cl_put(cldev->me_cl);
mei_dev_bus_put(cldev->bus);
+ mei_cl_unlink(cldev->cl);
kfree(cldev->cl);
kfree(cldev);
}

static const struct device_type mei_cl_device_type = {
- .release = mei_cl_bus_dev_release,
+ .release = mei_cl_bus_dev_release,
};

/**
--
2.14.4


2018-08-27 19:44:53

by Tomas Winkler

[permalink] [raw]
Subject: [char-misc for 4.9 1/2] mei: bus: fix hw module get/put balance

In case the device is not connected it doesn't 'get'
hw module and hence should not 'put' it on disable.

Cc: <[email protected]> 4.16+
Fixes:'commit 257355a44b99 ("mei: make module referencing local to the bus.c")'
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200455
Tested-by: Georg Müller <[email protected]>
Signed-off-by: Tomas Winkler <[email protected]>
---
drivers/misc/mei/bus.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 7bba62a72921..13c6c9a2248a 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -616,9 +616,8 @@ int mei_cldev_disable(struct mei_cl_device *cldev)
if (err < 0)
dev_err(bus->dev, "Could not disconnect from the ME client\n");

-out:
mei_cl_bus_module_put(cldev);
-
+out:
/* Flush queues and remove any pending read */
mei_cl_flush_queues(cl, NULL);
mei_cl_unlink(cl);
--
2.14.4