From:   Song Liu <songliubraving@meta.com>
To:     Luis Chamberlain <mcgrof@kernel.org>
CC:     "linux-modules@vger.kernel.org" <linux-modules@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Christoph Hellwig <hch@lst.de>,
        Kernel Team <kernel-team@meta.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        Song Liu <song@kernel.org>
Subject: Re: [PATCH v10] module: replace module_layout with module_memory
Thread-Topic: [PATCH v10] module: replace module_layout with module_memory
Thread-Index: AQHZOosWgrFk94mbpUiAn23Fr3awBK7ED1wA
Date:   Tue, 7 Feb 2023 22:19:30 +0000
Message-ID: <8972F26D-34FE-4463-83AD-BBCC6F92C952@fb.com>
References: <20230207002802.2514802-1-song@kernel.org>
In-Reply-To: <20230207002802.2514802-1-song@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?cnAsled0Z2/mGoijH+v8q7HuLGdnjPRl2gWyO9AHLu8wbljb7iG/Dxm+hM8c?=
 =?us-ascii?Q?kDU7IsI4OoxieAPIq7ahil4V6Nr3CuqEb65VJi+GNV/Q3KUjyBp6JWiH/kO3?=
 =?us-ascii?Q?B33b5UcHebDDEDqqYQ2CejJyt3EIxiq7sq6rhIOYI5/IoWZFxkrrUF5ssDAT?=
 =?us-ascii?Q?ExifjsynJvjbP/LpgesErbXWmrHeeJtIC36MBKmeeCsQRZGqyQ4Zyu1sgvk2?=
 =?us-ascii?Q?hZ269YsgucZj0euGhiNKi3b91pl7mkQ9MHinwCS145fgdYDZPylS7LzvXfzK?=
 =?us-ascii?Q?Xp0UPssKYGC1nwAIbetVtHsW3CIb47fLO972MlVCUn1aRJ1Zvxf6OZ/puzk2?=
 =?us-ascii?Q?KXUInMTbD+EvfcyHo4TrEPSBW9H2yrpLRRa8aX1uVDoucQo6+ycDaq30tuj4?=
 =?us-ascii?Q?0T5LdDMbUNJAs4snq5F/SgzZ9IWAQdo87VakQreJehfZCQ0pWilZBAx7Aqhr?=
 =?us-ascii?Q?FWGVjKb/f6JOUqwbWSlsyw2V2XO17YW+3n1dD/CfNLPktm/XUOp0RXC1dC47?=
 =?us-ascii?Q?Bd8N9mD+PnCHTmSR+TR7yJxglYBolcTeCOroZZoTwhRrLShP60rtAEt4CKkX?=
 =?us-ascii?Q?3KVVvO+U5Gqjmm9DK//sxVblayL1kAqhBCH5gZCCt6w933otWNAzZXiszkkm?=
 =?us-ascii?Q?14kFl0/wymonT+XAAZNWwkZFEHwNS5zMGM6wTAaBg5ST1y67n9yRphMmaMfc?=
 =?us-ascii?Q?R6mrTt8V9M1OGW6QcdWAiQMf1GmNm9k98lSI1MeCe3IoFyChwmbnrJd3nwtt?=
 =?us-ascii?Q?+ZxMRcxHM9PEbCh6lGvTDacM3cNRRQnjePy6i/h/50DsFbctT45AQQMc6tn7?=
 =?us-ascii?Q?keyFj+Z6XM1V/W7NQhX4y0RtFULpwD89LeYuNhv2NBnBEoxgJdEtDA0IcbJ8?=
 =?us-ascii?Q?fBpAK4mttoZlrhkZc+0rMqIJZC3DWaAp9KClYJ6lQRv3x4blTf6Utx/kOKT3?=
 =?us-ascii?Q?LfbVyvaQcmOLZT0L4ZmeznVhZmIxeHmTitgEk/uyabKjOg4PBUuTCIZXOG2T?=
 =?us-ascii?Q?eCtq1+cIZOlXD4U8Z6XAnvS1sceNnui5Dxn2ieI+SZYSiRpnn6B5Mr8X0YXn?=
 =?us-ascii?Q?xHd45r/jDe3z9+92GvFF19x9XUKG2XZ2+ifequXIg2Xo+1yo0V5uRU/xPolp?=
 =?us-ascii?Q?vpoh6jQQ+oIGW7HGmTrByhkjzQTVsBiHFq68chGbRIFgkipFFI2xYMykaKCH?=
 =?us-ascii?Q?Q61oUQSou3SY9XP34sngv0+oFbqTkYBGbKxy0snQ/ozS4ZvSwIZLdWmC8mXy?=
 =?us-ascii?Q?O7eCUaffrGWZ67AUEaoUvHSgA56TriGtwTgdm0+QccEOGIDbHeca/7ABWf9r?=
 =?us-ascii?Q?k+RH6AsEHC6eXqqXmXUsWaWehtTQSFqsgLzXkD8Qrt6TVkssxIBUA9dMP5MW?=
 =?us-ascii?Q?BfB6tN3NpR3Zw4xQZblwPRGgXRHP0CNwrHav5CD1WKh2P6p3k8+HQ0s5irlZ?=
 =?us-ascii?Q?wxMaAjFusNi7DHyIutIXtw1T+6zlhuNUeY5ySCytdphIiFg7Va8l8QfQoyaG?=
 =?us-ascii?Q?mK5cH9nSAx4Q4a8pl86A1FvWBhcuFGnYaoALwWNYWeNIWASDyQ7rNWp+BEzZ?=
 =?us-ascii?Q?03lPKsEiyORAAOk5raYWbGnzy/XLixWSgvwAwwFlz5gtl601y3hmE2iGGkJ4?=
 =?us-ascii?Q?YVlxbkc13u+U5QicWqIGj/IgIntS72UcWHo01o4Q2hox?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <F1A97DF26DBCDB4CA27143DC92C2A9C3@namprd15.prod.outlook.com>
