* [PATCH] drm: zynqmp_dp: Fix uninitialized variable in debugfs()
@ 2026-05-25 7:16 Dan Carpenter
2026-05-28 0:31 ` Sean Anderson
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2026-05-25 7:16 UTC (permalink / raw)
To: Sean Anderson
Cc: Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Michal Simek, dri-devel, linux-arm-kernel, linux-kernel,
kernel-janitors
If the *ppos is non-zero then simple_write_to_buffer() will not
initialize the start of the buf[] buffer. It doesn't really make sense
to allow non-zero values for *ppos, so check for that at the start and
return -EINVAL.
Fixes: 28edaacb821c ("drm: zynqmp_dp: Add debugfs interface for compliance testing")
Signed-off-by: Dan Carpenter <error27@gmail•com>
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 7fb11b0a44f0..847179476041 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1888,6 +1888,9 @@ static ssize_t zynqmp_dp_pattern_write(struct file *file,
ssize_t ret;
int pattern;
+ if (*ppos != 0)
+ return -EINVAL;
+
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf,
count);
if (ret < 0)
@@ -2028,6 +2031,9 @@ static ssize_t zynqmp_dp_custom_write(struct file *file,
ssize_t ret;
char buf[sizeof(dp->test.custom)];
+ if (*ppos != 0)
+ return -EINVAL;
+
ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
if (ret < 0)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: zynqmp_dp: Fix uninitialized variable in debugfs()
2026-05-25 7:16 [PATCH] drm: zynqmp_dp: Fix uninitialized variable in debugfs() Dan Carpenter
@ 2026-05-28 0:31 ` Sean Anderson
2026-05-28 6:18 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Sean Anderson @ 2026-05-28 0:31 UTC (permalink / raw)
To: Dan Carpenter
Cc: Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Michal Simek, dri-devel, linux-arm-kernel, linux-kernel,
kernel-janitors
On 5/25/26 03:16, Dan Carpenter wrote:
> If the *ppos is non-zero then simple_write_to_buffer() will not
> initialize the start of the buf[] buffer. It doesn't really make sense
> to allow non-zero values for *ppos, so check for that at the start and
> return -EINVAL.
non-zero ppos seems to be handled properly by simple_write_to_buffer.
--Sean
> Fixes: 28edaacb821c ("drm: zynqmp_dp: Add debugfs interface for compliance testing")
> Signed-off-by: Dan Carpenter <error27@gmail•com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 7fb11b0a44f0..847179476041 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1888,6 +1888,9 @@ static ssize_t zynqmp_dp_pattern_write(struct file *file,
> ssize_t ret;
> int pattern;
>
> + if (*ppos != 0)
> + return -EINVAL;
> +
> ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf,
> count);
> if (ret < 0)
> @@ -2028,6 +2031,9 @@ static ssize_t zynqmp_dp_custom_write(struct file *file,
> ssize_t ret;
> char buf[sizeof(dp->test.custom)];
>
> + if (*ppos != 0)
> + return -EINVAL;
> +
> ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
> if (ret < 0)
> return ret;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: zynqmp_dp: Fix uninitialized variable in debugfs()
2026-05-28 0:31 ` Sean Anderson
@ 2026-05-28 6:18 ` Dan Carpenter
2026-05-28 8:00 ` Sean Anderson
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2026-05-28 6:18 UTC (permalink / raw)
To: Sean Anderson
Cc: Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Michal Simek, dri-devel, linux-arm-kernel, linux-kernel,
kernel-janitors
On Wed, May 27, 2026 at 08:31:59PM -0400, Sean Anderson wrote:
> On 5/25/26 03:16, Dan Carpenter wrote:
> > If the *ppos is non-zero then simple_write_to_buffer() will not
> > initialize the start of the buf[] buffer. It doesn't really make sense
> > to allow non-zero values for *ppos, so check for that at the start and
> > return -EINVAL.
>
> non-zero ppos seems to be handled properly by simple_write_to_buffer.
>
It's not an overflow bug, it's an uninitialized variable bug.
The simple_write_to_buffer() is designed to handle partial writes so it
leaves the first "written" (scare quotes) part of the string as is. But
in this case, we can't handle a partial write and the first part of
buf[] is left uninitialized.
https://staticthinking.wordpress.com/2026/05/23/simple_write_to_buffer-is-complicated/
Also this appears to be dead code since fops_zynqmp_dp_pattern is never
used.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: zynqmp_dp: Fix uninitialized variable in debugfs()
2026-05-28 6:18 ` Dan Carpenter
@ 2026-05-28 8:00 ` Sean Anderson
2026-05-28 8:59 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Sean Anderson @ 2026-05-28 8:00 UTC (permalink / raw)
To: Dan Carpenter
Cc: Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Michal Simek, dri-devel, linux-arm-kernel, linux-kernel,
kernel-janitors
On 5/28/26 02:18, Dan Carpenter wrote:
> On Wed, May 27, 2026 at 08:31:59PM -0400, Sean Anderson wrote:
>> On 5/25/26 03:16, Dan Carpenter wrote:
>>> If the *ppos is non-zero then simple_write_to_buffer() will not
>>> initialize the start of the buf[] buffer. It doesn't really make sense
>>> to allow non-zero values for *ppos, so check for that at the start and
>>> return -EINVAL.
>>
>> non-zero ppos seems to be handled properly by simple_write_to_buffer.
>>
>
> It's not an overflow bug, it's an uninitialized variable bug.
>
> The simple_write_to_buffer() is designed to handle partial writes so it
> leaves the first "written" (scare quotes) part of the string as is. But
> in this case, we can't handle a partial write and the first part of
> buf[] is left uninitialized.
>
> https://staticthinking.wordpress.com/2026/05/23/simple_write_to_buffer-is-complicated/
OK, that's a bit strange. Can you add this to the doc comment? I certainly
missed it when looking around for appropriate functions.
And I like the explicit copy_from_user variant better, especially since
we are setting the nul-terminator anyway.
> Also this appears to be dead code since fops_zynqmp_dp_pattern is never
> used.
It's of course used in zynqmp_dp_bridge_debugfs_init.
--Sean
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: zynqmp_dp: Fix uninitialized variable in debugfs()
2026-05-28 8:00 ` Sean Anderson
@ 2026-05-28 8:59 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2026-05-28 8:59 UTC (permalink / raw)
To: Sean Anderson
Cc: Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Michal Simek, dri-devel, linux-arm-kernel, linux-kernel,
kernel-janitors
On Thu, May 28, 2026 at 04:00:53AM -0400, Sean Anderson wrote:
> On 5/28/26 02:18, Dan Carpenter wrote:
> > On Wed, May 27, 2026 at 08:31:59PM -0400, Sean Anderson wrote:
> > > On 5/25/26 03:16, Dan Carpenter wrote:
> > > > If the *ppos is non-zero then simple_write_to_buffer() will not
> > > > initialize the start of the buf[] buffer. It doesn't really make sense
> > > > to allow non-zero values for *ppos, so check for that at the start and
> > > > return -EINVAL.
> > >
> > > non-zero ppos seems to be handled properly by simple_write_to_buffer.
> > >
> >
> > It's not an overflow bug, it's an uninitialized variable bug.
> >
> > The simple_write_to_buffer() is designed to handle partial writes so it
> > leaves the first "written" (scare quotes) part of the string as is. But
> > in this case, we can't handle a partial write and the first part of
> > buf[] is left uninitialized.
> >
> > https://staticthinking.wordpress.com/2026/05/23/simple_write_to_buffer-is-complicated/
>
> OK, that's a bit strange. Can you add this to the doc comment? I certainly
> missed it when looking around for appropriate functions.
>
> And I like the explicit copy_from_user variant better, especially since
> we are setting the nul-terminator anyway.
>
> > Also this appears to be dead code since fops_zynqmp_dp_pattern is never
> > used.
>
> It's of course used in zynqmp_dp_bridge_debugfs_init.
Ah, yes. I just did a git grep and the CREATE_FILE() macro confused me.
But obviously this wouldn't compile if we had an unused local variable.
Duh...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-28 9:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-25 7:16 [PATCH] drm: zynqmp_dp: Fix uninitialized variable in debugfs() Dan Carpenter
2026-05-28 0:31 ` Sean Anderson
2026-05-28 6:18 ` Dan Carpenter
2026-05-28 8:00 ` Sean Anderson
2026-05-28 8:59 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox