netdev - Re: [PATCH net] tipc: Return non-zero value from tipc_udp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240716.164535.1952205982608398288.syoshida@redhat.com>
Date: Tue, 16 Jul 2024 16:45:35 +0900 (JST)
From: Shigeru Yoshida <syoshida@...hat.com>
To: tung.q.nguyen@...ava.com
Cc: jmaloy@...hat.com, ying.xue@...driver.com, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 netdev@...r.kernel.org, tipc-discussion@...ts.sourceforge.net,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] tipc: Return non-zero value from
 tipc_udp_addr2str() on error

Hi Tung,

On Tue, 16 Jul 2024 07:35:50 +0000, Tung Nguyen wrote:
>>tipc_udp_addr2str() should return non-zero value if the UDP media address is invalid. Otherwise, a buffer overflow access can occur in
>>tipc_media_addr_printf(). Fix this by returning 1 on an invalid UDP media address.
>>
>>Fixes: d0f91938bede ("tipc: add ip/udp media type")
>>Signed-off-by: Shigeru Yoshida <syoshida@...hat.com>
>>---
>> net/tipc/udp_media.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>>diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c index b849a3d133a0..439f75539977 100644
>>--- a/net/tipc/udp_media.c
>>+++ b/net/tipc/udp_media.c
>>@@ -135,8 +135,11 @@ static int tipc_udp_addr2str(struct tipc_media_addr *a, char *buf, int size)
>>                snprintf(buf, size, "%pI4:%u", &ua->ipv4, ntohs(ua->port));
>>        else if (ntohs(ua->proto) == ETH_P_IPV6)
>>                snprintf(buf, size, "%pI6:%u", &ua->ipv6, ntohs(ua->port));
>>-       else
>>+       else {
>>                pr_err("Invalid UDP media address\n");
>>+               return 1;
> Please use -EINVAL instead.

Other addr2str functions like tipc_eth_addr2str() use 1, so I followed
convention. But -EINVAL is more appropriate, as you say.

Thanks,
Shigeru