linux-kernel - Re: [PATCH v2 4/6] iomap: correct the dirty length in page mkwrite

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: <53da5ebf-3d91-7c70-1be9-4c8a5485cd17@huaweicloud.com>
Date: Sat, 17 Aug 2024 14:43:10 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, djwong@...nel.org, hch@...radead.org,
 brauner@...nel.org, david@...morbit.com, jack@...e.cz, yi.zhang@...wei.com,
 chengzhihao1@...wei.com, yukuai3@...wei.com
Subject: Re: [PATCH v2 4/6] iomap: correct the dirty length in page mkwrite

On 2024/8/17 12:45, Matthew Wilcox wrote:
> On Mon, Aug 12, 2024 at 08:11:57PM +0800, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@...wei.com>
>>
>> When doing page mkwrite, iomap_folio_mkwrite_iter() dirty the entire
>> folio by folio_mark_dirty() even the map length is shorter than one
>> folio. However, on the filesystem with more than one blocks per folio,
>> we'd better to only set counterpart block's dirty bit according to
>> iomap_length(), so open code folio_mark_dirty() and pass the correct
>> length.
> 
> We shouldn't waste any time trying to optimise writing to files through
> mmap().  People have written papers about what a terrible programming
> model this is.  eg https://db.cs.cmu.edu/mmap-cidr2022/
> 
> There are some programs that do it, but they are few and far between.
> 

I don't think it's an optimization, this is a fix, the issue is the same
to the one that previous 2 patches want to fix: there left a hole with
ifs dirty bit set but without any valid data and without any block
allocated/reserved. This mmap() path is just once case that could lead
to this issue (The case if as I said in my reply to Christoph, suppose
we have a 3K regular file on a filesystem with 1K block size. If the
folio size is 4K, In iomap_page_mkwrite(), it will mark all 4 bits of ifs
dirty, the last one will become redundant if we expand the file to 4K
later).

Hence in my opinion, we need to fix this, or it's gonna be a potential
problem one day.

Thanks,
Yi.