Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <22ab53d1ae36d4925732e6e1dc989dc75af126da.camel@sipsolutions.net>
Subject: Re: [PATCH] Devcoredump: fix use-after-free issue when releasing
 devcd device
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Greg KH <gregkh@linuxfoundation.org>,
        Yu Wang <quic_yyuwang@quicinc.com>
Cc:     rafael@kernel.org, linux-kernel@vger.kernel.org, kernel@quicinc.com
Date:   Fri, 27 Oct 2023 14:45:57 +0200
In-Reply-To: <2023102730-twins-thieving-d04e@gregkh>
References: <20231027055521.2679-1-quic_yyuwang@quicinc.com>
         <2023102730-twins-thieving-d04e@gregkh>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2023-10-27 at 13:11 +0200, Greg KH wrote:
> >=20
> >     static void my_coredump()
> >     {
> >         struct my_coredump_state dump_state;
> >         struct device *new_device =3D
> >             kzalloc(sizeof(*new_device), GFP_KERNEL);
> >=20
> >         ...
> >         new_device->release =3D my_dev_release;
> >         device_initialize(new_device);
> >         ...
> >         device_add(new_device);
> >         ...
> >         init_completion(&dump_state.dump_done);
> >         dev_coredumpm(new_device, NULL, &dump_state, datalen, GFP_KERNE=
L,
> >                       my_coredump_read, my_coredump_free);
> >         wait_for_completion(&dump_state.dump_done);
> >         device_del(new_device);
> >         put_device(new_device);
> >     }
>=20
> Is there any in-kernel user like this?  If so, why not fix them up to
> not do this?
>=20

Maybe this is only a simplified scenario and whenever you remove a
device when a coredump is still pending this can happen?

Actually, no, wait, what is this doing??? Why is there a completion and
all that stuff there? You shouldn't really care about the dump once you
have created it, and not pass NULL for the struct module pointer
either?!

johannes