From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 023357AF for ; Tue, 4 Aug 2015 18:07:48 +0000 (UTC) Received: from mail-lb0-f171.google.com (mail-lb0-f171.google.com [209.85.217.171]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7314E10A for ; Tue, 4 Aug 2015 18:07:46 +0000 (UTC) Received: by lbbyj8 with SMTP id yj8so10845251lbb.0 for ; Tue, 04 Aug 2015 11:07:45 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <2111196.TG1k3f53YQ@avalon> <20150731165346.GA18984@infradead.org> <1624703.qdGzscHWSc@avalon> <20150804125556.GA5180@mwanda> Date: Tue, 4 Aug 2015 11:07:44 -0700 Message-ID: From: Dmitry Torokhov To: Julia Lawall Content-Type: multipart/alternative; boundary=001a11c3bfacb9b3f5051c8029d0 Cc: Shuah Khan , Russell King , ksummit-discuss@lists.linuxfoundation.org, Christoph Hellwig , Tejun Heo , Dan Carpenter Subject: Re: [Ksummit-discuss] [TECH TOPIC] Fix devm_kzalloc, its users, or both List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --001a11c3bfacb9b3f5051c8029d0 Content-Type: text/plain; charset=UTF-8 On Tue, Aug 4, 2015 at 11:03 AM, Julia Lawall wrote: > On Tue, 4 Aug 2015, Dmitry Torokhov wrote: > > > > > > > On Tue, Aug 4, 2015 at 5:55 AM, Dan Carpenter > > wrote: > > On Sat, Aug 01, 2015 at 01:21:09PM +0200, Julia Lawall wrote: > > > Currently, is there any documentation about when these > > functions can be > > > used? All I remember seeing is a discussion of what the > > functions do and > > > what functions are available. But nothing about when they > > should and > > > should not be used. > > > > > > > The documentation for devm_kmalloc() doesn't list common > > pitfalls. The > > other big one which should be obvious, but happens often is > > freeing > > devm_ memory with kfree(). > > > > /** > > * devm_kmalloc - Resource-managed kmalloc > > * @dev: Device to allocate memory for > > * @size: Allocation size > > * @gfp: Allocation gfp flags > > * > > * Managed kmalloc. Memory allocated with this function is > > * automatically freed on driver detach. Like all other devres > > * resources, guaranteed alignment is unsigned long long. > > * > > * RETURNS: > > * Pointer to allocated memory on success, NULL on failure. > > */ > > > > > > I wonder if it would be possible to note that the current thread is going > > through probe() or remove() code path in driver core and scream if we > > encounter devm* call outside of such path. That will give false > positives in > > cases when there is a legitimate mix of automatic and manual resource > > management (i.e. you rely on automatic cleanup in probe()/remove() but > need > > to free/reallocate object somewhere else), but we can create another call > > for that. > > This seems easy to do with Coccinelle, at least with respect to the > functions in a single file. I guess it doesn't solve the file operations > problem, though? > No, it does not, but I've seen a few examples with devm (mostly devm_k*alloc) used in wrong context and causing almost-memory-leaks (i.e. they had a chance to be eventually cleaned up, but not really). Thanks. -- Dmitry --001a11c3bfacb9b3f5051c8029d0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Tue, Aug 4, 2015 at 11:03 AM, Julia Lawall <julia.lawall@lip6.fr= > wrote:
On Tue, 4 Aug 2015, Dmitry Torokhov wrote:

>
>
> On Tue, Aug 4, 2015 at 5:55 AM, Dan Carpenter <dan.carpenter@oracle.com>
> wrote:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0On Sat, Aug 01, 2015 at 01:21:09PM +0200, Ju= lia Lawall wrote:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0> Currently, is there any documentation a= bout when these
>=C2=A0 =C2=A0 =C2=A0 =C2=A0functions can be
>=C2=A0 =C2=A0 =C2=A0 =C2=A0> used?=C2=A0 All I remember seeing is a = discussion of what the
>=C2=A0 =C2=A0 =C2=A0 =C2=A0functions do and
>=C2=A0 =C2=A0 =C2=A0 =C2=A0> what functions are available.=C2=A0 But= nothing about when they
>=C2=A0 =C2=A0 =C2=A0 =C2=A0should and
>=C2=A0 =C2=A0 =C2=A0 =C2=A0> should not be used.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0>
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0The documentation for devm_kmalloc() doesn&#= 39;t list common
>=C2=A0 =C2=A0 =C2=A0 =C2=A0pitfalls.=C2=A0 The
>=C2=A0 =C2=A0 =C2=A0 =C2=A0other big one which should be obvious, but h= appens often is
>=C2=A0 =C2=A0 =C2=A0 =C2=A0freeing
>=C2=A0 =C2=A0 =C2=A0 =C2=A0devm_ memory with kfree().
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0/**
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* devm_kmalloc - Resource-managed kmal= loc
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* @dev: Device to allocate memory for<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* @size: Allocation size
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* @gfp: Allocation gfp flags
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0*
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* Managed kmalloc.=C2=A0 Memory alloca= ted with this function is
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* automatically freed on driver detach= .=C2=A0 Like all other devres
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* resources, guaranteed alignment is u= nsigned long long.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0*
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* RETURNS:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0* Pointer to allocated memory on succe= ss, NULL on failure.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0*/
>
> =C2=A0
> I wonder if it would be possible to note that the current thread is go= ing
> through probe() or remove() code path in driver core and scream if we<= br> > encounter devm* call outside of such path. That will give false positi= ves in
> cases when there is a legitimate mix of automatic and manual resource<= br> > management (i.e. you rely on automatic cleanup in probe()/remove() but= need
> to free/reallocate object somewhere else), but we can create another c= all
> for that.

This seems easy to do with Coccinelle, at least with respect to= the
functions in a single file.=C2=A0 I guess it doesn't solve the file ope= rations
problem, though?

No, it does not, but I= 've seen a few examples with devm (mostly devm_k*alloc) used in wrong c= ontext and causing almost-memory-leaks (i.e. they had a chance to be eventu= ally cleaned up, but not really).

Thanks.

--=C2=A0
Dmi= try
--001a11c3bfacb9b3f5051c8029d0--