From eab59075d3cd7f3535aa2dbbc19a198dfee58892 Mon Sep 17 00:00:00 2001 From: Marcelo Ricardo Leitner Date: Wed, 28 Dec 2016 09:26:31 -0200 Subject: [PATCH 1/5] sctp: reduce indent level at sctp_sf_tabort_8_4_8 Signed-off-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- net/sctp/sm_statefuns.c | 52 ++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 8ec20a64a3f8..32587b1f84e7 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -3237,36 +3237,34 @@ static sctp_disposition_t sctp_sf_tabort_8_4_8(struct net *net, struct sctp_chunk *abort; packet = sctp_ootb_pkt_new(net, asoc, chunk); + if (!packet) + return SCTP_DISPOSITION_NOMEM; - if (packet) { - /* Make an ABORT. The T bit will be set if the asoc - * is NULL. - */ - abort = sctp_make_abort(asoc, chunk, 0); - if (!abort) { - sctp_ootb_pkt_free(packet); - return SCTP_DISPOSITION_NOMEM; - } - - /* Reflect vtag if T-Bit is set */ - if (sctp_test_T_bit(abort)) - packet->vtag = ntohl(chunk->sctp_hdr->vtag); - - /* Set the skb to the belonging sock for accounting. */ - abort->skb->sk = ep->base.sk; - - sctp_packet_append_chunk(packet, abort); - - sctp_add_cmd_sf(commands, SCTP_CMD_SEND_PKT, - SCTP_PACKET(packet)); - - SCTP_INC_STATS(net, SCTP_MIB_OUTCTRLCHUNKS); - - sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); - return SCTP_DISPOSITION_CONSUME; + /* Make an ABORT. The T bit will be set if the asoc + * is NULL. + */ + abort = sctp_make_abort(asoc, chunk, 0); + if (!abort) { + sctp_ootb_pkt_free(packet); + return SCTP_DISPOSITION_NOMEM; } - return SCTP_DISPOSITION_NOMEM; + /* Reflect vtag if T-Bit is set */ + if (sctp_test_T_bit(abort)) + packet->vtag = ntohl(chunk->sctp_hdr->vtag); + + /* Set the skb to the belonging sock for accounting. */ + abort->skb->sk = ep->base.sk; + + sctp_packet_append_chunk(packet, abort); + + sctp_add_cmd_sf(commands, SCTP_CMD_SEND_PKT, + SCTP_PACKET(packet)); + + SCTP_INC_STATS(net, SCTP_MIB_OUTCTRLCHUNKS); + + sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); + return SCTP_DISPOSITION_CONSUME; } /* From 1ff0156167dfb625c2e82aefded6b4cf245476ce Mon Sep 17 00:00:00 2001 From: Marcelo Ricardo Leitner Date: Wed, 28 Dec 2016 09:26:32 -0200 Subject: [PATCH 2/5] sctp: reduce indent level in sctp_sf_shut_8_4_5 Signed-off-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- net/sctp/sm_statefuns.c | 70 ++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 32587b1f84e7..a95915ef9dba 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -3501,45 +3501,43 @@ static sctp_disposition_t sctp_sf_shut_8_4_5(struct net *net, struct sctp_chunk *shut; packet = sctp_ootb_pkt_new(net, asoc, chunk); + if (!packet) + return SCTP_DISPOSITION_NOMEM; - if (packet) { - /* Make an SHUTDOWN_COMPLETE. - * The T bit will be set if the asoc is NULL. - */ - shut = sctp_make_shutdown_complete(asoc, chunk); - if (!shut) { - sctp_ootb_pkt_free(packet); - return SCTP_DISPOSITION_NOMEM; - } - - /* Reflect vtag if T-Bit is set */ - if (sctp_test_T_bit(shut)) - packet->vtag = ntohl(chunk->sctp_hdr->vtag); - - /* Set the skb to the belonging sock for accounting. */ - shut->skb->sk = ep->base.sk; - - sctp_packet_append_chunk(packet, shut); - - sctp_add_cmd_sf(commands, SCTP_CMD_SEND_PKT, - SCTP_PACKET(packet)); - - SCTP_INC_STATS(net, SCTP_MIB_OUTCTRLCHUNKS); - - /* If the chunk length is invalid, we don't want to process - * the reset of the packet. - */ - if (!sctp_chunk_length_valid(chunk, sizeof(sctp_chunkhdr_t))) - return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); - - /* We need to discard the rest of the packet to prevent - * potential bomming attacks from additional bundled chunks. - * This is documented in SCTP Threats ID. - */ - return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); + /* Make an SHUTDOWN_COMPLETE. + * The T bit will be set if the asoc is NULL. + */ + shut = sctp_make_shutdown_complete(asoc, chunk); + if (!shut) { + sctp_ootb_pkt_free(packet); + return SCTP_DISPOSITION_NOMEM; } - return SCTP_DISPOSITION_NOMEM; + /* Reflect vtag if T-Bit is set */ + if (sctp_test_T_bit(shut)) + packet->vtag = ntohl(chunk->sctp_hdr->vtag); + + /* Set the skb to the belonging sock for accounting. */ + shut->skb->sk = ep->base.sk; + + sctp_packet_append_chunk(packet, shut); + + sctp_add_cmd_sf(commands, SCTP_CMD_SEND_PKT, + SCTP_PACKET(packet)); + + SCTP_INC_STATS(net, SCTP_MIB_OUTCTRLCHUNKS); + + /* If the chunk length is invalid, we don't want to process + * the reset of the packet. + */ + if (!sctp_chunk_length_valid(chunk, sizeof(sctp_chunkhdr_t))) + return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); + + /* We need to discard the rest of the packet to prevent + * potential bomming attacks from additional bundled chunks. + * This is documented in SCTP Threats ID. + */ + return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); } /* From 0630c56e40b0bcca299d3b4c20ffcfddbe6a0218 Mon Sep 17 00:00:00 2001 From: Marcelo Ricardo Leitner Date: Wed, 28 Dec 2016 09:26:33 -0200 Subject: [PATCH 3/5] sctp: simplify addr copy Make it a bit easier to read. Signed-off-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- net/sctp/ipv6.c | 16 +++++++--------- net/sctp/protocol.c | 18 +++++++----------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c index 5ed8e79bf102..6619367bb6ca 100644 --- a/net/sctp/ipv6.c +++ b/net/sctp/ipv6.c @@ -412,22 +412,20 @@ static void sctp_v6_copy_addrlist(struct list_head *addrlist, static void sctp_v6_from_skb(union sctp_addr *addr, struct sk_buff *skb, int is_saddr) { - __be16 *port; - struct sctphdr *sh; + /* Always called on head skb, so this is safe */ + struct sctphdr *sh = sctp_hdr(skb); + struct sockaddr_in6 *sa = &addr->v6; - port = &addr->v6.sin6_port; addr->v6.sin6_family = AF_INET6; addr->v6.sin6_flowinfo = 0; /* FIXME */ addr->v6.sin6_scope_id = ((struct inet6_skb_parm *)skb->cb)->iif; - /* Always called on head skb, so this is safe */ - sh = sctp_hdr(skb); if (is_saddr) { - *port = sh->source; - addr->v6.sin6_addr = ipv6_hdr(skb)->saddr; + sa->sin6_port = sh->source; + sa->sin6_addr = ipv6_hdr(skb)->saddr; } else { - *port = sh->dest; - addr->v6.sin6_addr = ipv6_hdr(skb)->daddr; + sa->sin6_port = sh->dest; + sa->sin6_addr = ipv6_hdr(skb)->daddr; } } diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index 616a9428e0c4..f9c3c37c9ae0 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -237,23 +237,19 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp, static void sctp_v4_from_skb(union sctp_addr *addr, struct sk_buff *skb, int is_saddr) { - void *from; - __be16 *port; - struct sctphdr *sh; + /* Always called on head skb, so this is safe */ + struct sctphdr *sh = sctp_hdr(skb); + struct sockaddr_in *sa = &addr->v4; - port = &addr->v4.sin_port; addr->v4.sin_family = AF_INET; - /* Always called on head skb, so this is safe */ - sh = sctp_hdr(skb); if (is_saddr) { - *port = sh->source; - from = &ip_hdr(skb)->saddr; + sa->sin_port = sh->source; + sa->sin_addr.s_addr = ip_hdr(skb)->saddr; } else { - *port = sh->dest; - from = &ip_hdr(skb)->daddr; + sa->sin_port = sh->dest; + sa->sin_addr.s_addr = ip_hdr(skb)->daddr; } - memcpy(&addr->v4.sin_addr.s_addr, from, sizeof(struct in_addr)); } /* Initialize an sctp_addr from a socket. */ From 66b91d2cd0344c417194596ef6e387e52be69e57 Mon Sep 17 00:00:00 2001 From: Marcelo Ricardo Leitner Date: Wed, 28 Dec 2016 09:26:34 -0200 Subject: [PATCH 4/5] sctp: remove return value from sctp_packet_init/config There is no reason to use this cascading. It doesn't add anything. Let's remove it and simplify. Signed-off-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- include/net/sctp/structs.h | 7 +++---- net/sctp/output.c | 14 +++++--------- net/sctp/sm_statefuns.c | 5 +++-- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h index 92daabdc007d..87d56cc80a3c 100644 --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h @@ -722,10 +722,9 @@ struct sctp_packet { ipfragok:1; /* So let ip fragment this packet */ }; -struct sctp_packet *sctp_packet_init(struct sctp_packet *, - struct sctp_transport *, - __u16 sport, __u16 dport); -struct sctp_packet *sctp_packet_config(struct sctp_packet *, __u32 vtag, int); +void sctp_packet_init(struct sctp_packet *, struct sctp_transport *, + __u16 sport, __u16 dport); +void sctp_packet_config(struct sctp_packet *, __u32 vtag, int); sctp_xmit_t sctp_packet_transmit_chunk(struct sctp_packet *, struct sctp_chunk *, int, gfp_t); sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *, diff --git a/net/sctp/output.c b/net/sctp/output.c index f5320a87341e..07ab5062e541 100644 --- a/net/sctp/output.c +++ b/net/sctp/output.c @@ -81,8 +81,8 @@ static void sctp_packet_reset(struct sctp_packet *packet) /* Config a packet. * This appears to be a followup set of initializations. */ -struct sctp_packet *sctp_packet_config(struct sctp_packet *packet, - __u32 vtag, int ecn_capable) +void sctp_packet_config(struct sctp_packet *packet, __u32 vtag, + int ecn_capable) { struct sctp_transport *tp = packet->transport; struct sctp_association *asoc = tp->asoc; @@ -123,14 +123,12 @@ struct sctp_packet *sctp_packet_config(struct sctp_packet *packet, if (chunk) sctp_packet_append_chunk(packet, chunk); } - - return packet; } /* Initialize the packet structure. */ -struct sctp_packet *sctp_packet_init(struct sctp_packet *packet, - struct sctp_transport *transport, - __u16 sport, __u16 dport) +void sctp_packet_init(struct sctp_packet *packet, + struct sctp_transport *transport, + __u16 sport, __u16 dport) { struct sctp_association *asoc = transport->asoc; size_t overhead; @@ -151,8 +149,6 @@ struct sctp_packet *sctp_packet_init(struct sctp_packet *packet, packet->overhead = overhead; sctp_packet_reset(packet); packet->vtag = 0; - - return packet; } /* Free a packet. */ diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index a95915ef9dba..9a223d5b2314 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -6032,8 +6032,9 @@ static struct sctp_packet *sctp_ootb_pkt_new(struct net *net, sctp_transport_route(transport, (union sctp_addr *)&chunk->dest, sctp_sk(net->sctp.ctl_sock)); - packet = sctp_packet_init(&transport->packet, transport, sport, dport); - packet = sctp_packet_config(packet, vtag, 0); + packet = &transport->packet; + sctp_packet_init(packet, transport, sport, dport); + sctp_packet_config(packet, vtag, 0); return packet; From 509e7a311f0423bce6d3e1705ba0a470ffe770dc Mon Sep 17 00:00:00 2001 From: Marcelo Ricardo Leitner Date: Wed, 28 Dec 2016 09:26:35 -0200 Subject: [PATCH 5/5] sctp: sctp_chunk_length_valid should return bool Signed-off-by: Marcelo Ricardo Leitner Signed-off-by: David S. Miller --- net/sctp/sm_statefuns.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 9a223d5b2314..3382ef254e7b 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -160,23 +160,22 @@ static sctp_disposition_t __sctp_sf_do_9_1_abort(struct net *net, /* Small helper function that checks if the chunk length * is of the appropriate length. The 'required_length' argument * is set to be the size of a specific chunk we are testing. - * Return Values: 1 = Valid length - * 0 = Invalid length + * Return Values: true = Valid length + * false = Invalid length * */ -static inline int -sctp_chunk_length_valid(struct sctp_chunk *chunk, - __u16 required_length) +static inline bool +sctp_chunk_length_valid(struct sctp_chunk *chunk, __u16 required_length) { __u16 chunk_length = ntohs(chunk->chunk_hdr->length); /* Previously already marked? */ if (unlikely(chunk->pdiscard)) - return 0; + return false; if (unlikely(chunk_length < required_length)) - return 0; + return false; - return 1; + return true; } /**********************************************************