* [bug report] drm/xe: Enforce correct user fence signaling order using
@ 2025-11-18 15:14 Dan Carpenter
2025-11-18 17:05 ` Matthew Brost
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2025-11-18 15:14 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe
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(¶m->syncs[param->num_syncs]);
--> 1867 kfree(param->syncs);
^^^^^^^^^^^^^^^^^^^^
xe_oa_stream_init() already frees param->syncs when it calls
xe_oa_emit_oa_config().
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] drm/xe: Enforce correct user fence signaling order using
2025-11-18 15:14 [bug report] drm/xe: Enforce correct user fence signaling order using Dan Carpenter
@ 2025-11-18 17:05 ` Matthew Brost
2025-11-24 14:42 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Matthew Brost @ 2025-11-18 17:05 UTC (permalink / raw)
To: Dan Carpenter; +Cc: intel-xe
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(¶m->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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] drm/xe: Enforce correct user fence signaling order using
2025-11-18 17:05 ` Matthew Brost
@ 2025-11-24 14:42 ` Dan Carpenter
2025-11-26 2:32 ` Matthew Brost
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2025-11-24 14:42 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe
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(¶m->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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] drm/xe: Enforce correct user fence signaling order using
2025-11-24 14:42 ` Dan Carpenter
@ 2025-11-26 2:32 ` Matthew Brost
0 siblings, 0 replies; 4+ messages in thread
From: Matthew Brost @ 2025-11-26 2:32 UTC (permalink / raw)
To: Dan Carpenter; +Cc: intel-xe
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(¶m->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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-26 2:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 15:14 [bug report] drm/xe: Enforce correct user fence signaling order using Dan Carpenter
2025-11-18 17:05 ` Matthew Brost
2025-11-24 14:42 ` Dan Carpenter
2025-11-26 2:32 ` Matthew Brost
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox