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: <2d1f9ba9-ea2a-e41c-eae6-0ba348cdf202@gmail.com>
Date:   Thu, 31 Mar 2022 08:23:32 +0200
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:     Pavel Skripkin <paskripkin@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>
 <ecf7ccdd-0d6f-9407-6778-ae7f0a6bf538@gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <ckoenig.leichtzumerken@gmail.com>
In-Reply-To: <ecf7ccdd-0d6f-9407-6778-ae7f0a6bf538@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


Am 30.03.22 um 20:24 schrieb Pavel Skripkin:
> 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?

Well it is illegal for sync_file->fence to be NULL or we would run into 
NULL pointer dereference much more often, so num_fences can't be zero 
here either.

Regards,
Christian.

>
>
> Thanks!
>
>
>
>
> With regards,
> Pavel Skripkin