netdev - Re: [PATCH net-next v2 3/5] netconsole: add support for strings with new line in netpoll_parse_ip

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: <20250723144933.GA1036606@horms.kernel.org>
Date: Wed, 23 Jul 2025 15:54:11 +0100
From: Simon Horman <horms@...nel.org>
To: Breno Leitao <leitao@...ian.org>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Andrew Lunn <andrew+netdev@...n.ch>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH net-next v2 3/5] netconsole: add support for strings with
 new line in netpoll_parse_ip_addr

On Mon, Jul 21, 2025 at 06:02:03AM -0700, Breno Leitao wrote:
> The current IP address parsing logic fails when the input string
> contains a trailing newline character. This can occur when IP
> addresses are provided through configfs, which contains newlines in
> a const buffer.
> 
> Teach netpoll_parse_ip_addr() how to ignore newlines at the end of the
> IPs. Also, simplify the code by:
> 
>  * No need to check for separators. Try to parse ipv4, if it fails try
>    ipv6 similarly to ceph_pton()
>  * If ipv6 is not supported, don't call in6_pton() at all.
> 
> Signed-off-by: Breno Leitao <leitao@...ian.org>

My suggestion below not withstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@...nel.org>

> ---
>  drivers/net/netconsole.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 8d1b93264e0fd..f2c2b8852c603 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -303,20 +303,21 @@ static void netconsole_print_banner(struct netpoll *np)
>  static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
>  {
>  	const char *end;
> +	int len;
>  
> -	if (!strchr(str, ':') &&
> -	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
> -		if (!*end)
> -			return 0;
> -	}
> -	if (in6_pton(str, -1, addr->in6.s6_addr, -1, &end) > 0) {
> -#if IS_ENABLED(CONFIG_IPV6)
> -		if (!*end)
> -			return 1;
> -#else
> +	len = strlen(str);
> +	if (!len)
>  		return -1;
> +
> +	if (str[len - 1] == '\n')
> +		len -= 1;
> +
> +	if (in4_pton(str, len, (void *)addr, -1, &end) > 0)
> +		return 0;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (in6_pton(str, len, addr->in6.s6_addr, -1, &end) > 0)
> +		return 1;
>  #endif

I don't think it needs to block progress.
But FWIIW, I think it would be nice to increase
build coverage and express this as:

	if (IS_ENABLED(CONFIG_IPV6) &&
	    in6_pton(str, len, addr->in6.s6_addr, -1, &end) > 0)
		return 1;