Re: [PATCH v5 05/10] blk-mq: Unregister debugfs attributes earlier

Jens Axboe
2017-04-25T22:29:39Z

On 04/25/2017 03:24 PM, Bart Van Assche wrote:
> On Tue, 2017-04-25 at 14:30 -0700, Omar Sandoval wrote:
>> On Tue, Apr 25, 2017 at 01:37:40PM -0700, Bart Van Assche wrote:
>>> One of the debugfs attributes allows to run a queue. Since running
>>> a queue after a queue has entered the "dead" state is not allowed
>>> and triggers a use-after-free, unregister the debugfs attributes
>>> before a queue reaches the "dead" state.
>>
>> Still not happy with this commit message. I'd prefer:
>>
>> We currently call blk_mq_free_queue() from blk_cleanup_queue() before we
>> unregister the debugfs attributes for that queue in blk_release_queue().
>> This leaves a window open during which accessing most of the mq debugfs
>> attributes would cause a use-after-free. Additionally, the "state"
>> attribute allows running the queue, which we should not do after the
>> queue has entered the "dead" state. Fix both of these cases by
>> unregistering the debugfs attributes before this.
>
> Hello Omar,
>
> That's a very verbose description. How about this?
>
> Unregister the debugfs attributes before freeing of request queue
> resources starts to avoid that a use-after-free can be triggered
> through one of the debugfs attributes.

Personally I find Omar's commit message much cleaner to read, and
more easily understandable. We really don't need to be laconic in
commit messages.

--
Jens Axboe