netdev - Re: [PATCH 3/9] sctp: Check address length before reading srx

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: <20190412111219.GB15012@hmswarspite.think-freely.org>
Date:   Fri, 12 Apr 2019 07:12:19 -0400
From:   Neil Horman <nhorman@...driver.com>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     Vlad Yasevich <vyasevich@...il.com>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH 3/9] sctp: Check address length before reading
 srx_service field

On Fri, Apr 12, 2019 at 07:53:10PM +0900, Tetsuo Handa wrote:
> KMSAN will complain if valid address length passed to connect() is shorter
> than sizeof("struct sockaddr"->sa_family) bytes.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> ---
>  net/sctp/socket.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9874e60c9b0d..4583fa914e62 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4847,7 +4847,8 @@ static int sctp_connect(struct sock *sk, struct sockaddr *addr,
>  	}
>  
>  	/* Validate addr_len before calling common connect/connectx routine. */
> -	af = sctp_get_af_specific(addr->sa_family);
> +	af = addr_len < offsetofend(struct sockaddr, sa_family) ? NULL :
> +		sctp_get_af_specific(addr->sa_family);
I don't see anything wrong with the patch per se, but sctp_get_af_specific will
always return a value that is greater than offsetofend(struct sockaddr,
sa_family).  So while this patch doesn't do anything wrong, it seems your fixing
an erroneous KMSAN warning in the SCTP code.  Shouldn't KMSAN just be made to
not complain if the address length is valid?

Neil

>  	if (!af || addr_len < af->sockaddr_len) {
>  		err = -EINVAL;
>  	} else {
> -- 
> 2.16.5
> 
>