Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a55d468a-7358-5079-a903-e2240a6e30ee@vivo.com>
Date:   Mon, 10 Jul 2023 17:37:28 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.1
Subject: Re: [PATCH] erofs: fix two loop issues when read page beyond EOF
To:     Gao Xiang <hsiangkao@linux.alibaba.com>,
        "xiang@kernel.org" <xiang@kernel.org>,
        "chao@kernel.org" <chao@kernel.org>
Cc:     "huyue2@coolpad.com" <huyue2@coolpad.com>,
        "jefflexu@linux.alibaba.com" <jefflexu@linux.alibaba.com>,
        "linux-erofs@lists.ozlabs.org" <linux-erofs@lists.ozlabs.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20230708062432.67344-1-guochunhai@vivo.com>
 <97875049-8df9-e041-61ca-d90723ba6e82@linux.alibaba.com>
 <d6ee4571-64d6-ebd2-4adb-83f33e5e608d@vivo.com>
 <fd738d38-17de-4b61-e4e8-d4f98ef8d1db@linux.alibaba.com>
 <ac05d6e3-79b4-a470-2a30-8c809c277209@vivo.com>
 <1a107593-e411-70a0-b6b8-3c34a9036ff3@linux.alibaba.com>
From:   Chunhai Guo <guochunhai@vivo.com>
In-Reply-To: <1a107593-e411-70a0-b6b8-3c34a9036ff3@linux.alibaba.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cGhNNTBOd0liMUtFTWF1L3FZdkFSQ0phUVNlWVZyVS9Ea2c1b2xlVUpQdVZZ?=
 =?utf-8?B?WGhZSCtQeVFPZGxrMTZlclR3akdWZXRPWFZ1bERWb0pIdVpManNZMnZRRzJC?=
 =?utf-8?B?T3NqQXN0b3B3VGh0QldLMzl0eHBCNlFRcHJyMVBFR0hiRUI2aXl1bCtqbmlB?=
 =?utf-8?B?VHR0em1xeWF6UVFQSWRqNDZadUpPOU1jdWVLWmluaXBBTHBvRVNiV2JWaVcr?=
 =?utf-8?B?M0RtN3o2bzgvYmFYTCt0NnR0WENJdkxyV1k1YTNveTFWTUtJdDRLd0IzeTBm?=
 =?utf-8?B?R0ZON2xtMkFOM2o3MGdCaTFCQ1ZOSVpOR1Q0SFVaNXNVOEhjRzhLUWtzRFgv?=
 =?utf-8?B?QVFkU25EaW12WGJHYjNJYXdwN1RKWUdESkJHVHk0YlJyYU50cXpHM0RHbEZH?=
 =?utf-8?B?cUpSYWJRUVNVOFRpaWxIcnk0dnB3NGNBd3NzbWRoRVl4K0doRGtFVXh3d3d1?=
 =?utf-8?B?d0pvUmZ3RlhOSlAxd0djS2N5aHNvMkN0L3F6b0tvL3EyVHJMRDVUdFB6b2Vo?=
 =?utf-8?B?ZituNmVJRlF6ODVSSHdCeEdlcUlwQ3REYkF1Q1BiMEtkNkpjaXFzdHhyWWFS?=
 =?utf-8?B?cnRUSFQrVHlHSE9sZVFRRHc0ZXVJTWI4MGZOSGtrd1FFWVNqRjRhajZqazdS?=
 =?utf-8?B?VHdueFJOQjNRTm5GNSt2SHlQQVJVVHFpclljaktHNlQwOUNqVUlQR2lTVUFE?=
 =?utf-8?B?d2dLcG9UeEltdWFUbUZQelpBVmNvNVc2d2k2R3RMUG9LZHByMHB2V0hEYnJU?=
 =?utf-8?B?SXRIc01qVFp2Q2dVT2RqREZJbzZwMThSOXorbjZUQ0JpeHhmS2Y4cWRHYmxk?=
 =?utf-8?B?VUZVdHN2NG1OZ2gxR3ZGbkVOSFpFU25ybEdCeGJNUG9UMTkrMDZUQ0liK21F?=
 =?utf-8?B?V1YxQjZtbTlvOFRtTFJoQlFIS2R6NXhEakVacE50ZTJDT05SUXhtdU1PWlVl?=
 =?utf-8?B?N2cxaTJUNThpMXBDRjcwQng0cGtwWVpKMWJvQms1NDNCR0hNUENSc25mbU5m?=
 =?utf-8?B?UHhQc2pEQlBVSTFBaVkxYnBWTFBTajRXYlo0ZWVIcmlKeGw1QVZKNFN4bzA4?=
 =?utf-8?B?bFRmamxjUGErK25ZL1NBd3NvUWtLQ1M3ZENyUlZjNUplQW5veWVsaUlWM2s2?=
 =?utf-8?B?SEZWTkNWUnozaWtaV0ZsODVrZjExVVZxZzBMYmtzeGt4YmU4S1RVVUM5UE9u?=
 =?utf-8?B?WkxHeGlYWFpaMFFadk9LS00rQkNQdVc4dHRKZ0NsLytIZWRBTjhJNXU4MnE5?=
 =?utf-8?B?UTlqK0M1ZHJYYTR2RHM2WVlnOGs5M3NIYUZCQzU2Q0hyRkEvcGZsL0I0VGJC?=
 =?utf-8?B?R0hVTFlBT05jVzNydm5taXB6UFR6SU1XK1ZoNWVQQ1pCK2E0dzNEYWtaQ1R1?=
 =?utf-8?B?KzBrRUFPM1h1QkcwZmozdzgwM2tIYWN0ZEt6THhRb1VsaFkyRG9SWitWejYx?=
 =?utf-8?B?ZVZkZXErNFNuam9MbGZ5U0k2MEh5RXQ2YUJXQVZ4eHRoSEFsMWpYSWJEU3Fy?=
 =?utf-8?B?d2ZTbUJPTTE5MzVxVlVwUVkxdnpJdUlMd21lUzE1VjFBK1dvY2RMMXNUUnc1?=
 =?utf-8?B?RFJTZVIvUXRDK2NucmlzOWFCbmN6Rkhuay9RVEFxVHJRaHlkZDFYbjhHaC9U?=
 =?utf-8?B?alk1azIrV2o5NUNTYk9rUENuWEt4RzR1b0F4aVd0Q1pia050UHNSVTg1YnRx?=
 =?utf-8?B?Rk9uaExXNDRudWdlMFcrRERvMGFmbVpkaExnQlFlWHk4L2NFL2xUOHZCV1Qw?=
 =?utf-8?B?YVR3aVcwVHY1YlRjdE5OUmpVd3NIQmplZTdFdE93YkFLbXNWbWQzTW9TT3NG?=
 =?utf-8?B?WnB1WW9pNFhMdlJLaTd5WEx3M3oxMGF2L3Y0b2h1ankxanJ1OFJUanF4WjFs?=
 =?utf-8?B?VDJJT2RmNnhITkVNT3ZoZmNWcHdjcXk0VCs4Y3dUSng3eWRpQUpnZEJtdERG?=
 =?utf-8?B?Tk5tUFdHN0dlRjZVZmhBa0lhNjEvNzRBSS95N0I4MnJ6Vy83VmFCbndEUWly?=
 =?utf-8?B?b0IxaCtQcW1lTVU0M3krSDVSOEp6azAxUkJsTnlwS2RBSENUYzZsY0VHM3pz?=
 =?utf-8?B?aVR0dHZsWnIwZGQyNnVQY1pITmJ6c1N5QzRzOEVzMElTdVl4UG1NZnd4c3U0?=
 =?utf-8?Q?nICPdtaT5iRFqqkfOew+0E6uX?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0204bb1b-1d97-4b6a-ab3f-08db8129491d
