From: "Christian König" <ckoenig.leichtzumerken@gmail.com> 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 Subject: Re: [Linaro-mm-sig] [PATCH next] dma-buf/sync-file: do not allow zero size allocations Date: Thu, 31 Mar 2022 08:23:32 +0200 [thread overview] Message-ID: <2d1f9ba9-ea2a-e41c-eae6-0ba348cdf202@gmail.com> (raw) In-Reply-To: <ecf7ccdd-0d6f-9407-6778-ae7f0a6bf538@gmail.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: "Christian König" <ckoenig.leichtzumerken@gmail.com> To: Pavel Skripkin <paskripkin@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: Thu, 31 Mar 2022 08:23:32 +0200 [thread overview] Message-ID: <2d1f9ba9-ea2a-e41c-eae6-0ba348cdf202@gmail.com> (raw) In-Reply-To: <ecf7ccdd-0d6f-9407-6778-ae7f0a6bf538@gmail.com> 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
next prev parent reply other threads:[~2022-03-31 6:23 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 2022-03-30 18:24 ` Pavel Skripkin 2022-03-31 6:23 ` Christian König [this message] 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=2d1f9ba9-ea2a-e41c-eae6-0ba348cdf202@gmail.com \ --to=ckoenig.leichtzumerken@gmail.com \ --cc=christian.koenig@amd.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=paskripkin@gmail.com \ --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: linkBe 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.