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