From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753985AbcBVPkL (ORCPT ); Mon, 22 Feb 2016 10:40:11 -0500 Received: from mga01.intel.com ([192.55.52.88]:18619 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753043AbcBVPkJ (ORCPT ); Mon, 22 Feb 2016 10:40:09 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,485,1449561600"; d="scan'208";a="908668881" Message-ID: <1456155650.13244.24.camel@linux.intel.com> Subject: Re: [PATCH 2/2] device property: fix for a case of use-after-free From: Andy Shevchenko To: Heikki Krogerus , "Rafael J. Wysocki" Cc: Mika Westerberg , John Youn , linux-kernel@vger.kernel.org Date: Mon, 22 Feb 2016 17:40:50 +0200 In-Reply-To: <1456152641-127948-3-git-send-email-heikki.krogerus@linux.intel.com> References: <1456152641-127948-1-git-send-email-heikki.krogerus@linux.intel.com> <1456152641-127948-3-git-send-email-heikki.krogerus@linux.intel.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.3-1 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, 2016-02-22 at 16:50 +0200, Heikki Krogerus wrote: > In device_remove_property_set(), if the primary fwnode is > of type "pset", it has to be set pointing to NULL before > calling set_secondary_fwnode(). Otherwise > set_secondary_fwnode() will attempt to set the > fwnode->secondary member after the fwnode has been freed. > > Reported-by: John Youn > Signed-off-by: Heikki Krogerus > --- >  drivers/base/property.c | 4 +++- >  1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index a163f2c..ddf2987 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -820,7 +820,9 @@ void device_remove_property_set(struct device > *dev) >    * the pset. If there is no real firmware node (ACPI/DT) > primary >    * will hold the pset. >    */ > - if (!is_pset_node(fwnode)) > + if (is_pset_node(fwnode)) > + dev->fwnode = NULL; > + else >   fwnode = fwnode->secondary; >   if (!IS_ERR(fwnode) && is_pset_node(fwnode)) >   pset_free_set(to_pset_node(fwnode)); What if we do the following --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -818,9 +818,13 @@ void device_remove_property_set(struct device *dev)          */         if (!is_pset_node(fwnode))                 fwnode = fwnode->secondary; + +       /* Set device fwnode to NULL before we free it */ +       set_secondary_fwnode(dev, NULL); + +       /* Free property set for the given device */         if (!IS_ERR(fwnode) && is_pset_node(fwnode))                 pset_free_set(to_pset_node(fwnode)); -       set_secondary_fwnode(dev, NULL);  }  EXPORT_SYMBOL_GPL(device_remove_property_set);   ? -- Andy Shevchenko Intel Finland Oy