Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190516142342.28019-1-smuchun@gmail.com> <20190524190443.GB29565@kroah.com>
 <CAPSr9jH3sowszuNtBaTM1Wdi9vW+iakYX1G3arj+2_r5r7bYwQ@mail.gmail.com>
In-Reply-To: <CAPSr9jH3sowszuNtBaTM1Wdi9vW+iakYX1G3arj+2_r5r7bYwQ@mail.gmail.com>
From:   Muchun Song <smuchun@gmail.com>
Date:   Tue, 18 Jun 2019 21:40:13 +0800
Message-ID: <CAPSr9jFG17YnQC3UZrTZjqytB5wpTMeqqqOcJ7Sf6gAr8o5Uhg@mail.gmail.com>
Subject: Re: [PATCH v4] driver core: Fix use-after-free and double free on
 glue directory
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Prateek Sood <prsood@codeaurora.org>,
        Mukesh Ojha <mojha@codeaurora.org>, gkohli@codeaurora.org,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        zhaowuyun@wingtech.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Ping guys ? I think this is worth fixing.

Muchun Song <smuchun@gmail.com> =E4=BA=8E2019=E5=B9=B45=E6=9C=8825=E6=97=A5=
=E5=91=A8=E5=85=AD =E4=B8=8B=E5=8D=888:15=E5=86=99=E9=81=93=EF=BC=9A

>
> Hi greg k-h,
>
> Greg KH <gregkh@linuxfoundation.org> =E4=BA=8E2019=E5=B9=B45=E6=9C=8825=
=E6=97=A5=E5=91=A8=E5=85=AD =E4=B8=8A=E5=8D=883:04=E5=86=99=E9=81=93=EF=BC=
=9A
> >
> > On Thu, May 16, 2019 at 10:23:42PM +0800, Muchun Song wrote:
> > > There is a race condition between removing glue directory and adding =
a new
> > > device under the glue directory. It can be reproduced in following te=
st:
> >
> > <snip>
> >
> > Is this related to:
> >         Subject: [PATCH v3] drivers: core: Remove glue dirs early only =
when refcount is 1
> >
> > ?
> >
> > If so, why is the solution so different?
>
> In the v1 patch, the solution is that remove glue dirs early only when
> refcount is 1. So
> the v1 patch like below:
>
> @@ -1825,7 +1825,7 @@ static void cleanup_glue_dir(struct device *dev,
> struct kobject *glue_dir)
>                 return;
>
>         mutex_lock(&gdp_mutex);
> -       if (!kobject_has_children(glue_dir))
> +       if (!kobject_has_children(glue_dir) && kref_read(&glue_dir->kref)=
 =3D=3D 1)
>                 kobject_del(glue_dir);
>         kobject_put(glue_dir);
>         mutex_unlock(&gdp_mutex);
> -----------------------------------------------------------------------
>
> But from Ben's suggestion as below:
>
> I find relying on the object count for such decisions rather fragile as
> it could be taken temporarily for other reasons, couldn't it ? In which
> case we would just fail...
>
> Ideally, the looking up of the glue dir and creation of its child
> should be protected by the same lock instance (the gdp_mutex in that
> case).
> -----------------------------------------------------------------------
>
> So another solution is used from Ben's suggestion in the v2 patch. But
> I forgot to update the commit message until the v4 patch. Thanks.
>
> Yours,
> Muchun