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: <0e55585d-d6d7-b9ac-aed5-8df56895e209@amd.com>
Date:   Fri, 2 Jun 2023 13:43:40 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v3] amdgpu: validate offset_in_bo of drm_amdgpu_gem_va
Content-Language: en-US
To:     Chia-I Wu <olvaffe@gmail.com>, dri-devel@lists.freedesktop.org
Cc:     Alex Deucher <alexander.deucher@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Felix Kuehling <Felix.Kuehling@amd.com>,
        Philip Yang <Philip.Yang@amd.com>,
        Hawking Zhang <Hawking.Zhang@amd.com>,
        Mukul Joshi <mukul.joshi@amd.com>,
        Danijel Slivka <danijel.slivka@amd.com>,
        Lang Yu <Lang.Yu@amd.com>,
        =?UTF-8?B?TWFyZWsgT2zFocOhaw==?= <marek.olsak@amd.com>,
        Jammy Zhou <Jammy.Zhou@amd.com>, amd-gfx@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
References: <20230601224419.2392422-1-olvaffe@gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20230601224419.2392422-1-olvaffe@gmail.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?SlZkSzFITkRMS21LYndxZ3Y0Z2hrN2t1Z29POHFtdzZXdE1XQ0o3cG1Gbk4w?=
 =?utf-8?B?cytVL2JZZUNUVXlLWHBrQ3JEa1A3V2tDQ0VobTNCK0VzNDBvY1hLYnB3QjFq?=
 =?utf-8?B?anRWRmU3WnhHWmdXaU1MbmM1NXBpZEVnNTg3b2RHbHdFZk1wbjlzeStHQ3R6?=
 =?utf-8?B?c3JDdlgrZGt1Sm9JZWlJY2hjK3EwWWNJUnNjOHVVQlZKVW5PeEQwaURDM1F3?=
 =?utf-8?B?YXcrejB5V3VhUzRUSmhBQWFVaXB3N2NLVTZMeG1iTUtkQTM3Mkdjd1hvcGJn?=
 =?utf-8?B?YTJYbEFVMTFjOFg5TlBEV1VEeHZSRC9HS3dZcE9PbzFQWFpSc3JIcmYxdWtj?=
 =?utf-8?B?Y0FLOUl4cWxnQTVJbmIycnIyeGVXbHNHM3orMk1kTUR2eE8xVUVIeW84QW1P?=
 =?utf-8?B?SHdUeDAzTGN3eW9hM3ZiUU4vVzZFckxJVU1IazR3NDdQbHVFeEFVWGhMY2VL?=
 =?utf-8?B?ME9kUmdRcU9HWmx0czRwdGo4V2RJckhpK3hHQ0VBNy8wRnFINlZENkRBUXdF?=
 =?utf-8?B?QnZVUWZvelptdGs0bXZhUHRCWXlmaG4vekJEcDNSVkk3SDgvV0VKdFVvenF4?=
 =?utf-8?B?Mnl5R0IwT3BUaGw0MUJqWDBCTUdPanNwTWcySFdaTDdrMzhGRFNJdWlvWDFR?=
 =?utf-8?B?Q3J4c21pUm53U3c3NG10OHlEblhNZlZYL2tVMWJHR1NReVNEeFlQd2hackpa?=
 =?utf-8?B?U1pHRE9EcFJOQkpReUhucm8wWUE2NVhkR2tkQ1RiZHZEb0hIaUppbzk3U25u?=
 =?utf-8?B?ZzUvZGduajBSbGJocmVIVXMrS3lFRE9KSk9ONWlqSGZqaDVSY1o2ZGZnejVz?=
 =?utf-8?B?ZlpRNFprM0pPcktFNVZjSWJrYlp6MkI1KzNGWHhIS1MrR0dkaUFPRmd3NFlR?=
 =?utf-8?B?eXRwdzFEZVU2TnBXZFNZOWJ5RUE2cVJKZnE3dDlpZzExWU9oNG51UURLa0pP?=
 =?utf-8?B?eDJiSVJlWkw2ekxFRFZsdjlXTVBRdHA5YkZudnhaUDRLOGQ4RmRUVCtQbzI4?=
 =?utf-8?B?ZTFzL2JWZTB4K1VYVHNvcGMzUmlVODFtZ1lIR1RXS0pjZFNsWWVUNjhEWEZT?=
 =?utf-8?B?VytJV2lvN28wazBDOGFFTzB3Wlg2dXFxckxOZlJQOXF0YmFpeFJTaHo2YU5m?=
 =?utf-8?B?a2Iwc1phWW5aTUVJMEMyT2dJTW9TWDRvT2ZNNU9leG9mcXEraXV0WllyK09Z?=
 =?utf-8?B?SVQ0WHZCTHRVLzZ0SFpEYy9BSGg3SFNBMzh2U3RWV0c0aWpUV3l3dFM4RExx?=
 =?utf-8?B?NEhjS2dSSC9zSGd5OGRjRStFRTlIT3dva3MzV0E5NVB4QzB4dUpFZnZqeDhJ?=
 =?utf-8?B?WXFIOEkvclJ5NTNvYW51ZmtTeHpVU2FObG1NRTA3dTQzb3YwdDVOWi9lQ1hk?=
 =?utf-8?B?UEtpMlBaMWhLOG96WDJXOG9ndVF0VWJYbDB6WXBneGY5NHl4ZkpwVG1FaWc2?=
 =?utf-8?B?dUJkL3Z2em5pYW56cktvTFIwVld0UHMySHNWbUltSHN5Q0czTXY4Y3pBcnNH?=
 =?utf-8?B?SmNDbTRXNmdYUGN4Vzd4ZE80ZkJLOUNyNll6ZlJYd3pwdTVwRVlyMmx1SDRS?=
 =?utf-8?B?NFJjSkZ0TTVZT1cxbERaS1REVk5yRFZOUE5sTnNGWmhKYjR1Rm9EZmxvUzNI?=
 =?utf-8?B?ZS9wYyswTXFxSTgzM1phVmkxM2M0a0dpdURiU0Zhb0plLzhvVDNsSFEyd3Zh?=
 =?utf-8?B?TUhzNzRjS3BKWWwyR3FaTXBtVzVSKzBoQnZHWXJmTHlNVEpPNWYzSldrS1Bh?=
 =?utf-8?B?V3BxNXNIaEY4M1p5dWRyZ05hb3ZDakoxelNNUVlBQ0huZlFGTGxoSmYzcWhU?=
 =?utf-8?B?VVRHc092Ry9hUForTmorZkVyZHdhMUVFLzdYZkRIRjBwK2tjano5Tmoxd0RO?=
 =?utf-8?B?RnJIVUloK0QveHZxek5uTnpuamZ0RmJhZkUwZGg5T1pkYTJOS2NiaXNUUmpD?=
 =?utf-8?B?UXdHdGRwZVFHVURzcVBpRlFjTE1yOUhQTGt4STNjUDdzVVVPU0xPcTJBOEhw?=
 =?utf-8?B?c1lGMmlFdHJ0YU9MNHJOYlNOOXp6dkM3N1RrSU9ycCt4MHJHZ3M5bHdSWkpp?=
 =?utf-8?B?ZDVNQUNBVlVnTzdtZ0VYdkc3V3F2ZEU5Z0ZDbk85OFduUUlmYVFFanBtVWJr?=
 =?utf-8?B?ZVJ0TURZb3ZrMlpKS2xTL1hJNnpCbVNTdGk3QzZQcW9SUHgwTThBR0dwVFhm?=
 =?utf-8?Q?b4Os9kXqA8VIp9VT69ZPYPe4WGGegH/lyEXwLETKWwJ7?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bf1429fa-77b1-4ffc-63e5-08db635ea05f
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Jun 2023 11:43:47.4184
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: V+kmvz2YylqPCEC3ShhMkRYdTXWzLM831Kjf3OylkGHUyg+21zmxCGkLOOCnFe4g
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR12MB8716
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no 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

