From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756017AbdKNUOo (ORCPT ); Tue, 14 Nov 2017 15:14:44 -0500 Received: from imap1.codethink.co.uk ([176.9.8.82]:51822 "EHLO imap1.codethink.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752151AbdKNUOj (ORCPT ); Tue, 14 Nov 2017 15:14:39 -0500 Message-ID: <1510690474.18523.39.camel@codethink.co.uk> Subject: Re: [PATCH 4.4 13/56] iio: trigger: free trigger resource correctly From: Ben Hutchings To: Greg Kroah-Hartman , Alison Schofield , Jonathan Cameron Cc: stable@vger.kernel.org, Sasha Levin , LKML Date: Tue, 14 Nov 2017 20:14:34 +0000 In-Reply-To: <20171113125559.283539115@linuxfoundation.org> References: <20171113125557.613444087@linuxfoundation.org> <20171113125559.283539115@linuxfoundation.org> Organization: Codethink Ltd. Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.6-1+deb9u1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2017-11-13 at 13:55 +0100, Greg Kroah-Hartman wrote: > 4.4-stable review patch.  If anyone has any objections, please let me > know. > > ------------------ > > From: Alison Schofield > > > [ Upstream commit 10e840dfb0b7fc345082dd9e5fff3c1c02e7690e ] > > These stand-alone trigger drivers were using iio_trigger_put() > where they should have been using iio_trigger_free().  The > iio_trigger_put() adds a module_put which is bad since they > never did a module_get. > > In the sysfs driver, module_get/put's are used as triggers are > added & removed. This extra module_put() occurs on an error path > in the probe routine (probably rare). > > In the bfin-timer & interrupt trigger drivers, the module resources > are not explicitly managed, so it's doing a put on something that > was never get'd.  It occurs on the probe error path and on the > remove path (not so rare). > > Tested with the sysfs trigger driver. > The bfin & interrupt drivers were build tested & inspected only. In 4.4 the iio-trig-periodic-rtc seems to have the same bug (it has been removed upstream). Would it make sense to fix it in 4.4? Ben. > Signed-off-by: Alison Schofield > Signed-off-by: Jonathan Cameron > Signed-off-by: Sasha Levin > Signed-off-by: Greg Kroah-Hartman > --- >  drivers/iio/trigger/iio-trig-interrupt.c          |    8 ++++---- >  drivers/iio/trigger/iio-trig-sysfs.c              |    2 +- >  drivers/staging/iio/trigger/iio-trig-bfin-timer.c |    4 ++-- >  3 files changed, 7 insertions(+), 7 deletions(-) > > --- a/drivers/iio/trigger/iio-trig-interrupt.c > +++ b/drivers/iio/trigger/iio-trig-interrupt.c > @@ -58,7 +58,7 @@ static int iio_interrupt_trigger_probe(s >   trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL); >   if (!trig_info) { >   ret = -ENOMEM; > - goto error_put_trigger; > + goto error_free_trigger; >   } >   iio_trigger_set_drvdata(trig, trig_info); >   trig_info->irq = irq; > @@ -83,8 +83,8 @@ error_release_irq: >   free_irq(irq, trig); >  error_free_trig_info: >   kfree(trig_info); > -error_put_trigger: > - iio_trigger_put(trig); > +error_free_trigger: > + iio_trigger_free(trig); >  error_ret: >   return ret; >  } > @@ -99,7 +99,7 @@ static int iio_interrupt_trigger_remove( >   iio_trigger_unregister(trig); >   free_irq(trig_info->irq, trig); >   kfree(trig_info); > - iio_trigger_put(trig); > + iio_trigger_free(trig); >   >   return 0; >  } > --- a/drivers/iio/trigger/iio-trig-sysfs.c > +++ b/drivers/iio/trigger/iio-trig-sysfs.c > @@ -174,7 +174,7 @@ static int iio_sysfs_trigger_probe(int i >   return 0; >   >  out2: > - iio_trigger_put(t->trig); > + iio_trigger_free(t->trig); >  free_t: >   kfree(t); >  out1: > --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c > +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c > @@ -259,7 +259,7 @@ out_free_irq: >  out1: >   iio_trigger_unregister(st->trig); >  out: > - iio_trigger_put(st->trig); > + iio_trigger_free(st->trig); >   return ret; >  } >   > @@ -272,7 +272,7 @@ static int iio_bfin_tmr_trigger_remove(s >   peripheral_free(st->t->pin); >   free_irq(st->irq, st); >   iio_trigger_unregister(st->trig); > - iio_trigger_put(st->trig); > + iio_trigger_free(st->trig); >   >   return 0; >  } > > > -- Ben Hutchings Software Developer, Codethink Ltd.