Re: [GIT PULL] FPGA Manager changes for 6.6-final

From: Greg KH <gregkh@linuxfoundation.org>
To: Xu Yilun <yilun.xu@linux.intel.com>
Cc: yilun.xu@intel.com, linux-fpga@vger.kernel.org, hao.wu@intel.com,
	mdf@kernel.org
Subject: Re: [GIT PULL] FPGA Manager changes for 6.6-final
Date: Wed, 18 Oct 2023 09:50:37 +0200	[thread overview]
Message-ID: <2023101825-ligament-undergrad-cc4d@gregkh> (raw)
In-Reply-To: <ZS88oHe8T69/Kks7@yilunxu-OptiPlex-7050>

On Wed, Oct 18, 2023 at 10:02:08AM +0800, Xu Yilun wrote:
> On Tue, Oct 17, 2023 at 07:17:29PM +0200, Greg KH wrote:
> > On Tue, Oct 17, 2023 at 11:00:22PM +0800, Xu Yilun wrote:
> > > The following changes since commit 6465e260f48790807eef06b583b38ca9789b6072:
> > > 
> > >   Linux 6.6-rc3 (2023-09-24 14:31:13 -0700)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/fpga/linux-fpga tags/fpga-for-6.6-final
> > > 
> > > for you to fetch changes up to 6a935361500a21ef11a82814ee66fc58e59813f7:
> > > 
> > >   fpga: Fix memory leak for fpga_region_test_class_find() (2023-10-12 12:59:29 +0800)
> > > 
> > > ----------------------------------------------------------------
> > > FPGA Manager changes for 6.6-final
> > > 
> > > FPGA KUnit test:
> > > 
> > > - Marco's change fixes null-ptr-deref when try_module_get()
> > > - Jinjie's change fixes a memory leak issue
> > > 
> > > Intel m10 bmc secure update:
> > > 
> > > - Maintainer change from Russ Weight to Peter Colberg
> > > 
> > > All patches have been reviewed on the mailing list, and have been in the
> > > last linux-next releases (as part of our fixes branch)
> > > 
> > > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > > 
> > > ----------------------------------------------------------------
> > > Jinjie Ruan (1):
> > >       fpga: Fix memory leak for fpga_region_test_class_find()
> > > 
> > > Marco Pagani (4):
> > >       fpga: add helpers for the FPGA KUnit test suites.
> > >       fpga: add a platform driver to the FPGA Manager test suite
> > >       fpga: add a platform driver to the FPGA Bridge test suite
> > >       fpga: add a platform driver to the FPGA Region test suite
> > 
> > Why are all of these test suite patches here?  They are not relevant for
> > 6.6-final as they do not resolve anything.
> 
> Maybe the subjects indicate no bug fixing, but they fix null-ptr-deref
> issues when modprobe fpga-mgr/bridge/region-test.

That's not obvious, sorry.  So are the tests broken right now so that
they don't work at all?

> In fpga-mgr-test, the pdev->dev->driver is not assigned, so when
> 
>   fpga_mgr_test_get()->try_module_get(dev->parent->driver->owner)

That's a horrible line and should be fixed.  How do you know if a device
has a parent, or if that parent has a driver?  You don't, that should be
fixed instead.

And module_get on a driver pointer is also never a good idea for other
reasons, why is this happening at all?  It shouldn't be needed if the
code is set up properly (i.e. the unloading of a driver will handle the
shutdown and reference counting properly, no need to try to use module
references at all.)

> NULL ptr is referenced.
> 
> So do fpga-bridge/region-test.
> 
> Patch #1 adds a common helper to generate a platform driver.

Don't abuse platform devices/drivers like this, this is not a platform
device or driver.  If you really want to do this, use a real driver and
device, not a platform one please.

> Patch #2/3/4 fix the issues by matching the driver to pdev.
> 
> See:
> Closes: https://lore.kernel.org/linux-fpga/4d51e87f-830a-adae-d6f7-6aed9433fdc6@huawei.com/
> Closes: https://lore.kernel.org/linux-fpga/f2b30203-1a67-4533-eddc-b380044e2e68@huawei.com/
> Closes: https://lore.kernel.org/linux-fpga/d557b4ee-4b3a-8747-bdda-0ed480212a63@huawei.com/

Want to just disable all of these from the build entirely for now and
then fix them up properly for 6.7?

thanks,

greg k-h