[PATCH] dma-buf: Fix possible UAF in dma_buf_export

cuigaosheng cuigaosheng1 at huawei.com
Thu Nov 24 12:05:33 UTC 2022


Some tips:
     Before we call the dma_buf_stats_setup(), we have to finish creating the file,
otherwise dma_buf_stats_setup() will return -EINVAL, maybe we need to think about
this when making a new patch.

Hope these tips are useful, thanks!

On 2022/11/24 13:56, Charan Teja Kalla wrote:
> Thanks T.J and Christian for the inputs.
>
> On 11/19/2022 7:00 PM, Christian König wrote:
>>>      Yes, exactly that's the idea.
>>>
>>>      The only alternatives I can see would be to either move allocating
>>>      the
>>>      file and so completing the dma_buf initialization last again or just
>>>      ignore errors from sysfs.
>>>
>>>      > If we still want to avoid calling dmabuf->ops->release(dmabuf) in
>>>      > dma_buf_release like the comment says I guess we could use
>>>      sysfs_entry
>>>      > and ERR_PTR to flag that, otherwise it looks like we'd need a bit
>>>      > somewhere.
>>>
>>>      No, this should be dropped as far as I can see. The sysfs cleanup
>>>      code
>>>      looks like it can handle not initialized kobj pointers.
>>>
>>>
>>> Yeah there is also the null check in dma_buf_stats_teardown() that
>>> would prevent it from running, but I understood the comment to be
>>> referring to the release() dma_buf_ops call into the exporter which
>>> comes right after the teardown call. That looks like it's preventing
>>> the fput task work calling back into the exporter after the exporter
>>> already got an error from dma_buf_export(). Otherwise the exporter
>>> sees a release() for a buffer that it doesn't know about / thinks
>>> shouldn't exist. So I could imagine an exporter trying to double free:
>>> once for the failed dma_buf_export() call, and again when the
>>> release() op is called later.
>>
>> Oh, very good point as well. Yeah, then creating the file should
>> probably come last.
>>
> @Gaosheng: Could you please make these changes or you let me to do?
>
>> Regards,
>> Christian.
> .


More information about the dri-devel mailing list