On 2019-07-30 2:07 p.m., Heikki Krogerus wrote:
On Mon, Jul 29, 2019 at 10:31:04AM -0700, Guenter Roeck wrote:
On Mon, Jul 29, 2019 at 05:04:57PM +0300, Heikki Krogerus wrote:
Hi,
On Wed, Jul 24, 2019 at 09:30:37PM -0700, Guenter Roeck wrote:
TCPM may receive PD messages associated with unknown or unsupported
alternate modes. If that happens, calls to typec_match_altmode()
will return NULL. The tcpm code does not currently take this into
account. This results in crashes.
Unable to handle kernel NULL pointer dereference at virtual address 000001f0
pgd = 41dad9a1
[000001f0] *pgd=00000000
Internal error: Oops: 5 [#1] THUMB2
Modules linked in: tcpci tcpm
CPU: 0 PID: 2338 Comm: kworker/u2:0 Not tainted 5.1.18-sama5-armv7-r2 #6
Hardware name: Atmel SAMA5
Workqueue: 2-0050 tcpm_pd_rx_handler [tcpm]
PC is at typec_altmode_attention+0x0/0x14
LR is at tcpm_pd_rx_handler+0xa3b/0xda0 [tcpm]
...
[<c03fbee8>] (typec_altmode_attention) from [<bf8030fb>]
(tcpm_pd_rx_handler+0xa3b/0xda0 [tcpm])
[<bf8030fb>] (tcpm_pd_rx_handler [tcpm]) from [<c012082b>]
(process_one_work+0x123/0x2a8)
[<c012082b>] (process_one_work) from [<c0120a6d>]
(worker_thread+0xbd/0x3b0)
[<c0120a6d>] (worker_thread) from [<c012431f>] (kthread+0xcf/0xf4)
[<c012431f>] (kthread) from [<c01010f9>] (ret_from_fork+0x11/0x38)
Ignore PD messages if the asociated alternate mode is not supported.
Reported-by: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>
Cc: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>
Fixes: e9576fe8e605c ("usb: typec: tcpm: Support for Alternate Modes")
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
Taking a stab at the problem. I don't really know if this is the correct
fix, or even if my understanding of the problem is correct, thus marking
the patch as RFC.
My guess is that typec_match_altmode() is the real culprit. We can't
rely on the partner mode index number when identifying the port alt
mode.
Douglas, can you test the attached hack instead of this patch?
thanks,
--
heikki
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index ec525811a9eb..033dc097ba83 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1067,12 +1067,11 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
modep = &port->mode_data;
- adev = typec_match_altmode(port->port_altmode, ALTMODE_DISCOVERY_MAX,
- PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
-
pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX,
PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
+ adev = (void *)typec_altmode_get_partner(pdev);
+
I understand that typec_altmode_get_partner() returns a const *;
maybe adev should be declared as const struct typec_altmode *
instead of using a typecast.
Yes...
Also, typec_altmode_get_partner() can return NULL as well if pdev is NULL.
Is it guaranteed that typec_match_altmode() never returns NULL for pdev ?
...and probable no. But I don't think we can receive Attention to a
mode that hasn't been entered.
I'm not proposing that as a patch. It's just a hunch. That's why I'm
calling it a "hack". Before we prepare anything finalized, I would
like to here from Douglas if he's able to test that or not?
Hi,
I'm an ocean away from my test rig at the moment, won't be back home till
after August 10. Maybe I can set up something here as I have one OM13588.
Will try later today. Also, switching from an sama5d2_xplained to an Acme
Systems Arietta which is at91sam9g25 based to run Linux. So it won't be
exactly the same hardware.
Doug Gilbert
