netdev - Re: [PATCH net] sctp: always set frag

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]

Date:   Tue, 4 Dec 2018 16:58:32 -0200
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Jakub Audykowicz <jakub.audykowicz@...il.com>
Cc:     Xin Long <lucien.xin@...il.com>, linux-sctp@...r.kernel.org,
        Vlad Yasevich <vyasevich@...il.com>,
        Neil Horman <nhorman@...driver.com>,
        davem <davem@...emloft.net>, network dev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] sctp: always set frag_point on pmtu change

On Tue, Dec 04, 2018 at 07:51:54PM +0100, Jakub Audykowicz wrote:
...
> Thanks, I've taken your remarks into account and ended up with this 
> simple solution:

LGTM! Thanks

> 
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index ab9242e51d9e..3487686f2cf5 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -620,4 +620,9 @@ static inline bool sctp_transport_pmtu_check(struct sctp_transport *t)
>  	return false;
>  }
>  
> +static inline __u32 sctp_min_frag_point(struct sctp_sock *sp, __u16 datasize)
> +{
> +    return sctp_mtu_payload(sp, SCTP_DEFAULT_MINSEGMENT, datasize);

Not sure how final the patch is but be sure to run checkpatch.pl on
it before submitting it officially. It flagged some issues like the
above indentation using spaces, for example.

> +}
> +
>  #endif /* __net_sctp_h__ */
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index ce8087846f05..dc12c2ba487f 100644
> --- a/net/sctp/chunk.c
> +++ b/net/sctp/chunk.c
> @@ -191,6 +191,12 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
>  	 * the packet
>  	 */
>  	max_data = asoc->frag_point;
> +	if (unlikely(!max_data)) {
> +		pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum",
> +			__func__, asoc);
> +		max_data = sctp_min_frag_point(
> +			sctp_sk(asoc->base.sk), sctp_datachk_len(&asoc->stream));
> +	}
>  
>  	/* If the the peer requested that we authenticate DATA chunks
>  	 * we need to account for bundling of the AUTH chunks along with
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index bf618d1b41fd..b8cebd5a87e5 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -3324,8 +3324,7 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
>  		__u16 datasize = asoc ? sctp_datachk_len(&asoc->stream) :
>  				 sizeof(struct sctp_data_chunk);
>  
> -		min_len = sctp_mtu_payload(sp, SCTP_DEFAULT_MINSEGMENT,
> -					   datasize);
> +		min_len = sctp_min_frag_point(sp, datasize);
>  		max_len = SCTP_MAX_CHUNK_LEN - datasize;
>  
>  		if (val < min_len || val > max_len)
> 
>  
>