X-MS-Exchange-CrossTenant-AuthSource: SG2PR06MB3338.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jul 2023 09:37:32.4511
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Gn+bCT4hyz2+ixwAn5DrgD+M3EoX4jUnKA8E8WJN+G8CuyrMSvvJHjr0OBt5ouoINVKWlhsC4cfwgAAsttuzbw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PUZPR06MB5555
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 2023/7/10 13:02, Gao Xiang wrote:
> 
> 
> On 2023/7/10 12:35, Chunhai Guo wrote:
>>
>>
>> On 2023/7/10 11:37, Gao Xiang wrote:
>>>
>>>
>>> On 2023/7/10 11:32, Chunhai Guo wrote:
>>>> Hi Xiang,
>>>>
>>>> On 2023/7/8 17:00, Gao Xiang wrote:
>>>>> Hi Chunhai,
>>>>>
>>>>> On 2023/7/8 14:24, Chunhai Guo wrote:
>>>>>> When z_erofs_read_folio() reads a page with an offset far beyond EOF, two
>>>>>> issues may occur:
>>>>>> - z_erofs_pcluster_readmore() may take a long time to loop when the offset
>>>>>>       is big enough, which is unnecessary.
>>>>>>         - For example, it will loop 4691368 times and take about 27 seconds
>>>>>>           with following case.
>>>>>>             - offset = 19217289215
>>>>>>             - inode_size = 1442672
>>>>>> - z_erofs_do_read_page() may loop infinitely due to the inappropriate
>>>>>>       truncation in the below statement. Since the offset is 64 bits and
>>>>>> min_t() truncates the result to 32 bits. The solution is to replace
>>>>>> unsigned int with another 64-bit type, such as erofs_off_t.
>>>>>>         cur = end - min_t(unsigned int, offset + end - map->m_la, end);
>>>>>>         - For example:
>>>>>>             - offset = 0x400160000
>>>>>>             - end = 0x370
>>>>>>             - map->m_la = 0x160370
>>>>>>             - offset + end - map->m_la = 0x400000000
>>>>>>             - offset + end - map->m_la = 0x00000000 (truncated as unsigned int)
>>>>>
>>>>> Thanks for the catch!
>>>>>
>>>>> Could you split these two into two patches?
>>>>>
>>>>> how about using:
>>>>> cur = end - min_t(erofs_off_t, offend + end - map->m_la, end)
>>>>> for this?
>>>>>
>>>>> since cur and end are all [0, PAGE_SIZE - 1] for now, and
>>>>> folio_size() later.
>>>>
>>>> OK. I will split the patch.
>>>>
>>>> Sorry that I can not understand what is 'offend' refer to and what do you mean. Could you please describe it more clearly?
>>>
>>> Sorry, there is a typo here, I meant 'offset'.
>>>
>>> `cur` and `end` both are not exceed 4096 if your page_size
>>> is 4096.
>>>
>>> Does
>>> cur = end - min_t(erofs_off_t, offset + end - map->m_la, end)
>>>
>>> fix your issue?
>>
>> Yes. I think this will fix this issue. Do you mean the below change is unncessary?
>>   >>>> -    unsigned int cur, end, spiltted;
>>   >>>> +    erofs_off_t cur, end;
>>   >>>> +    unsigned int spiltted;
> 
> Yes, please help send a fix for this!
> 
> Thanks,
> Gao Xiang
> 

Got it. I have sent the patch. Please have a check.

Thanks,
Guo Chunhai