On Mon, 2017-08-07 at 16:37 +0300, Kalle Valo wrote:
> Luca Coelho <[email protected]> writes:
> 
> > On Mon, 2017-08-07 at 15:57 +0300, Kalle Valo wrote:
> > > Luca Coelho <[email protected]> writes:
> > > 
> > > > From: Christophe Jaillet <[email protected]>
> > > > 
> > > > We should free 'wgds.pointer' here as done a few lines above in another
> > > > error handling path.
> > > > It was allocated within 'acpi_evaluate_object()'.
> > > > 
> > > > Fixes: c52030a01ccc ("iwlwifi: mvm: add GEO_TX_POWER_LIMIT cmd for 
> > > > geographic tx power table")
> > > > Signed-off-by: Christophe JAILLET <[email protected]>
> > > > Signed-off-by: Luca Coelho <[email protected]>
> > > > ---
> > > >  drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 6 ++++--
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c 
> > > > b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> > > > index 79e7a7a285dc..82863e9273eb 100644
> > > > --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> > > > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> > > > @@ -1275,8 +1275,10 @@ static int iwl_mvm_sar_get_wgds_table(struct 
> > > > iwl_mvm *mvm)
> > > >  
> > > >                         entry = &wifi_pkg->package.elements[idx++];
> > > >                         if ((entry->type != ACPI_TYPE_INTEGER) ||
> > > > -                           (entry->integer.value > U8_MAX))
> > > > -                               return -EINVAL;
> > > > +                           (entry->integer.value > U8_MAX)) {
> > > > +                               ret = -EINVAL;
> > > > +                               goto out_free;
> > > > +                       }
> > > 
> > > How likely is this leak to happen in real world? To me it looks like
> > > more like a theoretical issue and could have easily waited for 4.14. But
> > > it's fine this time, just something to keep in mind in the future.
> > 
> > This is a one-liner fix and I consider memory leaks serious enough to
> > deserve a fix for rc5.  This bug can happen with broken ACPI tables and,
> > trust me, broken ACPI tables are not that hard to find.
> 
> Sure, anything's possible. But what I'm reading here this is still a
> theoretical issue, not a leak which we _know_ will happen to thousands
> of people.
> 
> > But you rule here, feel free to NACK my patches whenever you see fit! :)
> 
> I'm trying to minimise the numbers of patches going to wireless-drivers
> and striving for only fixes which really matter and keep the theoretical
> stuff for -next. The is mostly selfish reasons as wireless-drivers are a
> lot more work, especially if there are conflicts.

I totally understand.  It's a lot more work for me too, with all the
reordering I need to do and the conflicts these cause.


> But like I said in my previous mail, no need to drop this.

Okay, thanks!

--
Cheers,
Luca.

Reply via email to