Re: [PATCH] i2c: designware: Remove i2c_dw_remove_lock_support()

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Oct 14, 2025 at 03:39:55PM -0700, Nathan Chancellor wrote:
> On Tue, Oct 14, 2025 at 04:58:56PM -0500, Mario Limonciello (AMD) (kernel.org) wrote:
> > On 10/14/2025 3:33 PM, Bjorn Helgaas wrote:
> > > I'm totally fine with the patch itself, but I think the commit log
> > > could be trimmed to something like the following with no loss:
> > > 
> > >    Remove struct i2c_dw_semaphore_callbacks.remove() and
> > >    i2c_dw_remove_lock_support().
> > > 
> > >    440da737cf8d ("i2c: designware: Use PCI PSP driver for
> > >    communication") removed the last place that set
> > >    i2c_dw_semaphore_callbacks.remove(), which made
> > >    i2c_dw_remove_lock_support() a no-op.
> > > 
> > >    This has the side effect of avoiding this kCFI warning (see Link):
> > > 
> > >      dw_i2c_plat_remove+0x3c: no-cfi indirect call!
> > > 
> > >    Link: https://lore.kernel.org/r/20251013-dw_i2c_plat_remove-avoid-objtool-no-cfi-warning-v1-1-8cc4842967bf@kernel.org
> > > 
> > > FWIW,
> > > Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > I echo Bjorn's comments on the lengthy commit message.
> > Code change looks fine.
> > 
> > Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> 
> I have no objections to trimming the commit message if so desired but I
> think the solution (removing unused code) is more tangential to the
> problem (potentially accessing an array out of bounds). I am sometimes
> looking at changes from ten years ago where something was done to avoid
> a problem but the problem was never mentioned in the message but may
> have been elsewhere. Maybe nobody ever needs .remove() again but what if
> new IP comes out that necessitates it and they go to revert this change
> without avoiding this problem? I could try to make the analysis shorter
> if that would help.

OK, I missed that there was an out-of-bounds array access involved.
Maybe that warrants more details.

Bjorn