Am 02.06.23 um 00:44 schrieb Chia-I Wu:
> This is motivated by OOB access in amdgpu_vm_update_range when
> offset_in_bo+map_size overflows.
>
> v2: keep the validations in amdgpu_vm_bo_map
> v3: add the validations to amdgpu_vm_bo_map/amdgpu_vm_bo_replace_map
>      rather than to amdgpu_gem_va_ioctl
>
> Fixes: 9f7eb5367d00 ("drm/amdgpu: actually use the VM map parameters")
> Signed-off-by: Chia-I Wu <olvaffe@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 22f9a65ca0fc7..76d57bc7ac620 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1434,14 +1434,14 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
>   	uint64_t eaddr;
>   
>   	/* validate the parameters */
> -	if (saddr & ~PAGE_MASK || offset & ~PAGE_MASK ||
> -	    size == 0 || size & ~PAGE_MASK)
> +	if (saddr & ~PAGE_MASK || offset & ~PAGE_MASK || size & ~PAGE_MASK)
> +		return -EINVAL;
> +	if (saddr + size <= saddr || offset + size <= offset)
>   		return -EINVAL;
>   
>   	/* make sure object fit at this offset */
>   	eaddr = saddr + size - 1;
> -	if (saddr >= eaddr ||
> -	    (bo && offset + size > amdgpu_bo_size(bo)) ||
> +	if ((bo && offset + size > amdgpu_bo_size(bo)) ||
>   	    (eaddr >= adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT))
>   		return -EINVAL;
>   
> @@ -1500,14 +1500,14 @@ int amdgpu_vm_bo_replace_map(struct amdgpu_device *adev,
>   	int r;
>   
>   	/* validate the parameters */
> -	if (saddr & ~PAGE_MASK || offset & ~PAGE_MASK ||
> -	    size == 0 || size & ~PAGE_MASK)
> +	if (saddr & ~PAGE_MASK || offset & ~PAGE_MASK || size & ~PAGE_MASK)
> +		return -EINVAL;
> +	if (saddr + size <= saddr || offset + size <= offset)
>   		return -EINVAL;
>   
>   	/* make sure object fit at this offset */
>   	eaddr = saddr + size - 1;
> -	if (saddr >= eaddr ||
> -	    (bo && offset + size > amdgpu_bo_size(bo)) ||
> +	if ((bo && offset + size > amdgpu_bo_size(bo)) ||
>   	    (eaddr >= adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT))
>   		return -EINVAL;
>