On Tue, Jul 4, 2023 at 5:50 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > On 7/4/23 16:28, Andy Shevchenko wrote: > > On Tue, Jul 04, 2023 at 12:02:00PM +0100, Dan Scally wrote: > >> On 30/06/2023 16:23, Andy Shevchenko wrote: > >>> On Fri, Jun 30, 2023 at 2:06 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > >>>> When ipu_bridge_parse_rotation() and ipu_bridge_parse_orientation() run > >>>> sensor->adev is not set yet. > >>>> > >>>> So if either of the dev_warn() calls about unknown values are hit this > >>>> will lead to a NULL pointer deref. > >>>> > >>>> Set sensor->adev earlier, with a borrowed ref to avoid making unrolling > >>>> on errors harder, to fix this. > >>> TBH, I don't like this approach, it seems a bit dirty to me. > >>> > >>> First of all, why do we need pci_dev to be a parameter in this function? > >>> Second, why don't we consistently use the ACPI handle (with respective > >>> acpi_handle_*() macros to print messages)? > >>> > >>> So, my proposal here is to actually save the ACPI device handle in the > >>> sensor object and use it for the messaging. It makes it consistent and > >>> doesn't require to rewrite adev field which seems the dirty part to > >>> me. > >> > >> It's a bit finicky but I don't think it's so bad; the refcounting is all > >> fine, the later acpi_dev_get() is only to hold a reference once the next > >> loop iteration frees the existing one and the rewrite should store the exact > >> same pointer...we could just not store the result of the acpi_dev_get() call > >> to avoid that weird rewrite perhaps? > > > > For short term solution in between the patches I might agree with you, but > > backporting. Backporting a bad code doesn't make it better even if it fixes > > nasty bug. And I proposed the solution. We may kill the handle same way as > > we are killing the awkwardness of this assignment later in the series. > > Yeah, no sorry. As Dan pointed out this fix is fine and I don't feel > like re-writing it just because you don't like it. > > I don't see any real technical arguments against this approach, just > you not liking it. OK. -- With Best Regards, Andy Shevchenko
