Thread (33 messages) 33 messages, 6 authors, 2020-05-04

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help