* [PATCH] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field
@ 2021-04-05 5:33 Tetsuo Handa
2021-04-05 7:39 ` Sven Eckelmann
0 siblings, 1 reply; 7+ messages in thread
From: Tetsuo Handa @ 2021-04-05 5:33 UTC (permalink / raw)
To: Marek Lindner, Simon Wunderlich, Antonio Quartulli,
Sven Eckelmann, David S. Miller, Jakub Kicinski
Cc: netdev, Tetsuo Handa
KMSAN found uninitialized value at batadv_tt_prepare_tvlv_local_data()
[1], for commit ced72933a5e8ab52 ("batman-adv: use CRC32C instead of CRC16
in TT code") inserted 'reserved' field into "struct batadv_tvlv_tt_data"
and commit 7ea7b4a142758dea ("batman-adv: make the TT CRC logic VLAN
specific") moved that field to "struct batadv_tvlv_tt_vlan_data" but left
that field uninitialized.
Although this patch passed "#syz test:" check, there might be similar bugs
remaining. It might be better to proactively use __GFP_ZERO than trying to
find uninitialized fields in this module.
[1] https://syzkaller.appspot.com/bug?id=07f3e6dba96f0eb3cabab986adcd8a58b9bdbe9d
Reported-by: syzbot <syzbot+50ee810676e6a089487b@syzkaller.appspotmail.com>
Tested-by: syzbot <syzbot+50ee810676e6a089487b@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: ced72933a5e8ab52 ("batman-adv: use CRC32C instead of CRC16 in TT code")
Fixes: 7ea7b4a142758dea ("batman-adv: make the TT CRC logic VLAN specific")
---
net/batman-adv/translation-table.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index f8761281aab0..eb82576557e6 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -973,6 +973,7 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv,
tt_vlan->vid = htons(vlan->vid);
tt_vlan->crc = htonl(vlan->tt.crc);
+ tt_vlan->reserved = 0;
tt_vlan++;
}
--
2.18.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field
2021-04-05 5:33 [PATCH] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field Tetsuo Handa
@ 2021-04-05 7:39 ` Sven Eckelmann
2021-04-05 10:02 ` Tetsuo Handa
0 siblings, 1 reply; 7+ messages in thread
From: Sven Eckelmann @ 2021-04-05 7:39 UTC (permalink / raw)
To: Marek Lindner, Simon Wunderlich, Antonio Quartulli,
David S. Miller, Jakub Kicinski, Tetsuo Handa
Cc: netdev, Tetsuo Handa
[-- Attachment #1: Type: text/plain, Size: 726 bytes --]
On Monday, 5 April 2021 07:33:06 CEST Tetsuo Handa wrote:
[...]
> ---
> net/batman-adv/translation-table.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index f8761281aab0..eb82576557e6 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c
> @@ -973,6 +973,7 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv,
>
> tt_vlan->vid = htons(vlan->vid);
> tt_vlan->crc = htonl(vlan->tt.crc);
> + tt_vlan->reserved = 0;
>
> tt_vlan++;
> }
>
Thanks but this patch is incomplete. Please also fix
batadv_tt_prepare_tvlv_global_data (exactly the same way)
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field
2021-04-05 7:39 ` Sven Eckelmann
@ 2021-04-05 10:02 ` Tetsuo Handa
2021-04-05 10:06 ` Sven Eckelmann
0 siblings, 1 reply; 7+ messages in thread
From: Tetsuo Handa @ 2021-04-05 10:02 UTC (permalink / raw)
To: Sven Eckelmann, Marek Lindner, Simon Wunderlich,
Antonio Quartulli, David S. Miller, Jakub Kicinski
Cc: netdev
On 2021/04/05 16:39, Sven Eckelmann wrote:
> On Monday, 5 April 2021 07:33:06 CEST Tetsuo Handa wrote:
> [...]
>> ---
>> net/batman-adv/translation-table.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
>> index f8761281aab0..eb82576557e6 100644
>> --- a/net/batman-adv/translation-table.c
>> +++ b/net/batman-adv/translation-table.c
>> @@ -973,6 +973,7 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv,
>>
>> tt_vlan->vid = htons(vlan->vid);
>> tt_vlan->crc = htonl(vlan->tt.crc);
>> + tt_vlan->reserved = 0;
>>
>> tt_vlan++;
>> }
>>
>
> Thanks but this patch is incomplete. Please also fix
> batadv_tt_prepare_tvlv_global_data (exactly the same way)
Indeed. Hmm, batadv_send_tt_request() is already using kzalloc().
Which approach ( "->reserved = 0" or "kzalloc()") do you prefer for
batadv_tt_prepare_tvlv_global_data() and batadv_tt_prepare_tvlv_local_data() ?
>
> Kind regards,
> Sven
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field
2021-04-05 10:02 ` Tetsuo Handa
@ 2021-04-05 10:06 ` Sven Eckelmann
2021-04-05 10:16 ` [PATCH v2] " Tetsuo Handa
0 siblings, 1 reply; 7+ messages in thread
From: Sven Eckelmann @ 2021-04-05 10:06 UTC (permalink / raw)
To: Marek Lindner, Simon Wunderlich, Antonio Quartulli,
David S. Miller, Jakub Kicinski, Tetsuo Handa
Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 508 bytes --]
On Monday, 5 April 2021 12:02:02 CEST Tetsuo Handa wrote:
[...]
> > Thanks but this patch is incomplete. Please also fix
> > batadv_tt_prepare_tvlv_global_data (exactly the same way)
>
> Indeed. Hmm, batadv_send_tt_request() is already using kzalloc().
> Which approach ( "->reserved = 0" or "kzalloc()") do you prefer for
> batadv_tt_prepare_tvlv_global_data() and batadv_tt_prepare_tvlv_local_data() ?
Lets stick with the approach you've used in your v1 version - "->reserved = 0"
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field
2021-04-05 10:06 ` Sven Eckelmann
@ 2021-04-05 10:16 ` Tetsuo Handa
2021-04-05 11:09 ` Sven Eckelmann
2021-04-05 22:20 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 7+ messages in thread
From: Tetsuo Handa @ 2021-04-05 10:16 UTC (permalink / raw)
To: Marek Lindner, Simon Wunderlich, Antonio Quartulli,
Sven Eckelmann, David S. Miller, Jakub Kicinski
Cc: netdev, Tetsuo Handa
KMSAN found uninitialized value at batadv_tt_prepare_tvlv_local_data()
[1], for commit ced72933a5e8ab52 ("batman-adv: use CRC32C instead of CRC16
in TT code") inserted 'reserved' field into "struct batadv_tvlv_tt_data"
and commit 7ea7b4a142758dea ("batman-adv: make the TT CRC logic VLAN
specific") moved that field to "struct batadv_tvlv_tt_vlan_data" but left
that field uninitialized.
[1] https://syzkaller.appspot.com/bug?id=07f3e6dba96f0eb3cabab986adcd8a58b9bdbe9d
Reported-by: syzbot <syzbot+50ee810676e6a089487b@syzkaller.appspotmail.com>
Tested-by: syzbot <syzbot+50ee810676e6a089487b@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: ced72933a5e8ab52 ("batman-adv: use CRC32C instead of CRC16 in TT code")
Fixes: 7ea7b4a142758dea ("batman-adv: make the TT CRC logic VLAN specific")
---
net/batman-adv/translation-table.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index f8761281aab0..434b4f042909 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -890,6 +890,7 @@ batadv_tt_prepare_tvlv_global_data(struct batadv_orig_node *orig_node,
hlist_for_each_entry(vlan, &orig_node->vlan_list, list) {
tt_vlan->vid = htons(vlan->vid);
tt_vlan->crc = htonl(vlan->tt.crc);
+ tt_vlan->reserved = 0;
tt_vlan++;
}
@@ -973,6 +974,7 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv,
tt_vlan->vid = htons(vlan->vid);
tt_vlan->crc = htonl(vlan->tt.crc);
+ tt_vlan->reserved = 0;
tt_vlan++;
}
--
2.18.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field
2021-04-05 10:16 ` [PATCH v2] " Tetsuo Handa
@ 2021-04-05 11:09 ` Sven Eckelmann
2021-04-05 22:20 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 7+ messages in thread
From: Sven Eckelmann @ 2021-04-05 11:09 UTC (permalink / raw)
To: Marek Lindner, Simon Wunderlich, Antonio Quartulli,
David S. Miller, Jakub Kicinski, Tetsuo Handa
Cc: netdev, Tetsuo Handa
[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]
On Monday, 5 April 2021 12:16:50 CEST Tetsuo Handa wrote:
> KMSAN found uninitialized value at batadv_tt_prepare_tvlv_local_data()
> [1], for commit ced72933a5e8ab52 ("batman-adv: use CRC32C instead of CRC16
> in TT code") inserted 'reserved' field into "struct batadv_tvlv_tt_data"
> and commit 7ea7b4a142758dea ("batman-adv: make the TT CRC logic VLAN
> specific") moved that field to "struct batadv_tvlv_tt_vlan_data" but left
> that field uninitialized.
>
> [1] https://syzkaller.appspot.com/bug?id=07f3e6dba96f0eb3cabab986adcd8a58b9bdbe9d
>
> Reported-by: syzbot <syzbot+50ee810676e6a089487b@syzkaller.appspotmail.com>
> Tested-by: syzbot <syzbot+50ee810676e6a089487b@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Fixes: ced72933a5e8ab52 ("batman-adv: use CRC32C instead of CRC16 in TT code")
> Fixes: 7ea7b4a142758dea ("batman-adv: make the TT CRC logic VLAN specific")
> ---
> net/batman-adv/translation-table.c | 2 ++
> 1 file changed, 2 insertions(+)
Thanks,
Acked-by: Sven Eckelmann <sven@narfation.org>
> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index f8761281aab0..434b4f042909 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c
> @@ -890,6 +890,7 @@ batadv_tt_prepare_tvlv_global_data(struct batadv_orig_node *orig_node,
> hlist_for_each_entry(vlan, &orig_node->vlan_list, list) {
> tt_vlan->vid = htons(vlan->vid);
> tt_vlan->crc = htonl(vlan->tt.crc);
> + tt_vlan->reserved = 0;
>
> tt_vlan++;
> }
> @@ -973,6 +974,7 @@ batadv_tt_prepare_tvlv_local_data(struct batadv_priv *bat_priv,
>
> tt_vlan->vid = htons(vlan->vid);
> tt_vlan->crc = htonl(vlan->tt.crc);
> + tt_vlan->reserved = 0;
>
> tt_vlan++;
> }
>
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field
2021-04-05 10:16 ` [PATCH v2] " Tetsuo Handa
2021-04-05 11:09 ` Sven Eckelmann
@ 2021-04-05 22:20 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-04-05 22:20 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: mareklindner, sw, a, sven, davem, kuba, netdev
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Mon, 5 Apr 2021 19:16:50 +0900 you wrote:
> KMSAN found uninitialized value at batadv_tt_prepare_tvlv_local_data()
> [1], for commit ced72933a5e8ab52 ("batman-adv: use CRC32C instead of CRC16
> in TT code") inserted 'reserved' field into "struct batadv_tvlv_tt_data"
> and commit 7ea7b4a142758dea ("batman-adv: make the TT CRC logic VLAN
> specific") moved that field to "struct batadv_tvlv_tt_vlan_data" but left
> that field uninitialized.
>
> [...]
Here is the summary with links:
- [v2] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field
https://git.kernel.org/netdev/net/c/08c27f3322fe
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-05 22:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-05 5:33 [PATCH] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field Tetsuo Handa
2021-04-05 7:39 ` Sven Eckelmann
2021-04-05 10:02 ` Tetsuo Handa
2021-04-05 10:06 ` Sven Eckelmann
2021-04-05 10:16 ` [PATCH v2] " Tetsuo Handa
2021-04-05 11:09 ` Sven Eckelmann
2021-04-05 22:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox