[bug report] dma-buf: Exercise dma-fence-chain under selftests

[bug report] dma-buf: Exercise dma-fence-chain under selftests

[Dan Carpenter]

Hello Chris Wilson,

Commit dc2f7e67a28a ("dma-buf: Exercise dma-fence-chain under
selftests") from Apr 9, 2020 (linux-next), leads to the following
Smatch static checker warning:

	drivers/dma-buf/st-dma-fence-chain.c:216 find_seqno()
	warn: passing freed memory 'fence' (line 203)

drivers/dma-buf/st-dma-fence-chain.c
    181 static int find_seqno(void *arg)
    182 {
    183         struct fence_chains fc;
    184         struct dma_fence *fence;
    185         int err;
    186         int i;
    187 
    188         err = fence_chains_init(&fc, 64, seqno_inc);
    189         if (err)
    190                 return err;
    191 
    192         fence = dma_fence_get(fc.tail);
    193         err = dma_fence_chain_find_seqno(&fence, 0);
    194         dma_fence_put(fence);
    195         if (err) {
    196                 pr_err("Reported %d for find_seqno(0)!\n", err);
    197                 goto err;
    198         }
    199 
    200         for (i = 0; i < fc.chain_length; i++) {
    201                 fence = dma_fence_get(fc.tail);
    202                 err = dma_fence_chain_find_seqno(&fence, i + 1);
    203                 dma_fence_put(fence);
                        ^^^^^^^^^^^^^^^^^^^^^
I don't understand the ref counting.  Shouldn't we have to assume that
after this dma_fence_put() a different thread could release fence?

    204                 if (err) {
    205                         pr_err("Reported %d for find_seqno(%d:%d)!\n",
    206                                err, fc.chain_length + 1, i + 1);
    207                         goto err;
    208                 }
    209                 if (fence != fc.chains[i]) {
                            ^^^^^
Every later reference to fence would be a potential use after free.

    210                         pr_err("Incorrect fence reported by find_seqno(%d:%d)\n",
    211                                fc.chain_length + 1, i + 1);
    212                         err = -EINVAL;
    213                         goto err;
    214                 }
    215 
--> 216                 dma_fence_get(fence);

Calling dma_fence_get() wouldn't unfree it.

    217                 err = dma_fence_chain_find_seqno(&fence, i + 1);
    218                 dma_fence_put(fence);
    219                 if (err) {
    220                         pr_err("Error reported for finding self\n");
    221                         goto err;
    222                 }
    223                 if (fence != fc.chains[i]) {
    224                         pr_err("Incorrect fence reported by find self\n");
    225                         err = -EINVAL;
    226                         goto err;
    227                 }
    228 
    229                 dma_fence_get(fence);
    230                 err = dma_fence_chain_find_seqno(&fence, i + 2);
    231                 dma_fence_put(fence);
    232                 if (!err) {
    233                         pr_err("Error not reported for future fence: find_seqno(%d:%d)!\n",
    234                                i + 1, i + 2);
    235                         err = -EINVAL;
    236                         goto err;
    237                 }
    238 
    239                 dma_fence_get(fence);
    240                 err = dma_fence_chain_find_seqno(&fence, i);
    241                 dma_fence_put(fence);
    242                 if (err) {
    243                         pr_err("Error reported for previous fence!\n");
    244                         goto err;
    245                 }
    246                 if (i > 0 && fence != fc.chains[i - 1]) {
    247                         pr_err("Incorrect fence reported by find_seqno(%d:%d)\n",
    248                                i + 1, i);
    249                         err = -EINVAL;
    250                         goto err;
    251                 }
    252         }
    253 
    254 err:
    255         fence_chains_fini(&fc);
    256         return err;
    257 }

regards,
dan carpenter