[PATCH net] sctp: frag_point sanity check

[PATCH net] sctp: frag_point sanity check

[Jakub Audykowicz]

If for some reason an association's fragmentation point is zero,
sctp_datamsg_from_user will try to endlessly try to divide a message
into zero-sized chunks. This eventually causes kernel panic due to
running out of memory.

Although this situation is quite unlikely, it has occurred before as
reported. I propose to add this simple last-ditch sanity check due to
the severity of the potential consequences.

Signed-off-by: Jakub Audykowicz <jakub.audykowicz@gmail.com>
---
 include/net/sctp/sctp.h | 5 +++++
 net/sctp/chunk.c        | 6 ++++++
 net/sctp/socket.c       | 3 +--
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index ab9242e51d9e..2abbc15824af 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);
+}
+
 #endif /* __net_sctp_h__ */
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index ce8087846f05..d5b91bc8a377 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)) {
+		max_data = sctp_min_frag_point(sctp_sk(asoc->base.sk),
+					       sctp_datachk_len(&asoc->stream));
+		pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
+				    __func__, asoc, max_data);
+	}
 
 	/* 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)
-- 
2.17.1

Re: [PATCH net] sctp: frag_point sanity check

[Neil Horman]

On Tue, Dec 04, 2018 at 08:27:41PM +0100, Jakub Audykowicz wrote:
> If for some reason an association's fragmentation point is zero,
> sctp_datamsg_from_user will try to endlessly try to divide a message
> into zero-sized chunks. This eventually causes kernel panic due to
> running out of memory.
> 
> Although this situation is quite unlikely, it has occurred before as
> reported. I propose to add this simple last-ditch sanity check due to
> the severity of the potential consequences.
> 
> Signed-off-by: Jakub Audykowicz <jakub.audykowicz@gmail.com>
> ---
>  include/net/sctp/sctp.h | 5 +++++
>  net/sctp/chunk.c        | 6 ++++++
>  net/sctp/socket.c       | 3 +--
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index ab9242e51d9e..2abbc15824af 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);
> +}
> +
>  #endif /* __net_sctp_h__ */
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index ce8087846f05..d5b91bc8a377 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)) {
> +		max_data = sctp_min_frag_point(sctp_sk(asoc->base.sk),
> +					       sctp_datachk_len(&asoc->stream));
> +		pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
> +				    __func__, asoc, max_data);
> +	}
>  
>  	/* 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)
> -- 
> 2.17.1
> 
> 
Why not just prevent the frag point from ever going below
SCTP_DEFAULT_MINSEGMENT in the first place in sctp_assoc_update_frag_point?
Something like:

asoc->frag_point = SCTP_TRUNC4(frag) < SCTP_DEFAILT_MINSEGMENT) ? \
SCTP_DEFAILT_MINSEGMENT : SCTP_TRUNC4(frag);

Should do the trick I would think
Neil

Re: [PATCH net] sctp: frag_point sanity check

[Marcelo Ricardo Leitner]

On Tue, Dec 04, 2018 at 02:39:46PM -0500, Neil Horman wrote:
> On Tue, Dec 04, 2018 at 08:27:41PM +0100, Jakub Audykowicz wrote:
> > If for some reason an association's fragmentation point is zero,
> > sctp_datamsg_from_user will try to endlessly try to divide a message
> > into zero-sized chunks. This eventually causes kernel panic due to
> > running out of memory.
> > 
> > Although this situation is quite unlikely, it has occurred before as
> > reported. I propose to add this simple last-ditch sanity check due to
> > the severity of the potential consequences.
> > 
> > Signed-off-by: Jakub Audykowicz <jakub.audykowicz@gmail.com>
> > ---
> >  include/net/sctp/sctp.h | 5 +++++
> >  net/sctp/chunk.c        | 6 ++++++
> >  net/sctp/socket.c       | 3 +--
> >  3 files changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> > index ab9242e51d9e..2abbc15824af 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);
> > +}
> > +
> >  #endif /* __net_sctp_h__ */
> > diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> > index ce8087846f05..d5b91bc8a377 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)) {
> > +		max_data = sctp_min_frag_point(sctp_sk(asoc->base.sk),
> > +					       sctp_datachk_len(&asoc->stream));
> > +		pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
> > +				    __func__, asoc, max_data);
> > +	}
> >  
> >  	/* 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)
> > -- 
> > 2.17.1
> > 
> > 
> Why not just prevent the frag point from ever going below
> SCTP_DEFAULT_MINSEGMENT in the first place in sctp_assoc_update_frag_point?
> Something like:
> 
> asoc->frag_point = SCTP_TRUNC4(frag) < SCTP_DEFAILT_MINSEGMENT) ? \
> SCTP_DEFAILT_MINSEGMENT : SCTP_TRUNC4(frag);
> 
> Should do the trick I would think
> Neil

This is in the light of "sctp: update frag_point when
stream_interleave is set".

Because of
https://www.mail-archive.com/netdev@vger.kernel.org/msg256575.html
This wouldn't have helped because sctp_assoc_update_frag_point()
didn't get called. The issue is not that the calc issued a bad value,
but that it wasn't done.
 
  Marcelo

Re: [PATCH net] sctp: frag_point sanity check

[Neil Horman]

On Tue, Dec 04, 2018 at 05:52:02PM -0200, Marcelo Ricardo Leitner wrote:
> On Tue, Dec 04, 2018 at 02:39:46PM -0500, Neil Horman wrote:
> > On Tue, Dec 04, 2018 at 08:27:41PM +0100, Jakub Audykowicz wrote:
> > > If for some reason an association's fragmentation point is zero,
> > > sctp_datamsg_from_user will try to endlessly try to divide a message
> > > into zero-sized chunks. This eventually causes kernel panic due to
> > > running out of memory.
> > > 
> > > Although this situation is quite unlikely, it has occurred before as
> > > reported. I propose to add this simple last-ditch sanity check due to
> > > the severity of the potential consequences.
> > > 
> > > Signed-off-by: Jakub Audykowicz <jakub.audykowicz@gmail.com>
> > > ---
> > >  include/net/sctp/sctp.h | 5 +++++
> > >  net/sctp/chunk.c        | 6 ++++++
> > >  net/sctp/socket.c       | 3 +--
> > >  3 files changed, 12 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> > > index ab9242e51d9e..2abbc15824af 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);
> > > +}
> > > +
> > >  #endif /* __net_sctp_h__ */
> > > diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> > > index ce8087846f05..d5b91bc8a377 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)) {
> > > +		max_data = sctp_min_frag_point(sctp_sk(asoc->base.sk),
> > > +					       sctp_datachk_len(&asoc->stream));
> > > +		pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
> > > +				    __func__, asoc, max_data);
> > > +	}
> > >  
> > >  	/* 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)
> > > -- 
> > > 2.17.1
> > > 
> > > 
> > Why not just prevent the frag point from ever going below
> > SCTP_DEFAULT_MINSEGMENT in the first place in sctp_assoc_update_frag_point?
> > Something like:
> > 
> > asoc->frag_point = SCTP_TRUNC4(frag) < SCTP_DEFAILT_MINSEGMENT) ? \
> > SCTP_DEFAILT_MINSEGMENT : SCTP_TRUNC4(frag);
> > 
> > Should do the trick I would think
> > Neil
> 
> This is in the light of "sctp: update frag_point when
> stream_interleave is set".
> 
> Because of
> https://www.mail-archive.com/netdev@vger.kernel.org/msg256575.html
> This wouldn't have helped because sctp_assoc_update_frag_point()
> didn't get called. The issue is not that the calc issued a bad value,
> but that it wasn't done.
>  
>   Marcelo
> 
Ah, thank you for the clarification.

Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH net] sctp: frag_point sanity check

[Marcelo Ricardo Leitner]

On Tue, Dec 04, 2018 at 03:56:33PM -0500, Neil Horman wrote:
> On Tue, Dec 04, 2018 at 05:52:02PM -0200, Marcelo Ricardo Leitner wrote:
> > On Tue, Dec 04, 2018 at 02:39:46PM -0500, Neil Horman wrote:
> > > On Tue, Dec 04, 2018 at 08:27:41PM +0100, Jakub Audykowicz wrote:
> > > > If for some reason an association's fragmentation point is zero,
> > > > sctp_datamsg_from_user will try to endlessly try to divide a message
> > > > into zero-sized chunks. This eventually causes kernel panic due to
> > > > running out of memory.
> > > > 
> > > > Although this situation is quite unlikely, it has occurred before as
> > > > reported. I propose to add this simple last-ditch sanity check due to
> > > > the severity of the potential consequences.
> > > > 
> > > > Signed-off-by: Jakub Audykowicz <jakub.audykowicz@gmail.com>
> > > > ---
> > > >  include/net/sctp/sctp.h | 5 +++++
> > > >  net/sctp/chunk.c        | 6 ++++++
> > > >  net/sctp/socket.c       | 3 +--
> > > >  3 files changed, 12 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> > > > index ab9242e51d9e..2abbc15824af 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);
> > > > +}
> > > > +
> > > >  #endif /* __net_sctp_h__ */
> > > > diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> > > > index ce8087846f05..d5b91bc8a377 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)) {
> > > > +		max_data = sctp_min_frag_point(sctp_sk(asoc->base.sk),
> > > > +					       sctp_datachk_len(&asoc->stream));
> > > > +		pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
> > > > +				    __func__, asoc, max_data);
> > > > +	}
> > > >  
> > > >  	/* 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)
> > > > -- 
> > > > 2.17.1
> > > > 
> > > > 
> > > Why not just prevent the frag point from ever going below
> > > SCTP_DEFAULT_MINSEGMENT in the first place in sctp_assoc_update_frag_point?
> > > Something like:
> > > 
> > > asoc->frag_point = SCTP_TRUNC4(frag) < SCTP_DEFAILT_MINSEGMENT) ? \
> > > SCTP_DEFAILT_MINSEGMENT : SCTP_TRUNC4(frag);
> > > 
> > > Should do the trick I would think
> > > Neil
> > 
> > This is in the light of "sctp: update frag_point when
> > stream_interleave is set".
> > 
> > Because of
> > https://www.mail-archive.com/netdev@vger.kernel.org/msg256575.html
> > This wouldn't have helped because sctp_assoc_update_frag_point()
> > didn't get called. The issue is not that the calc issued a bad value,
> > but that it wasn't done.
> >  
> >   Marcelo
> > 
> Ah, thank you for the clarification.
> 
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> 

Cool!

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

Re: [PATCH net] sctp: frag_point sanity check

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7617 bytes --]

Hi Jakub,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]

url:    https://github.com/0day-ci/linux/commits/Jakub-Audykowicz/sctp-frag_point-sanity-check/20181206-011917
config: x86_64-randconfig-x015-12051035 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from net/sctp/chunk.c:36:
   net/sctp/chunk.c: In function 'sctp_datamsg_from_user':
   include/linux/kern_levels.h:5:18: warning: format '%d' expects argument of type 'int', but argument 4 has type 'size_t {aka long unsigned int}' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:429:10: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
             ^~~
   include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
    #define KERN_WARNING KERN_SOH "4" /* warning conditions */
                         ^~~~~~~~
   include/linux/printk.h:445:21: note: in expansion of macro 'KERN_WARNING'
     printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
                        ^~~~~~~~~~~~
