linux-kernel - Re: [PATCH] block: prevent merges of discard and write requests

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: <AANLkTikxw739=mZ2UZatSS8dTBb7+wz-AAL5uGBnM73N@mail.gmail.com>
Date:	Mon, 27 Sep 2010 01:12:39 -0400
From:	Mike Snitzer <snitzer@...hat.com>
To:	Jens Axboe <axboe@...nel.dk>
Cc:	Kyle McMartin <kyle@...artin.ca>,
	Adrian Hunter <adrian.hunter@...ia.com>,
	linux-kernel Mailing List <linux-kernel@...r.kernel.org>,
	stable@...nel.org, Christoph Hellwig <hch@....de>
Subject: Re: [PATCH] block: prevent merges of discard and write requests

On Mon, Sep 27, 2010 at 12:59 AM, Jens Axboe <axboe@...nel.dk> wrote:
> On 2010-09-27 12:30, Kyle McMartin wrote:
>> On Sat, Sep 25, 2010 at 12:40:48PM +0200, Jens Axboe wrote:
>>> On 2010-09-25 12:36, Adrian Hunter wrote:
>>>> Add logic to prevent two I/O requests being merged if
>>>> only one of them is a discard.  Ditto secure discard.
>>>>
>>>> Without this fix, it is possible for write requests
>>>> to transform into discard requests.  For example:
>>>>
>>>>   Submit bio 1 to discard 8 sectors from sector n
>>>>   Submit bio 2 to write 8 sectors from sector n + 16
>>>>   Submit bio 3 to write 8 sectors from sector n + 8
>>>>
>>>> Bio 1 becomes request 1.  Bio 2 becomes request 2.
>>>> Bio 3 is merged with request 2, and then subsequently
>>>> request 2 is merged with request 1 resulting in just
>>>> one I/O request which discards all 24 sectors.
>>>
>>> Wow, that's a disaster. We can now have requests in the
>>> same direction and of the same type (fs), but not mergeable.
>>>
>>> I would move the check up above the position calculations.
>>> I will apply this and upstream it right away. Thanks a lot!
>>>
>>
>> Jens, is this (the REQ_DISCARD hunk) required for stable as well? It
>> appears there's not much change relating to merging requests between
>> HEAD and v2.6.35, so I assume it is?
>
> No, 2.6.35 and earlier is safe, it's only 2.6.36-rc that is
> affected by this bug.

I'm not so sure... I think 2.6.35 is affected.  Jens, what do you hold
to be the regression point?

The initial discard merge implementation (commit e17fc0a1ccf88) always
set REQ_SOFTBARRIER for discards.  As such such non-ioctl
blkdev_issue_discard() discards were always !rq_mergeable().

As commit e17fc0a1ccf88 shared: it is always possible to send a
discard without the use of a barrier (but such callers were assumed to
be safe, e.g. mkfs.* discarding entire device via ioctl?).

But things really broke once we started playing games with barrier
flags associated with discards.  The regression point (relative to
discard merging) seems to have occurred when we got away from using
REQ_SOFTBARRIER with commit: fbbf055692aeb "block: fix DISCARD_BARRIER
requests".  Which was still committed to v2.6.35...

So long story short: it seems stable's 2.6.35.y needs this fix too.

Of course I could easily be missing something.. if I got any of this
wrong please correct me ;)

Thanks,
Mike
--
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/