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: <ecf7ccdd-0d6f-9407-6778-ae7f0a6bf538@gmail.com>
Date:   Wed, 30 Mar 2022 21:24:48 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [Linaro-mm-sig] [PATCH next] dma-buf/sync-file: do not allow zero
 size allocations
Content-Language: en-US
To:     =?UTF-8?Q?Christian_K=c3=b6nig?= <ckoenig.leichtzumerken@gmail.com>,
        sumit.semwal@linaro.org, gustavo@padovan.org,
        christian.koenig@amd.com, 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,
        syzbot+5c943fe38e86d615cac2@syzkaller.appspotmail.com
References: <20220329221425.22691-1-paskripkin@gmail.com>
 <8af3d213-6cb7-a021-c198-e1bd37c47e7c@gmail.com>
From:   Pavel Skripkin <paskripkin@gmail.com>
In-Reply-To: <8af3d213-6cb7-a021-c198-e1bd37c47e7c@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Christian,

On 3/30/22 10:09, Christian König wrote:
> That problem is already fixed with patch 21d139d73f77 dma-buf/sync-file:
> fix logic error in new fence merge code.
> 
> Am 30.03.22 um 00:14 schrieb Pavel Skripkin:
>> syzbot reported GPF in dma_fence_array_first(), which is caused by
>> dereferencing ZERO_PTR in dma-buf internals.
>>
>> ZERO_PTR was generated in sync_file_merge(). This functuion tries to
>> reduce allocation size, but does not check if it reducing to 0.
> 
> This is actually perfectly ok. The code above should have prevented the
> size to become 0.
> 
> Regards,
> Christian.
> 

Thanks for your reply! I see that 21d139d73f77 fixes GPF in 
dma_fence_array_first(), but what about this part:

>>   
>> -	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);
>> @@ -264,7 +264,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
>>   	if (index == 0)

If num_fences is equal to zero then fences dereference will cause an 
oops. Or this one is also fixed in your tree?


Thanks!


With regards,
Pavel Skripkin