linux-kernel - Re: [PATCH] ceph: fix end offset in truncate_inode_pages

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 02 Jul 2019 14:58:57 +0100
From:   Luis Henriques <lhenriques@...e.com>
To:     "Jeff Layton" <jlayton@...nel.org>
Cc:     "Ilya Dryomov" <idryomov@...il.com>, "Sage Weil" <sage@...hat.com>,
        "Zheng Yan" <zyan@...hat.com>, "zhengbin" <zhengbin13@...wei.com>,
        <ceph-devel@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ceph: fix end offset in truncate_inode_pages_range call

"Jeff Layton" <jlayton@...nel.org> writes:

> On Mon, 2019-07-01 at 18:16 +0100, Luis Henriques wrote:
>> Commit e450f4d1a5d6 ("ceph: pass inclusive lend parameter to
>> filemap_write_and_wait_range()") fixed the end offset parameter used to
>> call filemap_write_and_wait_range and invalidate_inode_pages2_range.
>> Unfortunately it missed truncate_inode_pages_range, introducing a
>> regression that is easily detected by xfstest generic/130.
>> 
>> The problem is that when doing direct IO it is possible that an extra page
>> is truncated from the page cache when the end offset is page aligned.
>> This can cause data loss if that page hasn't been sync'ed to the OSDs.
>> 
>> While there, change code to use PAGE_ALIGN macro instead.
>> 
>> Fixes: e450f4d1a5d6 ("ceph: pass inclusive lend parameter to filemap_write_and_wait_range()")
>> Signed-off-by: Luis Henriques <lhenriques@...e.com>
>> ---
>>  fs/ceph/file.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 183c37c0a8fc..7a57db8e2fa9 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -1007,7 +1007,7 @@ ceph_direct_read_write(struct kiocb *iocb, struct iov_iter *iter,
>>  			 * may block.
>>  			 */
>>  			truncate_inode_pages_range(inode->i_mapping, pos,
>> -					(pos+len) | (PAGE_SIZE - 1));
>> +						   PAGE_ALIGN(pos + len) - 1);
>>  
>>  			req->r_mtime = mtime;
>>  		}
>
> Luis, should this be sent to stable? It seems like a data corruption
> problem...

Yes, I believe so.  But I believe all the active stable kernels that
include commit e450f4d1a5d6 (or a backport of it) will pick it anyway
due to the 'Fixes:' tag.  AFAIK only 5.1 and 5.2 are affected.

Cheers,
-- 
Luis