From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 487E9C3DA47 for ; Tue, 9 Jul 2024 16:32:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D8E386B009A; Tue, 9 Jul 2024 12:32:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D650C6B009B; Tue, 9 Jul 2024 12:32:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C2CE46B009C; Tue, 9 Jul 2024 12:32:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id A05876B009A for ; Tue, 9 Jul 2024 12:32:30 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 5EB85A4DD6 for ; Tue, 9 Jul 2024 16:32:30 +0000 (UTC) X-FDA: 82320757260.07.BC48DA3 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf27.hostedemail.com (Postfix) with ESMTP id 187024000F for ; Tue, 9 Jul 2024 16:32:27 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=eTj75iwZ; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf27.hostedemail.com: domain of kees@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=kees@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720542725; a=rsa-sha256; cv=none; b=hSwcB1rEjw6ZuNA8CWXFgk/FAcn0CiqjF72KEjU3g5Mq89WJsR+MHzs6QByKldcFcLVUmC VhJJQFDW5UU6mOKQUhmTC/a4dNbYkZpoqDjLk8iI/E5Aen92MuN0DTqT8Xe+WtgbZyTOqC 1V/O+MgRV3knNSr3bqM2yL1ofEDQkTk= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=eTj75iwZ; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf27.hostedemail.com: domain of kees@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=kees@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720542725; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=BM8BMRPGapUKJeLzEzIMC6xLw+C7vvXqu32SPwN8+dg=; b=8LLag0nOn+cxjHK9qdsh7XSyZGdGMaXgDmU+vPBUSTLfPaQE/ilvh0PSrziEfdzCd1bY3l 6PSqjBvVuumuFArTpA8gp8+WeCg9kozxIHPW6G1LKh556x0TfH0XP/T2v0t9Tw3U81pdFM +G/hVQK/Aa684tfL+kKwoPBnQQNFwiU= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id E116FCE116A; Tue, 9 Jul 2024 16:32:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 17B78C3277B; Tue, 9 Jul 2024 16:32:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1720542743; bh=FLdjgr6meJn+TXvEGqYcf52cjuR6shaot6RCYkVxb8E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=eTj75iwZnk1OpJyXM9r7G1cS70S6Xcgm89zZriykuejpXMPaH/Y25kY9lldYBVoJ4 kIxag7yX2o+u4MESoVReiciDUmjyh64RxiOnjQEJ1xm0IVpO68q/Ab/AEESqTZ/sk1 qsNbE+sIyyoaYQUSe0PqxH9UDsgKtxCbNmast2D0ZdMJO39ZCYIZXU9pOllqszIofq Jq2PNHyxDDQSrTxoZIMjo3mK/nufqS3q0xb5N5rSiQXXoFOXBkdVBL6ddGTZ1hMQ7U UR7lW0cIwdXyvBiXsVImIQe1/YUiUT9TbruDmqM0IWMpxALo3ISTn0IMarUJx9mik3 sgQjVkaUZblgQ== Date: Tue, 9 Jul 2024 09:32:22 -0700 From: Kees Cook To: Przemek Kitszel Cc: Vlastimil Babka , Tony Luck , "Guilherme G. Piccoli" , linux-hardening@vger.kernel.org, Jann Horn , Nick Desaulniers , Miguel Ojeda , Marco Elver , Nathan Chancellor , Hao Luo , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Mark Rutland , Jakub Kicinski , Petr Pavlu , Alexander Lobakin , Tony Ambardar , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC][PATCH 4/4] pstore: Replace classic kmalloc code pattern with typed argument Message-ID: <202407090910.87821CD@keescook> References: <20240708190924.work.846-kees@kernel.org> <20240708191840.335463-4-kees@kernel.org> <093108ea-f567-4210-9f2e-eb8e5039d1eb@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <093108ea-f567-4210-9f2e-eb8e5039d1eb@intel.com> X-Rspamd-Queue-Id: 187024000F X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: aqupgpepehgijk7r3kj6ep34y6anm1y6 X-HE-Tag: 1720542747-31585 X-HE-Meta: U2FsdGVkX186ekiIovvSYkmiohOkjgLkjbP6Vi8B62XzsfuA3i+boKpPDxnV+PgjBejnZ9vcCrfbaIkgXKzBd+C8y3yNsEaKv/VP1BQNOChYDxTzuaoYKJcYhNZbNVaZBqDsv01iMMRaM/g9F2VJfkv2nGgBudNiOWlT3GarvMBoKAifH9zoQSORu+SDPT/V9UBVdY7XyY00N+yjGgiNyqTBES9l/qAHgkyzDCy6P4JII04A3QXFAd8fD3MEo/ImnH+Yq5DUJiuiKyQK8a4wAgPD5yvUoa6PVzNICLi27sE2fXpg+sOYN5aGni9W9mmjLng4O8IgTk3Zc5Gik990GDn8e8D8Jh9E+fE6q2yJxSTvCg26fPWQVu5YgVaO6BE4NZkid2d2oRhDBdovTKVO0Bz/D7RFHm800zBAueZ6ZCqR59N9yeY0riRWVZatvWUDEN4ZwYoztWoY/3QoM2Ng7deqKZsWc0f2nctz5lFuFFgNQwp5Re7cksX7WLSlDCRy/kFCzHpv28iO/fb92FGf55lZsVZvWkmV55Yl58T4QjMZjrrhCSJsjUHNUpRd3ELyfMXE0d8e2R6pRQrPPCTXxRzxt+fmjlQcpR3Mbh63x5nX7n71f24Lsffwrzx9BIiMQhp0JxTw6Cqu3Vgm8W2qyF1dncIzDS69g9Pdt3zRXgB7eoAvLhNSiN4GhaZLurVWAaaOZKEOxkTCV+WtsYrig6x4wEPvALsPKMc5p0TdCQ5Cg7N0I4gew05zmOC2zySfukzlKaMNUf5RM6E0r5BWMVE5w4fJO3wA9yVA4+1KLnWhX4JXL1l76ti1w/kBMEpdhCiT9/lVpL8mCWCJ+5chOBXQ3JuQGuU96bjWs4VCVHGC2MncIHLm7IaDGaXmcz9o90pJ7q2VPy9c8mF9DcL83ixFFyQdfmrDpW+egUpR6CJlMCjFOFQETi0FVIVLXNUnntbZe/8CAYOds4de8Od 46RHPVds hv6XZYG0+4Cg1kCQoc3XS0UnLRm4U5MX9XjrcUNuCcs0yYBJ1dXZbr3f71sWnGqTsf6O4qMGgFeHkEEsajiiUnI4tSaFTHf1NlosFpGhxu9kMKNR9ytbl4WNmkQxFTtp7X58MDRsdI8rTmKfddBdCtFMxyQ0Hkzi5wiC2Y6tzJRjjrDqv042ZlrWbxQ+Mwc/rhlnzDt5lqD/WuHVu5m1g9iMC6zz7zGM7LJ8VcPPnFLzRdO3o21HauRIDLZWRnd7wU05k5vv5SpSJ4U6ivVM9e+zuT4HcFD0kLQan X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Jul 09, 2024 at 09:06:58AM +0200, Przemek Kitszel wrote: > On 7/8/24 21:18, Kees Cook wrote: > > diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c > > index de8cf5d75f34..7bb9cacb380f 100644 > > --- a/fs/pstore/blk.c > > +++ b/fs/pstore/blk.c > > @@ -297,7 +297,7 @@ static int __init __best_effort_init(void) > > return -EINVAL; > > } > > - best_effort_dev = kzalloc(sizeof(*best_effort_dev), GFP_KERNEL); > > + best_effort_dev = kzalloc(best_effort_dev, GFP_KERNEL); > > if (!best_effort_dev) > > return -ENOMEM; > I expect raised eyebrows and typical vocalizations of amusement :D - > IOW: we should consider changing the name of the macro, although I like > it as is :) Yeah, I prefer keeping the name too. I feel like adding yet more macros around the allocators does not make anyone too happy. :) > other: > you repeat the pointer name twice, and code is magic anyway, so perhaps: > kzalloc_me(best_effort_dev, GFP_KERNEL); > and another variant to cover declaration-with-init. Switch the calling style is, however, where I think it'd be good to change the name. I've had push-back in the past on changing away from the "assignment style" to the "pass by reference" style, but I would honestly prefer dropping the assignment: it is almost always redundant. (I understood the push-back to be around the case of not being able to easily use the "return kmalloc(...)" code pattern.) It also makes it easier to deal with fixed array and flexible array variants, as the argument count can be examined to determine if this is a fixed-size struct or a flexible object: kzalloc_struct(ptr, GFP_KERNEL); kzalloc_struct(ptr, flex_member, count, GFP_KERNEL); -> uses struct_size() to get allocation size I wonder if we can find a way to also handle the array case at compile time: kzalloc_struct(array, count, GFP_KERNEL); And if so, maybe the naming should be "kzalloc_me" like you suggest, or maybe "kzalloc_obj"? The resulting Coccinelle script gets a little more complex since we have to rewrite the matched function, but it's not bad: @find@ type TYPE; TYPE *P; expression GFP; identifier ALLOC =~ "k[mz]alloc"; @@ P = ALLOC(\(sizeof(*P)\|sizeof(TYPE)\), GFP) @script:python rename@ alloc_name << find.ALLOC; ALLOC_OBJ; @@ coccinelle.ALLOC_OBJ = cocci.make_ident(alloc_name + "_obj") @convert@ type find.TYPE; TYPE *find.P; expression find.GFP; identifier find.ALLOC; identifier rename.ALLOC_OBJ; @@ - P = ALLOC(\(sizeof(*P)\|sizeof(TYPE)\), GFP) + ALLOC_OBJ(P, GFP) And the results in fs/pstore/ look like this: diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c index de8cf5d75f34..bc0e0a170604 100644 --- a/fs/pstore/blk.c +++ b/fs/pstore/blk.c @@ -297,7 +297,7 @@ static int __init __best_effort_init(void) return -EINVAL; } - best_effort_dev = kzalloc(sizeof(*best_effort_dev), GFP_KERNEL); + kzalloc_obj(best_effort_dev, GFP_KERNEL); if (!best_effort_dev) return -ENOMEM; diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 03425928d2fb..e0ba70543121 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -682,7 +682,7 @@ void pstore_get_backend_records(struct pstore_info *psi, struct pstore_record *record; int rc; - record = kzalloc(sizeof(*record), GFP_KERNEL); + kzalloc_obj(record, GFP_KERNEL); if (!record) { pr_err("out of memory creating record\n"); break; diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index b1a455f42e93..93064ba2c480 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -228,8 +228,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record) */ struct persistent_ram_zone *tmp_prz, *prz_next; - tmp_prz = kzalloc(sizeof(struct persistent_ram_zone), - GFP_KERNEL); + kzalloc_obj(tmp_prz, GFP_KERNEL); if (!tmp_prz) return -ENOMEM; prz = tmp_prz; diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index f1848cdd6d34..9561f4dfa853 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -588,7 +588,7 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, struct persistent_ram_zone *prz; int ret = -ENOMEM; - prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL); + kzalloc_obj(prz, GFP_KERNEL); if (!prz) { pr_err("failed to allocate persistent ram zone\n"); goto err; diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c index 694db616663f..312796dc93f0 100644 --- a/fs/pstore/zone.c +++ b/fs/pstore/zone.c @@ -1165,7 +1165,7 @@ static struct pstore_zone *psz_init_zone(enum pstore_type_id type, return ERR_PTR(-ENOMEM); } - zone = kzalloc(sizeof(struct pstore_zone), GFP_KERNEL); + kzalloc_obj(zone, GFP_KERNEL); if (!zone) return ERR_PTR(-ENOMEM); -- Kees Cook