All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Skripkin <paskripkin@gmail.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
	sumit.semwal@linaro.org, gustavo@padovan.org,
	christian.koenig@amd.com, daniel.vetter@ffwll.ch
Cc: linaro-mm-sig@lists.linaro.org,
	syzbot+5c943fe38e86d615cac2@syzkaller.appspotmail.com,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-media@vger.kernel.org
Subject: Re: [Linaro-mm-sig] [PATCH next] dma-buf/sync-file: do not allow zero size allocations
Date: Wed, 30 Mar 2022 21:24:48 +0300	[thread overview]
Message-ID: <ecf7ccdd-0d6f-9407-6778-ae7f0a6bf538@gmail.com> (raw)
In-Reply-To: <8af3d213-6cb7-a021-c198-e1bd37c47e7c@gmail.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Pavel Skripkin <paskripkin@gmail.com>
To: "Christian König" <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
Subject: Re: [Linaro-mm-sig] [PATCH next] dma-buf/sync-file: do not allow zero size allocations
Date: Wed, 30 Mar 2022 21:24:48 +0300	[thread overview]
Message-ID: <ecf7ccdd-0d6f-9407-6778-ae7f0a6bf538@gmail.com> (raw)
In-Reply-To: <8af3d213-6cb7-a021-c198-e1bd37c47e7c@gmail.com>

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

  reply	other threads:[~2022-03-30 18:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-29 22:14 [PATCH next] dma-buf/sync-file: do not allow zero size allocations Pavel Skripkin
2022-03-29 22:14 ` Pavel Skripkin
2022-03-30  7:09 ` [Linaro-mm-sig] " Christian König
2022-03-30  7:09   ` Christian König
2022-03-30 18:24   ` Pavel Skripkin [this message]
2022-03-30 18:24     ` Pavel Skripkin
2022-03-31  6:23     ` Christian König
2022-03-31  6:23       ` Christian König
2022-04-01 21:31       ` [PATCH next v2] dma-buf/sync-file: do not allow zero size allocation Pavel Skripkin
2022-04-01 21:31         ` Pavel Skripkin
2022-04-03 15:05         ` Christian König
2022-04-03 15:05           ` Christian König

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ecf7ccdd-0d6f-9407-6778-ae7f0a6bf538@gmail.com \
    --to=paskripkin@gmail.com \
    --cc=christian.koenig@amd.com \
    --cc=ckoenig.leichtzumerken@gmail.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gustavo@padovan.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=syzbot+5c943fe38e86d615cac2@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.