Re: [PATCH v2] blktrace: put bounds on BLKTRACESETUP buf_size and buf_nr

From: Christoph Hellwig <hch@infradead.org>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	Harshad Shirwadkar <harshadshirwadkar@gmail.com>,
	linux-block@vger.kernel.org, Chaitanya.Kulkarni@wdc.com
Subject: Re: [PATCH v2] blktrace: put bounds on BLKTRACESETUP buf_size and buf_nr
Date: Mon, 15 Jun 2020 07:04:38 -0700	[thread overview]
Message-ID: <20200615140438.GA21927@infradead.org> (raw)
In-Reply-To: <904b17e6-3ac6-8474-d6e5-32ada0dc2d40@acm.org>

On Mon, Jun 15, 2020 at 07:01:00AM -0700, Bart Van Assche wrote:
> On 2020-06-15 04:52, Christoph Hellwig wrote:
> > I don't think the configurability makes any sense.  We need to put
> > a hard upper cap on, preferably one that doesn't break widely used
> > userspace.  Just pick a reasonable large value to get started.
> 
> Hi Christoph,
> 
> Something that has not been mentioned in the patch description is that
> this patch addresses an issue that was discovered by syzbot. Do you
> agree that the following changes are useful on top of a hard upper limit
> for the blktrace buffer and that these changes will address more
> potential syzbot issues?
> - Use __GFP_ACCOUNT in the relay_open() call that allocates blktrace
> buffers such that the memory allocation is accounted to a process and
> such that the OOM killer becomes aware of blktrace buffer allocations.

Sounds ok. 

> - Making blkdev_close(), raw_release() and sg_release() shut down
> tracing in case the BLKTRACETEARDOWN ioctl has not been invoked. That
> will cause the blktrace buffers to be freed when e.g. the process that
> owns these buffers is killed.

While this sounds useful I think it might very well break existing
use cases.