RE: [PATCH 2/2] drm: anx7625: add I2S input stream right justified control

Hi sahiko, thanks for the review, OK, I'll try to use fmt HDMI_RIGHT_J.

> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s)
> to consider:
> 
> New issues:
> - [Medium] Bypassing standard ALSA HDMI codec DAI format negotiation.
> - [Medium] RIGHT_JUSTIFY_CTRL bit is conditionally set but never cleared.
> 
> Pre-existing issues:
> - [High] Use-After-Free due to interrupt firing after workqueue destruction.
> --
> 
> --- Patch [2]: [PATCH 2/2] drm: anx7625: add I2S input stream right justified
> control --- commit e78186b015f668998903e293729ea71c5bebfbd8
> Author: Xin Ji <[email protected]>
> 
> drm: anx7625: add I2S input stream right justified control
> 
> The default audio data stream is left justify. When the property
> "analogix,audio-right-justify-enable" is detected, the code enables right
> justify.
> 
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index fffcd6154c710..d00aa6ea7273d 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> 
> [ ... ]
> 
> > @@ -2083,6 +2086,12 @@ static int anx7625_audio_hw_params(struct
> device *dev, void *data,
> >               ret |= anx7625_write_and(ctx, ctx->i2c.tx_p2_client,
> >                               AUDIO_CHANNEL_STATUS_6, ~AUDIO_LAYOUT);
> >
> > +     /* Right justified */
> > +     if (ctx->pdata.audio_right_justify)
> 
> [Severity: Medium]
> Instead of bypassing standard ALSA HDMI codec DAI format negotiation,
> should this code check the standard fmt->fmt parameter for HDMI_RIGHT_J?
> 
> When the driver receives HDMI_RIGHT_J via hdmi_codec_daifmt->fmt at the
> start of anx7625_audio_hw_params(), it rejects it with -EINVAL. Relying on a
> custom static DT property breaks compatibility with generic ALSA sound cards
> that dynamically negotiate the DAI format.
> 
> [Severity: Medium]
> Is there a missing clear operation for this bit?
> 
> If a bootloader or previous kernel execution left the RIGHT_JUSTIFY_CTRL bit
> set, and ctx->pdata.audio_right_justify is false, the hardware will remain in
> right-justified mode even when standard I2S is expected. Should there be an
> anx7625_write_and() to clear the bit on the false path?
> 
> > +             ret |= anx7625_write_or(ctx, ctx->i2c.tx_p2_client,
> > +                                     AUDIO_CONTROL_REGISTER,
> > +                                     RIGHT_JUSTIFY_CTRL);
> > +
> >       /* FS */
> >       switch (params->sample_rate) {
> >       case 32000:
> 
> [ ... ]
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does the driver have a use-after-
> free vulnerability during unbind?
> 
> In anx7625_i2c_remove(), the workqueue is destroyed before the interrupt is
> disabled:
> 
>         if (platform->pdata.intp_irq)
>                 destroy_workqueue(platform->workqueue);
> 
> Because the driver uses devm_request_threaded_irq(), the interrupt is only
> freed after remove() returns. If an interrupt (like Hot Plug Detect) fires after
> destroy_workqueue(), the ISR anx7625_intr_hpd_isr() will attempt to use it:
> 
>         queue_work(ctx->workqueue, &ctx->work);
> 
> Should disable_irq() be called before destroying the workqueue to prevent
> the ISR from dereferencing the freed workqueue pointer?
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818020559.961335-
> [email protected]?part=2