[bug report] misc: fastrpc: Rework fastrpc_req_munmap - Linaro-mm-sig

29 Nov 2022


      Hello Abel Vesa,
The patch 72fa6f7820c4: "misc: fastrpc: Rework fastrpc_req_munmap"
from Nov 25, 2022, leads to the following Smatch static checker
warning:
drivers/misc/fastrpc.c:1927 fastrpc_req_mmap()
    error: double free of 'buf'
drivers/misc/fastrpc.c
    1831 static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
    1832 {
    1833         struct fastrpc_invoke_args args[3] = { [0 ... 2] = { 0 } };
    1834         struct fastrpc_buf *buf = NULL;
    1835         struct fastrpc_mmap_req_msg req_msg;
    1836         struct fastrpc_mmap_rsp_msg rsp_msg;
    1837         struct fastrpc_phy_page pages;
    1838         struct fastrpc_req_mmap req;
    1839         struct device *dev = fl->sctx->dev;
    1840         int err;
    1841         u32 sc;
    1842 
    1843         if (copy_from_user(&req, argp, sizeof(req)))
    1844                 return -EFAULT;
    1845 
    1846         if (req.flags != ADSP_MMAP_ADD_PAGES && req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR) {
    1847                 dev_err(dev, "flag not supported 0x%x\n", req.flags);
    1848 
    1849                 return -EINVAL;
    1850         }
    1851 
    1852         if (req.vaddrin) {
    1853                 dev_err(dev, "adding user allocated pages is not supported\n");
    1854                 return -EINVAL;
    1855         }
    1856 
    1857         err = fastrpc_buf_alloc(fl, fl->sctx->dev, req.size, &buf);
    1858         if (err) {
    1859                 dev_err(dev, "failed to allocate buffer\n");
    1860                 return err;
    1861         }
    1862 
    1863         req_msg.pgid = fl->tgid;
    1864         req_msg.flags = req.flags;
    1865         req_msg.vaddr = req.vaddrin;
    1866         req_msg.num = sizeof(pages);
    1867 
    1868         args[0].ptr = (u64) (uintptr_t) &req_msg;
    1869         args[0].length = sizeof(req_msg);
    1870 
    1871         pages.addr = buf->phys;
    1872         pages.size = buf->size;
    1873 
    1874         args[1].ptr = (u64) (uintptr_t) &pages;
    1875         args[1].length = sizeof(pages);
    1876 
    1877         args[2].ptr = (u64) (uintptr_t) &rsp_msg;
    1878         args[2].length = sizeof(rsp_msg);
    1879 
    1880         sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1);
    1881         err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
    1882                                       &args[0]);
    1883         if (err) {
    1884                 dev_err(dev, "mmap error (len 0x%08llx)\n", buf->size);
    1885                 goto err_invoke;
    1886         }
    1887 
    1888         /* update the buffer to be able to deallocate the memory on the DSP */
    1889         buf->raddr = (uintptr_t) rsp_msg.vaddr;
    1890 
    1891         /* let the client know the address to use */
    1892         req.vaddrout = rsp_msg.vaddr;
    1893 
    1894         /* Add memory to static PD pool, protection thru hypervisor */
    1895         if (req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
    1896                 struct qcom_scm_vmperm perm;
    1897 
    1898                 perm.vmid = QCOM_SCM_VMID_HLOS;
    1899                 perm.perm = QCOM_SCM_PERM_RWX;
    1900                 err = qcom_scm_assign_mem(buf->phys, buf->size,
    1901                         &(fl->cctx->vmperms[0].vmid), &perm, 1);
    1902                 if (err) {
    1903                         dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
    1904                                         buf->phys, buf->size, err);
    1905                         goto err_assign;
    1906                 }
    1907         }
    1908 
    1909         spin_lock(&fl->lock);
    1910         list_add_tail(&buf->node, &fl->mmaps);
    1911         spin_unlock(&fl->lock);
    1912 
    1913         if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
    1914                 err = -EFAULT;
    1915                 goto err_assign;
    1916         }
    1917 
    1918         dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
    1919                 buf->raddr, buf->size);
    1920 
    1921         return 0;
    1922 
    1923 err_assign:
    1924         fastrpc_req_munmap_impl(fl, buf);
                                             ^^^
"buf" freed here.
1925 err_invoke:
    1926         fastrpc_buf_free(buf);
                                  ^^^
freed again here.
--> 1927 
    1928         return err;
    1929 }
regards,
dan carpenter