Re: [PATCH] usb: typec: ucsi: Set ucsi->connector before enabling notifications

[Uttkarsh Aggarwal]

On Wed, Sep 23, 2026 at 12:54 PM Peter Chen <peter.chen@xxxxxxxxxxxxxxxx> wrote:
>
> On 26-09-23 11:34:34, Uttkarsh Aggarwal wrote:
> > In ucsi_init(), ucsi->connector is assigned only after the
> > UCSI_SET_NOTIFICATION_ENABLE command completes. But once that command
> > is sent, the PPM is free to raise a connector change notification at
> > any time, which is handled through:
> >
> >     ucsi_notify_common() -> ucsi_connector_change()
> >
> > ucsi_connector_change() indexes into ucsi->connector to get the
> > connector and schedule its work item. If this notification arrives
> > before ucsi_init() reaches the "ucsi->connector = connector;" line,
> > ucsi->connector is still NULL and the driver crashes dereferencing it.
>
> If you have logs for it, you can attach it.

[    9.344626][   T12] Unable to handle kernel NULL pointer
dereference at virtual address 0000000000000038
[   10.264527][   T12] Call trace:
[   10.264527][   T12]  queue_work_on+0x24/0xcc (P)
[   10.273545][  T558] [03:35:41:266-E][
558][charger_partition]get_charger_partition_info:684 partion: sda
start 519(block) size 256(block)
[   10.285497][   T12]  ucsi_notify_common+0x6c/0x150
[   10.285498][   T12]  ucsi_callback+0x1f0/0x310 [ucsi_qti_glink
582e698363365d861463162e2b10d3a93557b748]
[   10.285501][   T12]  pmic_glink_rx_work+0xe4/0x1d0 [qti_pmic_glink
9f1cb39181e1207dca3707d04498cfe86db1a757]
[   10.293012][  T558] [03:35:41:285-E][
558][charger_partition]charger_partition_work:1389 get partition info
ok
[   10.353543][   T12]  process_scheduled_works+0x1cc/0x46c
[   10.353544][   T12]  worker_thread+0x204/0x2fc
[   10.353546][   T12]  kthread+0x14c/0x280
[   10.353547][   T12]  ret_from_fork+0x10/0x20
[   10.353551][   T12] Kernel panic - not syncing: Oops: Fatal exception

>From additional logs i can see there is a block for 11ms, that means
the call flow ucsi_notify_commom  --> ucsi_connector_change is running
during this block
>
> >
> > Fix this by setting ucsi->connector right after the connectors are
> > registered, before notifications are enabled, so it is always valid
> > by the time a connector change event can be delivered. Also clear
> > ucsi->connector in the err_unregister path so it isn't left pointing
> > at freed memory if a later step in ucsi_init() fails.
> >
> > Signed-off-by: Uttkarsh Aggarwal <uttkarsh.aggarwal@xxxxxxxxxxxxxxxx>
>
> It seems it is a bug. You could add Fixed-by tag and Cc to stable tree.

Sure, I will push v2 for that.

Uttkarsh


>
> Peter
> > ---
> >  drivers/usb/typec/ucsi/ucsi.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> > index bef3f9b71d71..8f7b73a9f4f4 100644
> > --- a/drivers/usb/typec/ucsi/ucsi.c
> > +++ b/drivers/usb/typec/ucsi/ucsi.c
> > @@ -2122,6 +2122,14 @@ static int ucsi_init(struct ucsi *ucsi)
> >                       goto err_unregister;
> >       }
> >
> > +     /*
> > +      * Set ucsi->connector before enabling notifications. A connector
> > +      * change event can be handled as soon as the notifications are
> > +      * enabled below, and ucsi_connector_change() indexes into
> > +      * ucsi->connector, so it must not be NULL at that point.
> > +      */
> > +     ucsi->connector = connector;
> > +
> >       /* Enable all supported notifications */
> >       ntfy = ucsi_get_supported_notifications(ucsi);
> >       command = UCSI_SET_NOTIFICATION_ENABLE | ntfy;
> > @@ -2129,7 +2137,6 @@ static int ucsi_init(struct ucsi *ucsi)
> >       if (ret < 0)
> >               goto err_unregister;
> >
> > -     ucsi->connector = connector;
> >       ucsi->ntfy = ntfy;
> >
> >       mutex_lock(&ucsi->ppm_lock);
> > @@ -2143,6 +2150,7 @@ static int ucsi_init(struct ucsi *ucsi)
> >       return 0;
> >
> >  err_unregister:
> > +     ucsi->connector = NULL;
> >       for (con = connector; con->port; con++)
> >               ucsi_unregister_port(con);
> >       for (i = 0; i < ucsi->cap.num_connectors; i++)
> > --
> > 2.34.1
> >
> >
>
> --
>
> Thanks,
> Peter Chen