Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues: - [High] Early return misses EC hardware acknowledgement, potentially causing an interrupt storm. - [High] Deadlock between EC threaded IRQ handler and UCSI notifier causing inevitable timeout. - [High] Out-of-bounds array access due to dynamic `num_ports` update surpassing allocated size. - [Medium] Undefined Behavior due to shift operation with negative exponent. -- commit 73f2964a00c46ffc80bf958e8a195aee63f9493d Author: Dmitry Baryshkov <[email protected]> usb: typec: ucsi: huawei-gaokun: pass down HPD_IRQ events Pass IRQ_HPD events from the EC to the HPD bridge, letting those to be delivered to the DisplayPort driver. > diff --git a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c > b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c > --- a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c > +++ b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c > @@ -299,10 +299,13 @@ static void gaokun_ucsi_handle_altmode(struct > gaokun_ucsi_port *port) [ ... ] > /* UCSI callback .connector_status() have set orientation */ > if (port->bridge) > - drm_aux_hpd_bridge_notify(&port->bridge->dev, > - port->hpd_state ? > - connector_status_connected : > - connector_status_disconnected); > + drm_aux_hpd_bridge_notify_extra(&port->bridge->dev, > + port->hpd_state ? > + connector_status_connected : > + connector_status_disconnected, > + port->hpd_irq ? > + DRM_CONNECTOR_DP_IRQ_HPD : > + DRM_CONNECTOR_NO_EXTRA_STATUS); > > gaokun_ec_ucsi_pan_ack(uec->ec, port->idx); [Severity: High] This isn't a bug introduced by this patch, but I noticed a potential issue in gaokun_ucsi_handle_altmode() regarding the early return. If an EC event has a port index that surpasses the UCSI capability connector bounds, the function warns and returns early. Because the event has already been consumed by the hardware, does skipping the gaokun_ec_ucsi_pan_ack() call here cause the EC firmware to infinitely resend the interrupt or stall subsequent operations? [Severity: Medium] This is a pre-existing issue, but looking at the gaokun_ec_ucsi_pan_ack() call at the end of this function: drivers/platform/arm64/huawei-gaokun-ec.c:gaokun_ec_ucsi_pan_ack() { ... u8 data = 1 << port_id; if (port_id == GAOKUN_UCSI_NO_PORT_UPDATE) data = 0; ... } When gaokun_ucsi_altmode_notify_ind() explicitly calls this with GAOKUN_UCSI_NO_PORT_UPDATE (which equals -1), the shift operation evaluates unconditionally before the safety check. Does this cause undefined behavior by shifting with a negative exponent? > } [Severity: High] This is another pre-existing issue, but I noticed a potential deadlock between the EC threaded IRQ handler and the UCSI notifier. If the shared gaokun_ec_irq_handler() invokes gaokun_ucsi_notify() through the blocking notifier, it can call gaokun_ucsi_handle_no_usb_event() which waits on a completion: drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c:gaokun_ucsi_handle_no_usb_event() { if (!wait_for_completion_timeout(&port->usb_ack, 2 * HZ)) { ... } } Since the single EC event thread is blocked inside the notifier chain waiting for this completion, does this prevent the system from extracting the upcoming EC_EVENT_USB to trigger the completion, imposing a guaranteed timeout delay? [Severity: High] This isn't a bug introduced by this patch, but there might be an out-of-bounds array access in gaokun_ucsi_refresh() when updating port data. During initialization, the uec->ports array is allocated based on the initial number of ports: drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c:gaokun_ucsi_ports_init() { uec->ports = devm_kcalloc(dev, num_ports, sizeof(*uec->ports), GFP_KERNEL); ... } In gaokun_ucsi_refresh(), when an interrupt occurs, it reads the new limit and uses it: drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c:gaokun_ucsi_refresh() { uec->num_ports = ureg.num_ports; ... gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data); } If the EC dynamically reports a larger number of ports than the original allocation, does this bypass the boundary check and overwrite unallocated memory? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
