Received-SPF: pass (google.com: domain of linux-kernel+bounces-155280-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <c2bf36b5-3145-491b-b272-d5afae73db7f@oracle.com>
Date: Tue, 23 Apr 2024 18:56:52 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] fbdev: fix incorrect address computation in deferred
 IO
To: Thomas Zimmermann <tzimmermann@suse.de>, Nam Cao <namcao@linutronix.de>,
        Jaya Kumar <jayalk@intworks.biz>, Daniel Vetter <daniel@ffwll.ch>,
        Helge Deller <deller@gmx.de>,
        Javier Martinez Canillas <javierm@redhat.com>,
        linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
Cc: tiwai@suse.de, bigeasy@linutronix.de, patrik.r.jakobsson@gmail.com,
        Vegard Nossum <vegard.nossum@oracle.com>,
        George Kennedy <george.kennedy@oracle.com>,
        Darren Kenny <darren.kenny@oracle.com>, chuansheng.liu@intel.com,
        stable@vger.kernel.org
References: <20240423115053.4490-1-namcao@linutronix.de>
 <1722c1b6-59a5-429d-905c-bc1951a68a68@suse.de>
Content-Language: en-US
From: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
In-Reply-To: <1722c1b6-59a5-429d-905c-bc1951a68a68@suse.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?TzJhdE5PTkQwOVd4NkVyRjZLRDdXV3JBeElPYTR6WnlOL3ZiQ0wxenBDcHVa?=
 =?utf-8?B?cXZVVXFVZTN2Q0JQRnlrNDZSY3phUGdiQ1k0eUFIZHZ2RTlGR3NxTmVFOWN5?=
 =?utf-8?B?cDhvaDFTN0ZPUm1HVmdBNzJUdlkxR0tXWW5YYzVSVjRYR0N1NmhyMjdmNm5O?=
 =?utf-8?B?MkYxQk9MVWhpTlc0clpXaFVKTHV1ZWkrQVl6S1h5TW11TldyZEx2QU1XSm9R?=
 =?utf-8?B?K2luNkJxMTRUNEEwY2h6bmVwZkFIb2VhQ1BwMkpjSHUveWw1Z1pQVG55LzRC?=
 =?utf-8?B?dGVQRUxGZ1VmdUJTMVBjYXI4eCs4T2RoS1dRVlVJa2hsdjJCTXRKYXlvYVcz?=
 =?utf-8?B?YnNhcVEzeEowUDRBNFpTVEZic004VytZNzZNVHFmWmhZdXJGZ0wxNnJJYU1Q?=
 =?utf-8?B?dEpUK0NoUmFtYjFDV042S2I0S1JTMUsyQytYVGVKV3djL1g2T0FVWTU0NkJm?=
 =?utf-8?B?MHZvQVNKeFUzb1N0NDZWUXZDTEFBc1oxclF2eFhnMERyOE9KNVUwNmxJRVBN?=
 =?utf-8?B?T1FWUzkyR3pvWnpDV3N4VmN6bVJjZUlDMjMzSmdzNGRxMjQ2djlZQUNBbkZR?=
 =?utf-8?B?dUFyUTJkclQ5YWlFTm5TN2I2d0Fobjh4VGlDTUwwTnFQWitPSmpRWWd2MVpG?=
 =?utf-8?B?eHZ5T3ZQSGVaZEdVTFNHM3QralUvMGRPVDYvdVd6ODloc0JTM2U2dEp2OVpH?=
 =?utf-8?B?V3lnK0hJSWVOY2FSVzlVMWZLMGg2emRTcWlRWUdHbWFYajFlUXFQNlltUW1C?=
 =?utf-8?B?SVlBaWVoalRUczd3YzZZVUlkUS8rYmlybFliUXlKOExpOTZEVTJsNU1VbjRE?=
 =?utf-8?B?Sms5ejd5dFBhWlliY0s5TUs4dnI5Zk0vMWFEVlBHejlycWw4MGR6WDB2NjdE?=
 =?utf-8?B?N0tTeU5CN1JGSU1NazZPek9lT21oRXFTRGFuZ1ZqMTFXOFJYVmNiSFY0Q3hO?=
 =?utf-8?B?WE9FaEZPUXhKRjJpanBwYWlTdWkwZlV3VU5vQzBKYXgyb0RlYmttcDB6Tjhx?=
 =?utf-8?B?TEVmRUlIcmpCQXg2MTE0ZmJKcWxXYktCTlRKUkJaaXdiR2ZGNmtWUDhFdHp2?=
 =?utf-8?B?Q2lVYk1wcW43QXRDQ1pESWFMU2xrTG9oTDJmZzcyR21lSmZwdlBvcGdTZ3d1?=
 =?utf-8?B?aEpzczJnTENKS3hWamhuYkhKOEJUb3FqL1hnekxPZzNjay9ZYUZPZHJGb0Y4?=
 =?utf-8?B?MXQzVHJHdkpDUkRFZ255TmplTUtTOTlvbmViMGRLQUN3dE9qKzVRNVd3a0JV?=
 =?utf-8?B?MFZ1MDZMU2srVDcya0h1UXNwMjRSVnBFb292R2s0ZWRvZCs0WGw0YTgyallO?=
 =?utf-8?B?MFpuZFJZRmZNbHJObUNBWjViZitkV1EyenVzeERBV0ZvdTRvS28xOFJzeTFn?=
 =?utf-8?B?QWlvMHZyUnlCc3llNmx6ZXVxR2o4S01KblpRamhzZDFyQlk3VzlNNnFVZm9K?=
 =?utf-8?B?VVBpRnBYdzczZWsvRXFMdmEyTUd5V1ZtUkVqMXIvRFFvMWU5czVhdkFYOUNx?=
 =?utf-8?B?a2RYMUhPM2NaaUEyTWNCYVdhc2RaMFN3bXpLdnNORzFWN2hVdmUyQWNzVmxU?=
 =?utf-8?B?NjBocGpzNUdJRFVqb2R3Rnkra2hCcy9KUzlXZUcwUHFRYVNUSU14NzNiT0No?=
 =?utf-8?B?TG9UN3NibmpyRVBHc0Y4NVZTbWFBN3NHN2FIa2F2a2FNNUVDVkwrV0svQnc0?=
 =?utf-8?B?dlNUd3JHeURjM2FZZVNmWkNJSHpGYlhlWHJGam1SazJKeU1XWitXRVJRYXpy?=
 =?utf-8?B?MVdYczVzc29WblBsYmdCOFcyVzNZZE1hRVBrQ2NYZlpwNHZGTi9HOUlwR0pp?=
 =?utf-8?B?R01SL3JMQWpPcmhlRE53eWdmTXY3Wm5ZMWhBODNBREJyMnR4dFExMFk3Vis2?=
 =?utf-8?B?blp3Q2pDdXpoYmZoSHl4dGxjYXBtSkRDd1VGaWx5cUc4YzR6STFLc3BrUUhG?=
 =?utf-8?B?VG8rdkZvZ1hTb2gyaXEweDJFeXhvNkxtekFZVXVWY2NwZkdVSjVoaWxuUnk5?=
 =?utf-8?B?ZEMwT3AzSWZJNFdiYjE4YmpwRGNKOUF6Mm5GNzBMMXlIVVdPeEN6KzZOcE5J?=
 =?utf-8?B?bFNMbTVsekIrR0oyem5wNkNScndnL0duWWpCODF1eWhuVGVGZDRaQ1l4d2NQ?=
 =?utf-8?B?dy9pMlhNaC9TRzRlSkwzcXU2cXhHWnZaNjBBcFErNXlHRmNWbjFWUDhuOU5P?=
 =?utf-8?Q?3G4DvJUZHOhKIY/OPm424R0=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	9ERhjl2NqTaCb/nzIty9FZFqI5xr2G2fnRIpZOQzBx4Cg93AcCC+7/26PKHeOiL43gOKAzAPljQPHBhrgqaVWOBOzTzlAq44TqQycTpLm+IMmfdAijfw76Lpfxtd4ZIlV/p2+3bKxLLvSVA5nOI8dHZWDmDRFTYDFON4qdZ1PS0j+19iHTC5bOrfKSmi9to5f/X5+j0vPhW5GbNfAJq29XQRKbbo5nabZp2MAxXuHV57MNTYz+pXKHIwhooY/L2fP2hQDwY6a9V9TFu0OqvIoIfM90PSl/FqVN8eCq8VXQHsybYwGq1gEaZaMlwWpSIFcHV6390bgdMvPVev5KXI6JnfseHyEyN+x0z7lC6zxfZMJN4LFNuWCzEy9+cEVu19QGGSZf3sje2GKsnsdxp83R3rrXQwuSERYxJ2BmK0vp/hq0xzKkHKG2tQ/BSRUyq+PknCUUtxzOAKGv5c508qpcK/VX6Z8dNDheuxospnFCP75RH5t3JQ1f5dLjPxJxmtcMrYDcrDoRN+p4PPy8NnSOx17oVXKJkoekOC1w0V97ed8rmxrTBeS/LNMWHSpDSGahyjMOLp1P1nFbm5G2UrsHF8CBUfYjpkz6CMlcfUgdA=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b0fb3b9e-714c-40f5-1a5f-08dc63990ff4
