Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <4ca00dee-86b2-56b0-423a-76ef28260385@amd.com>
Date:   Sun, 3 Apr 2022 17:05:30 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH next v2] dma-buf/sync-file: do not allow zero size
 allocation
Content-Language: en-US
To:     Pavel Skripkin <paskripkin@gmail.com>, sumit.semwal@linaro.org,
        gustavo@padovan.org, daniel.vetter@ffwll.ch
Cc:     linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org
References: <2d1f9ba9-ea2a-e41c-eae6-0ba348cdf202@gmail.com>
 <20220401213114.11956-1-paskripkin@gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20220401213114.11956-1-paskripkin@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 2
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?L1Bzcm1RQ0luTmYrcU5mNDNhbnZkRXRlRmdDS044NWNIVnRWVGNnRzJhcHJq?=
 =?utf-8?B?UDhBaUFVZDhicXI4ak5HVDRIMk9wd3NpUStqTlppblJKTVBlL1Qya1RDTmtq?=
 =?utf-8?B?TmVaaitJOHFZNlZXN1RZUUlkVnZXc1BwcHlLR1dmVlhCY0RzbHgzRnQwNXVS?=
 =?utf-8?B?aXNmdjRrakZQRndzYVFvbEFGRGdDYnVxRmdVWXBuUytEZkxvbDlTMmsrWVpq?=
 =?utf-8?B?ZWN3K0FpZE1CSEplNGpHK2V4QVBvSHg4QzRHTjFxWGRpMXpMNFNqUXFUbzRj?=
 =?utf-8?B?M1RnamlmMWdhK0FtMk0zd3dCUndNVkFOQlFrRXVUOGJrcUlrbDFJN29YTkFR?=
 =?utf-8?B?Q0dzUy9NMzVLT091RXg5RjNxd0IyTWgyMU1ucjdDazdXSEFVTU85dndMdm1i?=
 =?utf-8?B?bGkxTW9xa3VNRHRJN2hNTWdzZDA4TGtJQm5nSW94VzdmNytLS3BzMWhFdVlu?=
 =?utf-8?B?NkVQOGZhOTNvbFNmcU5IdTVxK1FhTTByL0NBYzJBZkQveDREZUpIY3hackMw?=
 =?utf-8?B?ZGRMV0tZOWNibFpoUG55Z1Z0TVUzM1JBYm45aEw4WWNod1pSc1B3REpXRHpC?=
 =?utf-8?B?NTBrU1NzUjlkWjQ0Unc2RDFoTUNsY1M5bWswTFgxdDdTdENSMzZGelJ2MUEv?=
 =?utf-8?B?QjhNclI3c2tMaldrY3NNU3NwbmNlZ1A5QVMwYWhJSEJlY0lNbmtmTXg3bndy?=
 =?utf-8?B?cU92M0VwcFVrVklqV3ZGcHlwZFdSOWx1cDI4R1ZZWFlubW5BWkNTTTFxdUtr?=
 =?utf-8?B?S3ozSHBLb0JwNEJOZEMvMmxVRHc3SER4enpwSzdVTEFHRURoUGtvVFJHeEps?=
 =?utf-8?B?djZUajdJU3ZZOUdtN2p1STZzUHdHZENrY0dwTWlqYm1tcEpjVjlhTUpRYXNu?=
 =?utf-8?B?UUpXSVdSWHVtRHkraE5GY3VsdmhGMkZIZVF3SHJ2NTBBUG1IT2JmRWwzTVox?=
 =?utf-8?B?SC9uT2hJakd1c2JpeEI3dzZBQ0QvVFNSekxCSWNhMDA0WTZjaHdBb0w5STNv?=
 =?utf-8?B?QytqdVEwYjZ5NHdaUHZSRjgrZWQrdU4xV093YVVJZjRhOW1SakVqcUtsRFhD?=
 =?utf-8?B?QXdjVU1nRlc4NFNkRExaY2lRVTJvaW5wY0llWU9ZTGVoU1g5ZEV4YVN0Uk5V?=
 =?utf-8?B?Rk42UTg4TEtKM0hVeUNzUFpBYmZMUC9BdlVtMVVxazFOQzhsZnBGdHdXa3pm?=
 =?utf-8?B?cmpyd01XcXpIeTNhSGxROXZnMkFxQVozOGF1bFEzQTNFczFNWGhQQ1pxQjAw?=
 =?utf-8?B?dEFzSGdxWURQR0VHOVJ0eHhtVXh0dlF4L1AvY1k2RWN4YmpuRURXYndNNldl?=
 =?utf-8?B?RUliNXNCRUk0VHJLQ1BHcDZoYmFlSVdRMks2RGNXaFVpRTFLSnFkNjA1bmlz?=
 =?utf-8?B?Qk4xNkxYWDQvSHpDRk9ubE9zSWw4T3p5dG9yZEl5aDlwMVdMM3N5REk4TnQw?=
 =?utf-8?B?RU8wVnAvU3NSTUVCU3VxNjhOL0dCZ2JJTFc3REltK1hqbkdRanFDLy9OVXdh?=
 =?utf-8?B?VTl3Q2k2Rm0reEpiVzM3T0ZFSmYzMzJGK1ZvU0FrajFPU0QxVDZ3TWdzdXF4?=
 =?utf-8?B?UGw5M0ZIUllpOHFrbkhWTWRvVStSZCtqaWFONThEOTNxeVlaQUpEb2NRMDl4?=
 =?utf-8?B?bjJRYlNwMk02N3JPOUJYK1VlbElTVG5ZRGtvaWUyWVFDSVN1NmVYN0cyKzBY?=
 =?utf-8?B?TXFkUjRWazJnMmZod2pML2VhbFJFekc1WnNnUUNzZG4wNThSL0ZGUllnZUpT?=
 =?utf-8?B?dFVncXozUEZ1MERoZGxJVis1b0Z2aWZ5WkVzQ1h6YzRwbDR4QTZNVkMxRWl1?=
 =?utf-8?B?dXVWMXRBVXJLWS9XN1RnaE50NUM4QlVHQjFyQXl4YkRSZlZLVGtLdWhVa3Ax?=
 =?utf-8?B?R2N5QXlXVHIzYmg5UWpDRnEvMUlLbnJJb3hHN3M1cHduZWEvYUZJL2FxN3gv?=
 =?utf-8?B?dnBUVVZsV0hBNFJ2MVcrTXdBaEh3djNPYitucTRKdW5NdE9yV25MUW5pWjNM?=
 =?utf-8?B?aWNSSVVwVWo2TFp0V2JGQU84YkdwV2JobzFDLzdScVgvSGJRVmRsSWZtMjBZ?=
 =?utf-8?B?YkxZS1hCMUE2VXpxRGFNSmp0T0NwVE81UitIeSt2b1U3QnJXM0ZLcGEzS0JB?=
 =?utf-8?B?MUJOZVBFdm83ajNSWEtaMGZDWlhJS0dndDJhZEpkc0JLUHdPbXZHQTF1Z1Zk?=
 =?utf-8?B?NEk4L2VOMkpicHU3Sms2US81NmVJWmpXWDBYN0pTdXJCNHpWcVUvV3phMjNO?=
 =?utf-8?B?SVpQVkVMSGtKVHpBN2V3blViNDhZWm13WnpVSENFeVV2dGhqMnpMbWdwT3RY?=
 =?utf-8?B?VHVhaEEvRWVaNklTb2lGOHc3eTdUTGRQWWo2Z3p0bEFzZDRsNzJWNjN1Qkd2?=
 =?utf-8?Q?FqSFabkKn1uDfffonHVn99RT3Y7M2yyCTVw2jSOLkxbiG?=
