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 X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E6BCBC433E9 for ; Thu, 18 Feb 2021 12:30:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9C23C64EB6 for ; Thu, 18 Feb 2021 12:30:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230508AbhBRM3O (ORCPT ); Thu, 18 Feb 2021 07:29:14 -0500 Received: from mx2.suse.de ([195.135.220.15]:38734 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232113AbhBRKp7 (ORCPT ); Thu, 18 Feb 2021 05:45:59 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1613645111; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ZUYdGfBovPQofBTZOwpzeDRcMqQI0Cs/F1EUZmLN1Og=; b=j/Q23nWy3DDqB0knZYFLM3ufccVH6Vlqjyra2RiO1s3T7UvBaB7b3ZemTGW7q68CP70eEU wg5LP8GGVgjojxP3IK1rSuckmoksDNdmSiTEUFU3PiM6dbdXk6O/j9UV+uZnxX2PRqeGjL 260IOflu7cjHtraYDwFafFfcr/wtxf4= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 1D530AE04; Thu, 18 Feb 2021 10:45:11 +0000 (UTC) Date: Thu, 18 Feb 2021 11:45:10 +0100 From: Petr Mladek To: Chris Down Cc: linux-kernel@vger.kernel.org, Sergey Senozhatsky , John Ogness , Johannes Weiner , Andrew Morton , Steven Rostedt , Greg Kroah-Hartman , Kees Cook , kernel-team@fb.com Subject: Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 2021-02-17 16:32:21, Chris Down wrote: > Chris Down writes: > > open(f); > > debugfs_file_get(f); > > fops->open(); > > inode->private = ps; > > debugfs_file_put(f); > > > > remove_printk_fmt_sec(); /* kfree ps */ > > > > read(f); > > debugfs_file_get(f); > > fops->read(); > > ps = inode->private; /* invalid */ > > debugfs_file_put(f); > > Er, sorry, inode->private is populated at creation time, not at open(). The > same general concern applies though -- as far as I can tell there's some > period where we may be able to _read() and `ps` has already been freed. Honestly, I am a bit lost here. Also I have realized that you needed to release the struct from the module going notifier. And there you have only pointer to struct module. The thing is that I do not see similar tricks anywhere else. Well, other users are easier because they create the debugfs file for it own purpose. In our case, we actually create the file for another module. Anyway, we are going to use the seq_buf iterator API. IMHO, the seq_buf iterator functions should be able to get the structure directly via the data pointer. I wonder if it is similar to proc_seq_open() and proc_seq_release(). It is using the seq_buf iterator as well. It is created used by proc_create_seq_private(). This API is used, for example, in decnet_init(). It is a module, so there must be the similar problems. All data are gone when the module is removed. The only remaining problem is the module going notifier. For this purpose, we could store the pointer to struct module. There are many other fields for similar purposes. I am pretty sure that the module loader maintainer will agree. The result will be using some existing patterns. It should reduce problems with possible races. It should make the life easier for all involved APIs. Best Regards, Petr