X-MS-Exchange-CrossTenant-AuthSource: PH8PR10MB6290.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Apr 2024 13:27:03.2220
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Z8cN3miNXP2dRpsbjEppHkhTPEo6xlyjO+wFPIqK6Uur2J9ZIW4qeLjlfzAVRBcux3I8bQsqlK0zz+1bnOfbjinU2UGGlP4ykGxxWfjTLshqBnNF8jyK7ZZOUtuE9uBM
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB4869
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-04-23_11,2024-04-23_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxlogscore=999
 mlxscore=0 phishscore=0 spamscore=0 malwarescore=0 adultscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2404010000 definitions=main-2404230034
X-Proofpoint-ORIG-GUID: IxEP8HjvjfCcDORTbagzNIHyYdueev_-
X-Proofpoint-GUID: IxEP8HjvjfCcDORTbagzNIHyYdueev_-

Hi,

On 23/04/24 18:37, Thomas Zimmermann wrote:
> Hi
> 
> Am 23.04.24 um 13:50 schrieb Nam Cao:
>> With deferred IO enabled, a page fault happens when data is written to 
>> the
>> framebuffer device. Then driver determines which page is being updated by
>> calculating the offset of the written virtual address within the virtual
>> memory area, and uses this offset to get the updated page within the
>> internal buffer. This page is later copied to hardware (thus the name
>> "deferred IO").
>>
>> This offset calculation is only correct if the virtual memory area is
>> mapped to the beginning of the internal buffer. Otherwise this is wrong.
>> For example, if users do:
>>      mmap(ptr, 4096, PROT_WRITE, MAP_FIXED | MAP_SHARED, fd, 0xff000);
>>
>> Then the virtual memory area will mapped at offset 0xff000 within the
>> internal buffer. This offset 0xff000 is not accounted for, and wrong page
>> is updated.
>>
>> Correct the calculation by using vmf->pgoff instead. With this change, 
>> the
>> variable "offset" will no longer hold the exact offset value, but it is
>> rounded down to multiples of PAGE_SIZE. But this is still correct, 
>> because
>> this variable is only used to calculate the page offset.
>>
>> Reported-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> Closes: 
>> https://lore.kernel.org/linux-fbdev/271372d6-e665-4e7f-b088-dee5f4ab341a@oracle.com
>> Fixes: 56c134f7f1b5 ("fbdev: Track deferred-I/O pages in pageref struct")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> 

