Re: [bug report] drm/xe: Enforce correct user fence signaling order using

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Nov 24, 2025 at 05:42:23PM +0300, Dan Carpenter wrote:
> On Tue, Nov 18, 2025 at 09:05:15AM -0800, Matthew Brost wrote:
> > On Tue, Nov 18, 2025 at 06:14:54PM +0300, Dan Carpenter wrote:
> > > Hello Matthew Brost,
> > > 
> > > Commit adda4e855ab6 ("drm/xe: Enforce correct user fence signaling
> > > order using") from Oct 31, 2025 (linux-next), leads to the following
> > > Smatch static checker warning:
> > > 
> > > 	drivers/gpu/drm/xe/xe_oa.c:1867 xe_oa_stream_open_ioctl_locked()
> > > 	error: double free of 'param->syncs' (line 1863)
> > > 
> > > drivers/gpu/drm/xe/xe_oa.c
> > >     1831 static int xe_oa_stream_open_ioctl_locked(struct xe_oa *oa,
> > >     1832                                           struct xe_oa_open_param *param)
> > >     1833 {
> > >     1834         struct xe_oa_stream *stream;
> > >     1835         struct drm_syncobj *ufence_syncobj;
> > >     1836         int stream_fd;
> > >     1837         int ret;
> > >     1838 
> > >     1839         /* We currently only allow exclusive access */
> > >     1840         if (param->oa_unit->exclusive_stream) {
> > >     1841                 drm_dbg(&oa->xe->drm, "OA unit already in use\n");
> > >     1842                 ret = -EBUSY;
> > >     1843                 goto exit;
> > >     1844         }
> > >     1845 
> > >     1846         ret = drm_syncobj_create(&ufence_syncobj, DRM_SYNCOBJ_CREATE_SIGNALED,
> > >     1847                                  NULL);
> > >     1848         if (ret)
> > >     1849                 goto exit;
> > >     1850 
> > >     1851         stream = kzalloc(sizeof(*stream), GFP_KERNEL);
> > >     1852         if (!stream) {
> > >     1853                 ret = -ENOMEM;
> > >     1854                 goto err_syncobj;
> > >     1855         }
> > >     1856         stream->ufence_syncobj = ufence_syncobj;
> > >     1857         stream->oa = oa;
> > >     1858 
> > >     1859         ret = xe_oa_parse_syncs(oa, stream, param);
> > >     1860         if (ret)
> > >     1861                 goto err_free;
> > >     1862 
> > >     1863         ret = xe_oa_stream_init(stream, param);
> > >     1864         if (ret) {
> > >     1865                 while (param->num_syncs--)
> > >     1866                         xe_sync_entry_cleanup(&param->syncs[param->num_syncs]);
> > > --> 1867                 kfree(param->syncs);
> > >                          ^^^^^^^^^^^^^^^^^^^^
> > > 
> > > xe_oa_stream_init() already frees param->syncs when it calls
> > > xe_oa_emit_oa_config().
> > > 
> > 
> > Admittedly this coded poorly but I think this a false positive.
> > param->syncs is only freed when xe_oa_stream_init returns success.
> > 
> > That said this should probably be refactored a bit for clarity.
> > 
> 
> Sorry for this.  It's a bug in Smatch, yes.  I'm testing a fix for this.
> 

All good. I don’t think there is anyone in Linux who doesn’t appreciate Smatch.

Matt

> Thanks for looking into it.
> 
> regards,
> dan carpenter
> 
> 
> > Matt
> > 
> > >     1868                 goto err_free;
> > >     1869         }
> > >     1870 
> > >     1871         if (!param->disabled) {
> > >     1872                 ret = xe_oa_enable_locked(stream);
> > >     1873                 if (ret)
> > >     1874                         goto err_destroy;
> > >     1875         }
> > >     1876 
> > >     1877         stream_fd = anon_inode_getfd("[xe_oa]", &xe_oa_fops, stream, 0);
> > >     1878         if (stream_fd < 0) {
> > >     1879                 ret = stream_fd;
> > >     1880                 goto err_disable;
> > >     1881         }
> > >     1882 
> > >     1883         /* Hold a reference on the drm device till stream_fd is released */
> > >     1884         drm_dev_get(&stream->oa->xe->drm);
> > >     1885 
> > >     1886         return stream_fd;
> > >     1887 err_disable:
> > >     1888         if (!param->disabled)
> > >     1889                 xe_oa_disable_locked(stream);
> > >     1890 err_destroy:
> > >     1891         xe_oa_stream_destroy(stream);
> > >     1892 err_free:
> > >     1893         kfree(stream);
> > >     1894 err_syncobj:
> > >     1895         drm_syncobj_put(ufence_syncobj);
> > >     1896 exit:
> > >     1897         return ret;
> > >     1898 }
> > > 
> > > regards,
> > > dan carpenter