linux-kernel - Re: [PATCH v4 3/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]

Message-ID: <20200511134406.GN11244@42.do-not-panic.com>
Date:   Mon, 11 May 2020 13:44:06 +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>,
        Christof Schmitt <christof.schmitt@...ibm.com>,
        syzbot+603294af2d01acfdd6da@...kaller.appspotmail.com
Subject: Re: [PATCH v4 3/5] blktrace: fix debugfs use after free

On Sat, May 09, 2020 at 05:58:55PM -0700, Bart Van Assche wrote:
> On 2020-05-08 20:10, Luis Chamberlain wrote:
> > Screenshots of what the debugfs for block looks like after running
> > blktrace on a system with sg0  which has a raid controllerand then sg1
> > as the media changer:
> > 
> >  # ls -l /sys/kernel/debug/block
> > total 0
> > drwxr-xr-x  3 root root 0 May  9 02:31 bsg
> > drwxr-xr-x 19 root root 0 May  9 02:31 nvme0n1
> > drwxr-xr-x 19 root root 0 May  9 02:31 nvme1n1
> > lrwxrwxrwx  1 root root 0 May  9 02:31 nvme1n1p1 -> nvme1n1
> > lrwxrwxrwx  1 root root 0 May  9 02:31 nvme1n1p2 -> nvme1n1
> > lrwxrwxrwx  1 root root 0 May  9 02:31 nvme1n1p3 -> nvme1n1
> > lrwxrwxrwx  1 root root 0 May  9 02:31 nvme1n1p5 -> nvme1n1
> > lrwxrwxrwx  1 root root 0 May  9 02:31 nvme1n1p6 -> nvme1n1
> > drwxr-xr-x  2 root root 0 May  9 02:33 sch0
> > lrwxrwxrwx  1 root root 0 May  9 02:33 sg0 -> bsg/2:0:0:0
> > lrwxrwxrwx  1 root root 0 May  9 02:33 sg1 -> sch0
> > drwxr-xr-x  5 root root 0 May  9 02:31 vda
> > lrwxrwxrwx  1 root root 0 May  9 02:31 vda1 -> vda
> 
> So this patch creates one soft link per partition at partition creation
> time instead of letting the blktrace code create one directory per
> partition when tracing starts?

Yes.

> Does this break running blktrace
> simultaneously for a partition and for the entire block device?

blktrace already has this limitation, one blktrace is only allowed per
request_queue, the next patch clarifies this, as we currently just error
out without telling the user what has happened.e user what has
happened.e user what has happened.e user what has happened.

> > +static struct dentry *queue_debugfs_symlink_type(struct request_queue *q,
> > +						 const char *src,
> > +						 const char *dst,
> > +						 enum blk_debugfs_dir_type type)
> > +{
> > +	struct dentry *dentry = ERR_PTR(-EINVAL);
> > +	char *dir_dst;
> > +
> > +	dir_dst = kzalloc(PATH_MAX, GFP_KERNEL);
> > +	if (!dir_dst)
> > +		return dentry;
> > +
> > +	switch (type) {
> > +	case BLK_DBG_DIR_BASE:
> > +		if (dst)
> > +			snprintf(dir_dst, PATH_MAX, "%s", dst);
> > +		else if (!IS_ERR_OR_NULL(q->debugfs_dir))
> > +			snprintf(dir_dst, PATH_MAX, "%s",
> > +				 q->debugfs_dir->d_name.name);
> > +		else
> > +			goto out;
> > +		break;
> > +	case BLK_DBG_DIR_BSG:
> > +		if (dst)
> > +			snprintf(dir_dst, PATH_MAX, "bsg/%s", dst);
> > +		else
> > +			goto out;
> > +		break;
> > +	}
> > +
> > +	/*
> > +	 * The base block debugfs directory is always used for the symlinks,
> > +	 * their target is what changes.
> > +	 */
> > +	dentry = debugfs_create_symlink(src, blk_debugfs_root, dir_dst);
> > +out:
> > +	kfree(dir_dst);
> > +
> > +	return dentry;
> > +}
> 
> Please use kasprintf() instead of k?alloc() followed by snprintf().

Sure thing.

  Luis