MIME-Version: 1.0
X-OriginatorOrg: meta.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR15MB5109.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7287fa8d-0855-41a6-07a1-08db09596244
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Feb 2023 22:19:30.7953
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: f493Mktkb+IXI0ciqf2cl6e9bfEasfk6awFYfBE6MXQKhEeYYidgKH/jGFsUjd6ezHCz/Dymsvfgbjj4BnHPXA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR15MB4521
X-Proofpoint-GUID: W2fmatTDA3rk1dYnD1ZgSUDsawH4upUZ
X-Proofpoint-ORIG-GUID: W2fmatTDA3rk1dYnD1ZgSUDsawH4upUZ
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-02-07_13,2023-02-06_03,2022-06-22_01
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Luis

> On Feb 6, 2023, at 4:28 PM, Song Liu <song@kernel.org> wrote:
> 
> +static void free_mod_mem(struct module *mod)
> +{
> + /* free the memory in the right order to avoid use-after-free */
> + static enum mod_mem_type mod_mem_free_order[MOD_MEM_NUM_TYPES] = {
> + /* first free init sections */
> + MOD_INIT_TEXT,
> + MOD_INIT_DATA,
> + MOD_INIT_RODATA,
> +
> + /* then core sections, except rw data */
> + MOD_TEXT,
> + MOD_RODATA,
> + MOD_RO_AFTER_INIT,
> +
> + /* last, rw data */
> + MOD_DATA,
> + };
> + int i;
> +
> + BUILD_BUG_ON(ARRAY_SIZE(mod_mem_free_order) != MOD_MEM_NUM_TYPES);
> +
> + for (i = 0; i < MOD_MEM_NUM_TYPES; i++) {
> + enum mod_mem_type type = mod_mem_free_order[i];
> + struct module_memory *mod_mem = &mod->mem[type];
> +
> + /* Free lock-classes; relies on the preceding sync_rcu(). */
> + lockdep_free_key_range(mod_mem->base, mod_mem->size);
> + if (mod_mem->size)
> + module_memory_free(mod_mem->base, type);
> + }
> +}

If there are no other comments, would you mind make the following 
change when applying the patch?

Thanks,
Song


diff --git i/kernel/module/main.c w/kernel/module/main.c
index b9617d7e8db2..c598f11e7016 100644
--- i/kernel/module/main.c
+++ w/kernel/module/main.c
@@ -1211,7 +1211,7 @@ static void free_mod_mem(struct module *mod)
		MOD_RODATA,
		MOD_RO_AFTER_INIT,

-		/* last, rw data */
+		/* rw data need to be freed last, as it hosts mod */
		MOD_DATA,
	};
	int i;