Thanks everyone!

I have tested the patched kernel with the syzkaller reproducer and 
couldn't see a problem.

Regards,
Harshit

> Thank you so much. I'll take care of merging the patch later this week.
> 
> Best regards
> Thomas
> 
>> ---
>> v2:
>>    - simplify the patch by using vfg->pgoff
>>    - remove tested-by tag, as the patch is now different
>>
>>   drivers/video/fbdev/core/fb_defio.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/core/fb_defio.c 
>> b/drivers/video/fbdev/core/fb_defio.c
>> index 1ae1d35a5942..b9607d5a370d 100644
>> --- a/drivers/video/fbdev/core/fb_defio.c
>> +++ b/drivers/video/fbdev/core/fb_defio.c
>> @@ -196,7 +196,7 @@ static vm_fault_t fb_deferred_io_track_page(struct 
>> fb_info *info, unsigned long
>>    */
>>   static vm_fault_t fb_deferred_io_page_mkwrite(struct fb_info *info, 
>> struct vm_fault *vmf)
>>   {
>> -    unsigned long offset = vmf->address - vmf->vma->vm_start;
>> +    unsigned long offset = vmf->pgoff << PAGE_SHIFT;
>>       struct page *page = vmf->page;
>>       file_update_time(vmf->vma->vm_file);
>