Re: [PATCH v3 4/6] blktrace: fix debugfs use after free
From: Luis Chamberlain <mcgrof@kernel.org>
Date: 2020-05-01 15:24:53
Also in:
linux-fsdevel, linux-mm, lkml
On Wed, Apr 29, 2020 at 02:57:26PM +0200, Greg KH wrote:
On Wed, Apr 29, 2020 at 12:21:52PM +0000, Luis Chamberlain wrote:quoted
On Wed, Apr 29, 2020 at 05:04:06AM -0700, Christoph Hellwig wrote:quoted
On Wed, Apr 29, 2020 at 12:02:30PM +0000, Luis Chamberlain wrote:quoted
quoted
Err, that function is static and has two callers.Yes but that is to make it easier to look for who is creating the debugfs_dir for either the request_queue or partition. I'll export blk_debugfs_root and we'll open code all this.No, please not. exported variables are usually a bad idea. Just skip the somewhat pointless trivial static function.Alrighty. It has me thinking we might want to only export those symbols to a specific namespace. Thoughts, preferences? BLOCK_GENHD_PRIVATE ?That's a nice add-on issue after this is fixed. As Christoph and I pointed out, you have _less_ code in the file if you remove the static wrapper function. Do that now and then worry about symbol namespaces please.
So it turns out that in the old implementation, it was implicit that the request_queue directory was shared with the scsi drive. So, the q->debugfs_dir *will* be set, and as we have it here', we'd silently be overwriting the old q->debugfs_dir, as the queue is the same. To keep things working as it used to, with both, we just need to use a symlink here. With the old way, we'd *always* create the sg directory and re-use that, however since we can only have one blktrace per request_queue, it still had the same restriction, this was just implicit. Using a symlink will make this much more obvious and upkeep the old functionality. We'll need to only export one symbol. I'll roll this in. Luis