linux-kernel - Re: [PATCH] blktrace: fix race with open trace files and directory removal

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, 27 Sep 2013 14:56:12 -0400
From:	Jeff Moyer <jmoyer@...hat.com>
To:	Jeff Mahoney <jeffm@...e.com>
Cc:	Jens Axboe <axboe@...nel.dk>, Tejun Heo <tj@...nel.org>,
	Linux Kernel Maling List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] blktrace: fix race with open trace files and directory removal

Jeff Mahoney <jeffm@...e.com> writes:

> On 9/27/13 2:43 PM, Jeff Moyer wrote:
>> Jeff Mahoney <jeffm@...e.com> writes:
>> 
>>> There's a bug in the blktrace client where it will stop and tear down
>>> all of the tracing instances for devices it's opened whether it
>>> successfully completed the setup or not.
>>>
>>> By starting multiple blktrace processes on the same device, it's possible
>>> to permanently disable blktrace on that device. The cause is that when
>>> the first blktrace process to exit tears down the directory structure,
>>> the trace files are still held open. Debugfs removes the dentries for the
>>> open files just fine but the relay implementation doesn't remove the
>>> dentries until all of the references to the file are dropped. This means
>>> that if there are open files when debugfs_remove is called for the device
>>> directory, the directory is not empty and can't be removed. Since the
>>> shutdown of the blktrace structure xchg's the structure out, there's no
>>> way to clean up the directory and any new blktrace processes will fail
>>> to start because it can't create the directory.
>>>
>>> This patch adds a kref to blk_trace so that we can release it after the
>>> initial reference as well as all of the references accumulated by the
>>> relay files are dropped.
>> 
>> Can't we just do proper unwinding of errors in the do_blktrace_setup
>> function?  In other words, don't just blindly call blk_trace_free, but
>> instead just undo anything we've done.
>
> No. It's not the setup that's causing the problem. It's one process
> holding the trace files open while another process calls BLKTRACETEARDOWN.

Ah, right.  So, in that case I'd rather restrict the ioctl to just the
process that setup the trace.  Jens, Tejun, any opinions?

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/