>> net/sctp/chunk.c:197:3: note: in expansion of macro 'pr_warn_ratelimited'
      pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
      ^~~~~~~~~~~~~~~~~~~

vim +/pr_warn_ratelimited +197 net/sctp/chunk.c

   156	
   157	
   158	/* A data chunk can have a maximum payload of (2^16 - 20).  Break
   159	 * down any such message into smaller chunks.  Opportunistically, fragment
   160	 * the chunks down to the current MTU constraints.  We may get refragmented
   161	 * later if the PMTU changes, but it is _much better_ to fragment immediately
   162	 * with a reasonable guess than always doing our fragmentation on the
   163	 * soft-interrupt.
   164	 */
   165	struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
   166						    struct sctp_sndrcvinfo *sinfo,
   167						    struct iov_iter *from)
   168	{
   169		size_t len, first_len, max_data, remaining;
   170		size_t msg_len = iov_iter_count(from);
   171		struct sctp_shared_key *shkey = NULL;
   172		struct list_head *pos, *temp;
   173		struct sctp_chunk *chunk;
   174		struct sctp_datamsg *msg;
   175		int err;
   176	
   177		msg = sctp_datamsg_new(GFP_KERNEL);
   178		if (!msg)
   179			return ERR_PTR(-ENOMEM);
   180	
   181		/* Note: Calculate this outside of the loop, so that all fragments
   182		 * have the same expiration.
   183		 */
   184		if (asoc->peer.prsctp_capable && sinfo->sinfo_timetolive &&
   185		    (SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags) ||
   186		     !SCTP_PR_POLICY(sinfo->sinfo_flags)))
   187			msg->expires_at = jiffies +
   188					  msecs_to_jiffies(sinfo->sinfo_timetolive);
   189	
   190		/* This is the biggest possible DATA chunk that can fit into
   191		 * the packet
   192		 */
   193		max_data = asoc->frag_point;
   194		if (unlikely(!max_data)) {
   195			max_data = sctp_min_frag_point(sctp_sk(asoc->base.sk),
   196						       sctp_datachk_len(&asoc->stream));
 > 197			pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
   198					    __func__, asoc, max_data);
   199		}
   200	
   201		/* If the the peer requested that we authenticate DATA chunks
   202		 * we need to account for bundling of the AUTH chunks along with
   203		 * DATA.
   204		 */
   205		if (sctp_auth_send_cid(SCTP_CID_DATA, asoc)) {
   206			struct sctp_hmac *hmac_desc = sctp_auth_asoc_get_hmac(asoc);
   207	
   208			if (hmac_desc)
   209				max_data -= SCTP_PAD4(sizeof(struct sctp_auth_chunk) +
   210						      hmac_desc->hmac_len);
   211	
   212			if (sinfo->sinfo_tsn &&
   213			    sinfo->sinfo_ssn != asoc->active_key_id) {
   214				shkey = sctp_auth_get_shkey(asoc, sinfo->sinfo_ssn);
   215				if (!shkey) {
   216					err = -EINVAL;
   217					goto errout;
   218				}
   219			} else {
   220				shkey = asoc->shkey;
   221			}
   222		}
   223	
   224		/* Set first_len and then account for possible bundles on first frag */
   225		first_len = max_data;
   226	
   227		/* Check to see if we have a pending SACK and try to let it be bundled
   228		 * with this message.  Do this if we don't have any data queued already.
   229		 * To check that, look at out_qlen and retransmit list.
   230		 * NOTE: we will not reduce to account for SACK, if the message would
   231		 * not have been fragmented.
   232		 */
   233		if (timer_pending(&asoc->timers[SCTP_EVENT_TIMEOUT_SACK]) &&
   234		    asoc->outqueue.out_qlen == 0 &&
   235		    list_empty(&asoc->outqueue.retransmit) &&
   236		    msg_len > max_data)
   237			first_len -= SCTP_PAD4(sizeof(struct sctp_sack_chunk));
   238	
   239		/* Encourage Cookie-ECHO bundling. */
   240		if (asoc->state < SCTP_STATE_COOKIE_ECHOED)
   241			first_len -= SCTP_ARBITRARY_COOKIE_ECHO_LEN;
   242	
   243		/* Account for a different sized first fragment */
   244		if (msg_len >= first_len) {
   245			msg->can_delay = 0;
   246			if (msg_len > first_len)
   247				SCTP_INC_STATS(sock_net(asoc->base.sk),
   248					       SCTP_MIB_FRAGUSRMSGS);
   249		} else {
   250			/* Which may be the only one... */
   251			first_len = msg_len;
   252		}
   253	
   254		/* Create chunks for all DATA chunks. */
   255		for (remaining = msg_len; remaining; remaining -= len) {
   256			u8 frag = SCTP_DATA_MIDDLE_FRAG;
   257	
   258			if (remaining == msg_len) {
   259				/* First frag, which may also be the last */
   260				frag |= SCTP_DATA_FIRST_FRAG;
   261				len = first_len;
   262			} else {
   263				/* Middle frags */
   264				len = max_data;
   265			}
   266	
   267			if (len >= remaining) {
   268				/* Last frag, which may also be the first */
   269				len = remaining;
   270				frag |= SCTP_DATA_LAST_FRAG;
   271	
   272				/* The application requests to set the I-bit of the
   273				 * last DATA chunk of a user message when providing
   274				 * the user message to the SCTP implementation.
   275				 */
   276				if ((sinfo->sinfo_flags & SCTP_EOF) ||
   277				    (sinfo->sinfo_flags & SCTP_SACK_IMMEDIATELY))
   278					frag |= SCTP_DATA_SACK_IMM;
   279			}
   280	
   281			chunk = asoc->stream.si->make_datafrag(asoc, sinfo, len, frag,
   282							       GFP_KERNEL);
   283			if (!chunk) {
   284				err = -ENOMEM;
   285				goto errout;
   286			}
   287	
   288			err = sctp_user_addto_chunk(chunk, len, from);
   289			if (err < 0)
   290				goto errout_chunk_free;
   291	
   292			chunk->shkey = shkey;
   293	
   294			/* Put the chunk->skb back into the form expected by send.  */
   295			__skb_pull(chunk->skb, (__u8 *)chunk->chunk_hdr -
   296					       chunk->skb->data);
   297	
   298			sctp_datamsg_assign(msg, chunk);
   299			list_add_tail(&chunk->frag_list, &msg->chunks);
   300		}
   301	
   302		return msg;
   303	
   304	errout_chunk_free:
   305		sctp_chunk_free(chunk);
   306	
   307	errout:
   308		list_for_each_safe(pos, temp, &msg->chunks) {
   309			list_del_init(pos);
   310			chunk = list_entry(pos, struct sctp_chunk, frag_list);
   311			sctp_chunk_free(chunk);
   312		}
   313		sctp_datamsg_put(msg);
   314	
   315		return ERR_PTR(err);
   316	}
   317	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31788 bytes --]

Re: [PATCH net] sctp: frag_point sanity check

[David Miller]

From: Jakub Audykowicz <jakub.audykowicz@gmail.com>
Date: Tue,  4 Dec 2018 20:27:41 +0100

> If for some reason an association's fragmentation point is zero,
> sctp_datamsg_from_user will try to endlessly try to divide a message
> into zero-sized chunks. This eventually causes kernel panic due to
> running out of memory.
> 
> Although this situation is quite unlikely, it has occurred before as
> reported. I propose to add this simple last-ditch sanity check due to
> the severity of the potential consequences.
> 
> Signed-off-by: Jakub Audykowicz <jakub.audykowicz@gmail.com>

Applied.