[bug report] dma-buf: heaps: Rework heap allocation hooks to return struct dma_buf instead of fd

[bug report] dma-buf: heaps: Rework heap allocation hooks to return struct dma_buf instead of fd

[Dan Carpenter]

I'm not sure exactly who to report this bug too.  Probably the mm
devs?

	drivers/dma-buf/heaps/system_heap.c:499 system_heap_allocate()
	warn: passing positive error code 's32min-(-1),1' to 'ERR_PTR'

drivers/dma-buf/heaps/system_heap.c
    459         if (cc_shared) {
    460                 for_each_sgtable_sg(table, sg, i) {
    461                         ret = system_heap_set_page_decrypted(sg_page(sg));
    462                         if (ret)
    463                                 goto free_pages;

It kind of looks like system_heap_set_page_decrypted() can return 1.

    464                 }
    465         }
    466 
    467         /* create the dmabuf */
    468         exp_info.exp_name = dma_heap_get_name(heap);
    469         exp_info.ops = &system_heap_buf_ops;
    470         exp_info.size = buffer->len;
    471         exp_info.flags = fd_flags;
    472         exp_info.priv = buffer;
    473         dmabuf = dma_buf_export(&exp_info);
    474         if (IS_ERR(dmabuf)) {
    475                 ret = PTR_ERR(dmabuf);
    476                 goto free_pages;
    477         }
    478         return dmabuf;
    479 
    480 free_pages:
    481         for_each_sgtable_sg(table, sg, i) {
    482                 struct page *p = sg_page(sg);
    483 
    484                 /*
    485                  * Intentionally leak pages that cannot be re-encrypted
    486                  * to prevent shared memory from being reused.
    487                  */
    488                 if (buffer->cc_shared &&
    489                     system_heap_set_page_encrypted(p))
    490                         continue;
    491                 __free_pages(p, compound_order(p));
    492         }
    493         sg_free_table(table);
    494 free_buffer:
    495         list_for_each_entry_safe(page, tmp_page, &pages, lru)
    496                 __free_pages(page, compound_order(page));
    497         kfree(buffer);
    498 
--> 499         return ERR_PTR(ret);
    500 }

The problem is that add_to_pagemap() returns PM_END_OF_BUFFER (1)
which is used by pagemap_read() and nowhere else.  The call tree
is:

system_heap_allocate()
system_heap_set_page_decrypted()
set_memory_decrypted()
realm_set_memory_decrypted()
__set_memory_enc_dec()
__change_memory_common()
update_range_prot()
walk_kernel_page_table_range_lockless()
walk_pgd_range()
pagemap_pte_hole()
add_to_pagemap()

This code seems sort of old and I guess no one has reported the bug
so maybe it's a false positive, but it feels like it's asking for
problems to return the PM_END_OF_BUFFER.  There aren't any comments
on any of those functions above explaining what return values are
expected.

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter