Re: [PATCH V6 2/4] sctp: Add ip option support
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: 2018-02-14 20:30:52
Also in:
linux-sctp, linux-security-module, selinux
Hi, On Tue, Feb 13, 2018 at 08:54:44PM +0000, Richard Haines wrote: ...
quoted hunk ↗ jump to hunk
diff --git a/net/sctp/socket.c b/net/sctp/socket.c index bf271f8..8307968 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c@@ -3138,6 +3138,7 @@ static int sctp_setsockopt_mappedv4(struct sock *sk, char __user *optval, unsign static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned int optlen) { struct sctp_sock *sp = sctp_sk(sk); + struct sctp_af *af = sp->pf->af; struct sctp_assoc_value params; struct sctp_association *asoc; int val;@@ -3159,10 +3160,13 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned return -EINVAL; } + asoc = sctp_id2assoc(sk, params.assoc_id); + if (val) { int min_len, max_len; - min_len = SCTP_DEFAULT_MINSEGMENT - sp->pf->af->net_header_len; + min_len = SCTP_DEFAULT_MINSEGMENT - af->net_header_len; + min_len -= af->ip_options_len(asoc->base.sk);
We have an issue here. asoc may be NULL, in case the user supplied asoc id is invalid. Then if it also specified the maxseg 'val', it would trigger a NULL deref here. You don't need to find the asoc, to then get to the socket. You can use the sk function parameter, it's the same value here. Then you don't need to move the sctp_id2assoc() call.
quoted hunk ↗ jump to hunk
min_len -= sizeof(struct sctphdr) + sizeof(struct sctp_data_chunk);@@ -3172,10 +3176,10 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned return -EINVAL; } - asoc = sctp_id2assoc(sk, params.assoc_id); if (asoc) { if (val == 0) { - val = asoc->pathmtu - sp->pf->af->net_header_len; + val = asoc->pathmtu - af->net_header_len; + val -= af->ip_options_len(asoc->base.sk);
You may use sk directly here too. Other than these, LGTM.
quoted hunk ↗ jump to hunk
val -= sizeof(struct sctphdr) + sctp_datachk_len(&asoc->stream); }@@ -5087,9 +5091,11 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp) sctp_copy_sock(sock->sk, sk, asoc); /* Make peeled-off sockets more like 1-1 accepted sockets. - * Set the daddr and initialize id to something more random + * Set the daddr and initialize id to something more random and also + * copy over any ip options. */ sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sk); + sp->pf->copy_ip_options(sk, sock->sk); /* Populate the fields of the newsk from the oldsk and migrate the * asoc to the newsk.-- 2.14.3