X-MS-Exchange-AntiSpam-MessageData-1: kgXcSlhZPHI0LQ==
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 197eb604-0bb2-4712-50d7-08da158367c7
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Apr 2022 15:05:35.6267
 (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: 3QkQKY2nyWvG2OI03JCKRAKTsaSopBUWkUPA0F3wFEEW8nXoJ2+7g4Q3W5hwmr8B
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1340
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable 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 01.04.22 um 23:31 schrieb Pavel Skripkin:
> num_fences is user-controlled value and it can be equal to 0. Code
> should not pass 0 to kcalloc(), since it will cause kcalloc() to return
> ZERO_PTR. ZERO_PTR will pass `!fences` check and kernel will panic
> because of dereferencing ZERO_PTR in add_fence()
>
> Fix it by validating num_fences and bail out early if it is equal to 0

Well there are multiple issues with this patch. First of all as I wrote 
before it shouldn't be possible that num_fences is zero.

We could still just add this as a precaution, but then bailing out is 
the wrong thing to do here. Instead we should then make sure to allocate 
at least one slot for a fence in the array.

But I think the cleanest would just be to not add a fence into the array 
in the first place when num_fences is zero.

Regards,
Christian.

>
> Fixes: 519f490db07e ("dma-buf/sync-file: fix warning about fence containers")
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>
> Changes since v1:
> 	- Dropped already merged part
> 	- Removed syzkaller's tag
>
> ---
>   drivers/dma-buf/sync_file.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index b8dea4ec123b..024f22193e0c 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -212,7 +212,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
>   	dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence)
>   		++num_fences;
>   
> -	if (num_fences > INT_MAX)
> +	if (num_fences > INT_MAX || !num_fences)
>   		goto err_free_sync_file;
>   
>   	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);