* [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
@ 2026-02-16 21:17 Abd-Alrhman Masalkhi
2026-02-18 13:24 ` Thomas Weißschuh
0 siblings, 1 reply; 9+ messages in thread
From: Abd-Alrhman Masalkhi @ 2026-02-16 21:17 UTC (permalink / raw)
To: dwlsalmeida, mchehab, linmag7, thomas.weissschuh
Cc: linux-media, linux-kernel, Abd-Alrhman Masalkhi,
syzbot+96f901260a0b2d29cd1a
vidtv_ts_null_write_into() takes null_packet_write_args by value,
causing MSAN to report an uninit-value warning on buf_sz inside
the function.
Fix by passing the struct by pointer instead, avoiding the stack copy
entirely.
Reported-by: syzbot+96f901260a0b2d29cd1a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=96f901260a0b2d29cd1a
Fixes: cd7a5651db26 ("alpha: add missing address argument in call to page_table_check_pte_clear()")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/media/test-drivers/vidtv/vidtv_mux.c | 2 +-
drivers/media/test-drivers/vidtv/vidtv_ts.c | 18 +++++++++---------
drivers/media/test-drivers/vidtv/vidtv_ts.h | 2 +-
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/media/test-drivers/vidtv/vidtv_mux.c b/drivers/media/test-drivers/vidtv/vidtv_mux.c
index f99878eff7ac..67a580396112 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_mux.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_mux.c
@@ -363,7 +363,7 @@ static u32 vidtv_mux_pad_with_nulls(struct vidtv_mux *m, u32 npkts)
args.continuity_counter = &ctx->cc;
for (i = 0; i < npkts; ++i) {
- m->mux_buf_offset += vidtv_ts_null_write_into(args);
+ m->mux_buf_offset += vidtv_ts_null_write_into(&args);
args.dest_offset = m->mux_buf_offset;
}
diff --git a/drivers/media/test-drivers/vidtv/vidtv_ts.c b/drivers/media/test-drivers/vidtv/vidtv_ts.c
index ca4bb9c40b78..7e6e92503fb8 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_ts.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_ts.c
@@ -48,7 +48,7 @@ void vidtv_ts_inc_cc(u8 *continuity_counter)
*continuity_counter = 0;
}
-u32 vidtv_ts_null_write_into(struct null_packet_write_args args)
+u32 vidtv_ts_null_write_into(struct null_packet_write_args *args)
{
u32 nbytes = 0;
struct vidtv_mpeg_ts ts_header = {};
@@ -56,21 +56,21 @@ u32 vidtv_ts_null_write_into(struct null_packet_write_args args)
ts_header.sync_byte = TS_SYNC_BYTE;
ts_header.bitfield = cpu_to_be16(TS_NULL_PACKET_PID);
ts_header.payload = 1;
- ts_header.continuity_counter = *args.continuity_counter;
+ ts_header.continuity_counter = *args->continuity_counter;
/* copy TS header */
- nbytes += vidtv_memcpy(args.dest_buf,
- args.dest_offset + nbytes,
- args.buf_sz,
+ nbytes += vidtv_memcpy(args->dest_buf,
+ args->dest_offset + nbytes,
+ args->buf_sz,
&ts_header,
sizeof(ts_header));
- vidtv_ts_inc_cc(args.continuity_counter);
+ vidtv_ts_inc_cc(args->continuity_counter);
/* fill the rest with empty data */
- nbytes += vidtv_memset(args.dest_buf,
- args.dest_offset + nbytes,
- args.buf_sz,
+ nbytes += vidtv_memset(args->dest_buf,
+ args->dest_offset + nbytes,
+ args->buf_sz,
TS_FILL_BYTE,
TS_PACKET_LEN - nbytes);
diff --git a/drivers/media/test-drivers/vidtv/vidtv_ts.h b/drivers/media/test-drivers/vidtv/vidtv_ts.h
index 09b4ffd02829..28da15dcc697 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_ts.h
+++ b/drivers/media/test-drivers/vidtv/vidtv_ts.h
@@ -90,7 +90,7 @@ void vidtv_ts_inc_cc(u8 *continuity_counter);
*
* Return: The number of bytes written into the buffer.
*/
-u32 vidtv_ts_null_write_into(struct null_packet_write_args args);
+u32 vidtv_ts_null_write_into(struct null_packet_write_args *args);
/**
* vidtv_ts_pcr_write_into - Write a PCR packet into a buffer.
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
2026-02-16 21:17 [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value Abd-Alrhman Masalkhi
@ 2026-02-18 13:24 ` Thomas Weißschuh
2026-02-19 10:17 ` Abd-Alrhman Masalkhi
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Weißschuh @ 2026-02-18 13:24 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi
Cc: dwlsalmeida, mchehab, linmag7, linux-media, linux-kernel,
syzbot+96f901260a0b2d29cd1a
Hi Abd-Alrhman!
On Mon, Feb 16, 2026 at 10:17:03PM +0100, Abd-Alrhman Masalkhi wrote:
> vidtv_ts_null_write_into() takes null_packet_write_args by value,
> causing MSAN to report an uninit-value warning on buf_sz inside
> the function.
>
> Fix by passing the struct by pointer instead, avoiding the stack copy
> entirely.
>
> Reported-by: syzbot+96f901260a0b2d29cd1a@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=96f901260a0b2d29cd1a
> Fixes: cd7a5651db26 ("alpha: add missing address argument in call to page_table_check_pte_clear()")
The Fixes tag should point to the commit which originally introduced
the issue that is being fixed. My commit for alpha-specific code is
unlikely to be the root cause because a) syscaller is not running alpha
systems and b) the first occurrence of the syscaller was before my patch
was picked up by Linus. So please try to find a better Fixes tag.
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> drivers/media/test-drivers/vidtv/vidtv_mux.c | 2 +-
> drivers/media/test-drivers/vidtv/vidtv_ts.c | 18 +++++++++---------
> drivers/media/test-drivers/vidtv/vidtv_ts.h | 2 +-
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/test-drivers/vidtv/vidtv_mux.c b/drivers/media/test-drivers/vidtv/vidtv_mux.c
> index f99878eff7ac..67a580396112 100644
> --- a/drivers/media/test-drivers/vidtv/vidtv_mux.c
> +++ b/drivers/media/test-drivers/vidtv/vidtv_mux.c
> @@ -363,7 +363,7 @@ static u32 vidtv_mux_pad_with_nulls(struct vidtv_mux *m, u32 npkts)
> args.continuity_counter = &ctx->cc;
>
> for (i = 0; i < npkts; ++i) {
> - m->mux_buf_offset += vidtv_ts_null_write_into(args);
> + m->mux_buf_offset += vidtv_ts_null_write_into(&args);
Please explain in the commit message why this is change is safe to do.
Modifying shared data might break the logic. (I have not verified this)
> args.dest_offset = m->mux_buf_offset;
> }
(...)
Did have syscaller test this patch before submission?
See the following documentation from the report:
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
2026-02-18 13:24 ` Thomas Weißschuh
@ 2026-02-19 10:17 ` Abd-Alrhman Masalkhi
0 siblings, 0 replies; 9+ messages in thread
From: Abd-Alrhman Masalkhi @ 2026-02-19 10:17 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: dwlsalmeida, mchehab, linmag7, linux-media, linux-kernel,
syzbot+96f901260a0b2d29cd1a
On Wed, Feb 18, 2026 at 14:24 +0100, Thomas Weißschuh wrote:
Hi Thomas,
Thank you for the feedback.
> Hi Abd-Alrhman!
>
> On Mon, Feb 16, 2026 at 10:17:03PM +0100, Abd-Alrhman Masalkhi wrote:
>> vidtv_ts_null_write_into() takes null_packet_write_args by value,
>> causing MSAN to report an uninit-value warning on buf_sz inside
>> the function.
>>
>> Fix by passing the struct by pointer instead, avoiding the stack copy
>> entirely.
>>
>> Reported-by: syzbot+96f901260a0b2d29cd1a@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=96f901260a0b2d29cd1a
>> Fixes: cd7a5651db26 ("alpha: add missing address argument in call to page_table_check_pte_clear()")
>
> The Fixes tag should point to the commit which originally introduced
> the issue that is being fixed. My commit for alpha-specific code is
> unlikely to be the root cause because a) syscaller is not running alpha
> systems and b) the first occurrence of the syscaller was before my patch
> was picked up by Linus. So please try to find a better Fixes tag.
>
I will fix the Fixes tag to point to the correct vidtv commit.
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> drivers/media/test-drivers/vidtv/vidtv_mux.c | 2 +-
>> drivers/media/test-drivers/vidtv/vidtv_ts.c | 18 +++++++++---------
>> drivers/media/test-drivers/vidtv/vidtv_ts.h | 2 +-
>> 3 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/test-drivers/vidtv/vidtv_mux.c b/drivers/media/test-drivers/vidtv/vidtv_mux.c
>> index f99878eff7ac..67a580396112 100644
>> --- a/drivers/media/test-drivers/vidtv/vidtv_mux.c
>> +++ b/drivers/media/test-drivers/vidtv/vidtv_mux.c
>> @@ -363,7 +363,7 @@ static u32 vidtv_mux_pad_with_nulls(struct vidtv_mux *m, u32 npkts)
>> args.continuity_counter = &ctx->cc;
>>
>> for (i = 0; i < npkts; ++i) {
>> - m->mux_buf_offset += vidtv_ts_null_write_into(args);
>> + m->mux_buf_offset += vidtv_ts_null_write_into(&args);
>
> Please explain in the commit message why this is change is safe to do.
> Modifying shared data might break the logic. (I have not verified this)
>
vidtv_ts_null_write_into() only reads from the struct and does not
modify it. should I add a const qualifier to the parameter?
>> args.dest_offset = m->mux_buf_offset;
>> }
>
> (...)
>
> Did have syscaller test this patch before submission?
> See the following documentation from the report:
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
I will test with syzbot before sending v2.
>
> Thomas
--
Best Regards,
Abd-Alrhman
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
@ 2026-02-20 13:39 Ding Yihan
2026-02-20 13:56 ` Thomas Weißschuh
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Ding Yihan @ 2026-02-20 13:39 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, Thomas Weißschuh
Cc: dwlsalmeida, mchehab, linmag7, linux-media, linux-kernel,
syzbot+96f901260a0b2d29cd1a
Hi Thomas and Abd-Alrhman,
While looking into this exact same syzbot report, I noticed that
`vidtv_ts_pcr_write_into()` in the same file also suffers from the
exact same pass-by-value anti-pattern (passing `struct pcr_write_args` by value).
Since `pcr_write_args` also contains implicit padding, it remains a potential trigger
for identical KMSAN uninit-value warnings during fuzzing in the future.
Also, regarding Thomas's concern about modifying shared data: passing the struct
as a `const pointer` (e.g., `const struct null_packet_write_args *`)
would perfectly guarantee that the state remains read-only.
Thomas, would it be worth submitting a separate patch now to fix `vidtv_ts_pcr_write_into()`
to prevent future KMSAN errors? Or would you prefer this to be addressed together in Abd-Alrhman's v2?
Best regards,
Yihan Ding
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
2026-02-20 13:39 Ding Yihan
@ 2026-02-20 13:56 ` Thomas Weißschuh
2026-02-20 14:58 ` Abd-Alrhman Masalkhi
2026-02-20 14:53 ` Abd-Alrhman Masalkhi
2026-02-21 10:31 ` Abd-Alrhman Masalkhi
2 siblings, 1 reply; 9+ messages in thread
From: Thomas Weißschuh @ 2026-02-20 13:56 UTC (permalink / raw)
To: Ding Yihan
Cc: Abd-Alrhman Masalkhi, dwlsalmeida, mchehab, linmag7, linux-media,
linux-kernel, syzbot+96f901260a0b2d29cd1a
Hi Yihan Ding,
On Fri, Feb 20, 2026 at 09:39:45PM +0800, Ding Yihan wrote:
> While looking into this exact same syzbot report, I noticed that
> `vidtv_ts_pcr_write_into()` in the same file also suffers from the
> exact same pass-by-value anti-pattern (passing `struct pcr_write_args` by value).
Good catch.
> Since `pcr_write_args` also contains implicit padding, it remains a potential trigger
> for identical KMSAN uninit-value warnings during fuzzing in the future.
The fact that the report is about implicit padding is valuable information.
It should be part of the commit message.
> Also, regarding Thomas's concern about modifying shared data: passing the struct
> as a `const pointer` (e.g., `const struct null_packet_write_args *`)
> would perfectly guarantee that the state remains read-only.
Agreed.
> Thomas, would it be worth submitting a separate patch now to fix
> `vidtv_ts_pcr_write_into()` to prevent future KMSAN errors? Or would you
> prefer this to be addressed together in Abd-Alrhman's v2?
Doing it together sounds better. This is not urgent anyways in my opinion.
But on the other hand I am just a random guy whose commit got wrongly blamed
in the original Fixes tag and I don't know anything about this subsystem.
Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
2026-02-20 13:39 Ding Yihan
2026-02-20 13:56 ` Thomas Weißschuh
@ 2026-02-20 14:53 ` Abd-Alrhman Masalkhi
2026-02-21 10:31 ` Abd-Alrhman Masalkhi
2 siblings, 0 replies; 9+ messages in thread
From: Abd-Alrhman Masalkhi @ 2026-02-20 14:53 UTC (permalink / raw)
To: Ding Yihan, Thomas Weißschuh
Cc: dwlsalmeida, mchehab, linmag7, linux-media, linux-kernel,
syzbot+96f901260a0b2d29cd1a
Hi Yihan Ding,
On Fri, Feb 20, 2026 at 21:39 +0800, Ding Yihan wrote:
> Hi Thomas and Abd-Alrhman,
>
> While looking into this exact same syzbot report, I noticed that
> `vidtv_ts_pcr_write_into()` in the same file also suffers from the
> exact same pass-by-value anti-pattern (passing `struct pcr_write_args` by value).
>
> Since `pcr_write_args` also contains implicit padding, it remains a potential trigger
> for identical KMSAN uninit-value warnings during fuzzing in the future.
>
> Also, regarding Thomas's concern about modifying shared data: passing the struct
> as a `const pointer` (e.g., `const struct null_packet_write_args *`)
> would perfectly guarantee that the state remains read-only.
>
> Thomas, would it be worth submitting a separate patch now to fix `vidtv_ts_pcr_write_into()`
> to prevent future KMSAN errors? Or would you prefer this to be addressed together in Abd-Alrhman's v2?
>
> Best regards,
> Yihan Ding
>
Thanks for pointing that out. I agree that vidtv_ts_pcr_write_into()
should be updated in the same way, since it has identical padding issues.
For v2 I’ll incorporate both fixes together and switch both parameters
add the const modifier, as you and Thomas suggested.
Thanks again for the helpful review.
--
Best Regards,
Abd-Alrhman
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
2026-02-20 13:56 ` Thomas Weißschuh
@ 2026-02-20 14:58 ` Abd-Alrhman Masalkhi
2026-02-20 16:32 ` Thomas Weißschuh
0 siblings, 1 reply; 9+ messages in thread
From: Abd-Alrhman Masalkhi @ 2026-02-20 14:58 UTC (permalink / raw)
To: Thomas Weißschuh, Ding Yihan
Cc: dwlsalmeida, mchehab, linmag7, linux-media, linux-kernel,
syzbot+96f901260a0b2d29cd1a
On Fri, Feb 20, 2026 at 14:56 +0100, Thomas Weißschuh wrote:
> Hi Yihan Ding,
>
> On Fri, Feb 20, 2026 at 09:39:45PM +0800, Ding Yihan wrote:
>> While looking into this exact same syzbot report, I noticed that
>> `vidtv_ts_pcr_write_into()` in the same file also suffers from the
>> exact same pass-by-value anti-pattern (passing `struct pcr_write_args` by value).
>
> Good catch.
>
>> Since `pcr_write_args` also contains implicit padding, it remains a potential trigger
>> for identical KMSAN uninit-value warnings during fuzzing in the future.
>
> The fact that the report is about implicit padding is valuable information.
> It should be part of the commit message.
>
>> Also, regarding Thomas's concern about modifying shared data: passing the struct
>> as a `const pointer` (e.g., `const struct null_packet_write_args *`)
>> would perfectly guarantee that the state remains read-only.
>
> Agreed.
>
>> Thomas, would it be worth submitting a separate patch now to fix
>> `vidtv_ts_pcr_write_into()` to prevent future KMSAN errors? Or would you
>> prefer this to be addressed together in Abd-Alrhman's v2?
>
> Doing it together sounds better. This is not urgent anyways in my opinion.
> But on the other hand I am just a random guy whose commit got wrongly blamed
> in the original Fixes tag and I don't know anything about this subsystem.
>
Sorry, I have misunderstood how the 'Fixes' tag works.
>
> Thomas
--
Best Regards,
Abd-Alrhman
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
2026-02-20 14:58 ` Abd-Alrhman Masalkhi
@ 2026-02-20 16:32 ` Thomas Weißschuh
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Weißschuh @ 2026-02-20 16:32 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi
Cc: Ding Yihan, dwlsalmeida, mchehab, linmag7, linux-media,
linux-kernel, syzbot+96f901260a0b2d29cd1a
On Fri, Feb 20, 2026 at 03:58:13PM +0100, Abd-Alrhman Masalkhi wrote:
> On Fri, Feb 20, 2026 at 14:56 +0100, Thomas Weißschuh wrote:
(...)
> >> Thomas, would it be worth submitting a separate patch now to fix
> >> `vidtv_ts_pcr_write_into()` to prevent future KMSAN errors? Or would you
> >> prefer this to be addressed together in Abd-Alrhman's v2?
> >
> > Doing it together sounds better. This is not urgent anyways in my opinion.
> > But on the other hand I am just a random guy whose commit got wrongly blamed
> > in the original Fixes tag and I don't know anything about this subsystem.
> >
> Sorry, I have misunderstood how the 'Fixes' tag works.
No need to be sorry. I didn't want to come across grumpy, but wanted to
express that I am just a bystander and have no authority over this code.
Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value
2026-02-20 13:39 Ding Yihan
2026-02-20 13:56 ` Thomas Weißschuh
2026-02-20 14:53 ` Abd-Alrhman Masalkhi
@ 2026-02-21 10:31 ` Abd-Alrhman Masalkhi
2 siblings, 0 replies; 9+ messages in thread
From: Abd-Alrhman Masalkhi @ 2026-02-21 10:31 UTC (permalink / raw)
To: Ding Yihan, Thomas Weißschuh
Cc: dwlsalmeida, mchehab, linmag7, linux-media, linux-kernel,
syzbot+96f901260a0b2d29cd1a
On Fri, Feb 20, 2026 at 21:39 +0800, Ding Yihan wrote:
> Hi Thomas and Abd-Alrhman,
>
> While looking into this exact same syzbot report, I noticed that
> `vidtv_ts_pcr_write_into()` in the same file also suffers from the
> exact same pass-by-value anti-pattern (passing `struct pcr_write_args` by value).
>
> Since `pcr_write_args` also contains implicit padding, it remains a potential trigger
> for identical KMSAN uninit-value warnings during fuzzing in the future.
>
> Also, regarding Thomas's concern about modifying shared data: passing the struct
> as a `const pointer` (e.g., `const struct null_packet_write_args *`)
> would perfectly guarantee that the state remains read-only.
>
> Thomas, would it be worth submitting a separate patch now to fix `vidtv_ts_pcr_write_into()`
> to prevent future KMSAN errors? Or would you prefer this to be addressed together in Abd-Alrhman's v2?
>
> Best regards,
> Yihan Ding
>
Hi Yihan Ding,
Thanks again for pointing this out earlier.
I've added a fix for vidtv_ts_pcr_write_into() in v2, and I'd like to
include a Suggested-by: tag for you in the patch. I'm still new here,
but my understanding is that Suggested-by is the appropriate tag in
this case.
--
Best Regards,
Abd-Alrhman
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-02-21 10:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-16 21:17 [PATCH] media: vidtv: fix uninitialized args.buf_sz passed by value Abd-Alrhman Masalkhi
2026-02-18 13:24 ` Thomas Weißschuh
2026-02-19 10:17 ` Abd-Alrhman Masalkhi
-- strict thread matches above, loose matches on Subject: below --
2026-02-20 13:39 Ding Yihan
2026-02-20 13:56 ` Thomas Weißschuh
2026-02-20 14:58 ` Abd-Alrhman Masalkhi
2026-02-20 16:32 ` Thomas Weißschuh
2026-02-20 14:53 ` Abd-Alrhman Masalkhi
2026-02-21 10:31 ` Abd-Alrhman Masalkhi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox