linux-kernel - Re: [RFC v2 2/5] blktrace: fix debugfs use after free

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Apr 2020 19:58:05 +0000
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Bart Van Assche <bvanassche@....org>
Cc:     axboe@...nel.dk, viro@...iv.linux.org.uk,
        gregkh@...uxfoundation.org, rostedt@...dmis.org, mingo@...hat.com,
        jack@...e.cz, ming.lei@...hat.com, nstange@...e.de,
        akpm@...ux-foundation.org, mhocko@...e.com, yukuai3@...wei.com,
        linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Omar Sandoval <osandov@...com>,
        Hannes Reinecke <hare@...e.com>,
        Michal Hocko <mhocko@...nel.org>,
        syzbot+603294af2d01acfdd6da@...kaller.appspotmail.com
Subject: Re: [RFC v2 2/5] blktrace: fix debugfs use after free

On Thu, Apr 09, 2020 at 07:52:59PM -0700, Bart Van Assche wrote:
> On 2020-04-09 14:45, Luis Chamberlain wrote:
> > +void blk_q_debugfs_register(struct request_queue *q)
> > +{
> > +	q->debugfs_dir = debugfs_create_dir(kobject_name(q->kobj.parent),
> > +					    blk_debugfs_root);
> > +}
> > +
> > +void blk_q_debugfs_unregister(struct request_queue *q)
> > +{
> > +	debugfs_remove_recursive(q->debugfs_dir);
> > +	q->debugfs_dir = NULL;
> > +}
> 
> There are no other functions in the block layer that start with the
> prefix blk_q_. How about changing that prefix into blk_?

I the first patch already introduced blk_debugfs_register(), so I have
now changed the above to:

blk_debugfs_common_register()
blk_debugfs_common_unregister()

Let me know if something else is preferred.

> > -#ifdef CONFIG_BLK_DEBUG_FS
> > +#ifdef CONFIG_DEBUG_FS
> >  	struct dentry		*debugfs_dir;
> > +#endif
> 
> Please add a comment above 'debugfs_dir' that it is used not only by the
> code in block/blk-*debugfs.c but also by the code in
> kernel/trace/blktrace.c. Otherwise this patch looks good to me.

Sure, I'll do that.

In the future, after this patch set I'll follow up with another series
to clean that header file to make it easier to expand on proper
documentaiton with kdoc.

  Luis