On Tue, Nov 14, 2017 at 12:16:01AM +0000, Alan Cox wrote:
> On Tue, 14 Nov 2017 00:05:48 +0200
> Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
>
> > Hi Alan,
> >
> > On Mon, Nov 06, 2017 at 11:36:45PM +0000, Alan wrote:
> > > We load the firmware once, set pointers to it and then at some point release
> > > it. We should not be doing a vfree() on the pointers into the firmware.
> > >
> > > Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
> > > ---
> > > .../atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 2 --
> > > 1 file changed, 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> > > index 8158ea40d069..f181bd8fcee2 100644
> > > --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> > > +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> > > @@ -288,8 +288,6 @@ void sh_css_unload_firmware(void)
> > > for (i = 0; i < sh_css_num_binaries; i++) {
> > > if (fw_minibuffer[i].name)
> > > kfree((void *)fw_minibuffer[i].name);
> > > - if (fw_minibuffer[i].buffer)
> > > - vfree((void *)fw_minibuffer[i].buffer);
> >
> > You shouldn't end up here if the firmware is just loaded once. If multiple
> > times, then yes.
>
> You end up there when unloading the module.
Ah, that's for sure indeed. I thought loading would be already a challenge.
:-)
>
> > The memory appears to have been allocated using kmalloc() in some cases.
> > How about kvfree(), or changing that kmalloc() to vmalloc()
>
> I'll take a deeper look at what is going on.
Look for minibuffer in sh_css_load_blob_info(). The buffer field of the
struct is allocated using kmalloc, I wonder if changing that to vmalloc
would just address this.
The buffer is elsewhere allocated using vmalloc. I suspect that some of the
cleanup patches changed how this works but missed changing the other one.
--
Regards,
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx
