sctp: a bunch of fixes for some sparse warnings
Subsystems (3)
Xin Long
maintainercover sctp: a bunch of fixes for some sparse warnings
As Eric noticed, when running 'make C=2 M=net/sctp/', a plenty of warnings or errors checked by sparse appear. They are all problems about Endian and type cast.
Most of them are just warnings by which no issues could be caused while some might be bugs.
This patchset fixes them with four patches basically according to how they are introduced.
Xin Long (4):
sctp: fix some type cast warnings introduced by stream reconf
sctp: fix some type cast warnings introduced by transport rhashtable
sctp: fix a type cast warnings that causes a_rwnd gets the wrong value
sctp: fix some type cast warnings introduced since very beginning
include/linux/sctp.h | 34 +++++++++++++++++----------------- include/net/sctp/sm.h | 2 +- include/net/sctp/ulpevent.h | 2 +- include/uapi/linux/sctp.h | 2 +- net/sctp/input.c | 22 +++++++++++----------- net/sctp/ipv6.c | 2 +- net/sctp/sm_make_chunk.c | 9 +++++---- net/sctp/sm_sideeffect.c | 8 ++++---- net/sctp/stream.c | 26 +++++++++++++++++--------- net/sctp/ulpevent.c | 2 +- 10 files changed, 59 insertions(+), 50 deletions(-)
Signature · 1 line
2.1.0
Xin Long
maintainerPATCH 1/4 sctp: fix some type cast warnings introduced by stream reconf
These warnings were found by running 'make C=2 M=net/sctp/'.
They are introduced by not aware of Endian when coding stream reconf patches.
Since commit c0d8bab6ae51 ("sctp: add get and set sockopt for reconf_enable") enabled stream reconf feature for users, the Fixes tag below would use it.
--- include/linux/sctp.h | 32 ++++++++++++++++---------------- include/net/sctp/sm.h | 2 +- include/net/sctp/ulpevent.h | 2 +- net/sctp/sm_make_chunk.c | 5 +++-- net/sctp/stream.c | 26 +++++++++++++++++--------- net/sctp/ulpevent.c | 2 +- 6 files changed, 39 insertions(+), 30 deletions(-)
include/linux/sctp.h +16 -16
@@ -716,28 +716,28 @@ struct sctp_reconf_chunk {struct sctp_strreset_outreq {struct sctp_paramhdr param_hdr; -__u32 request_seq; -__u32 response_seq; -__u32 send_reset_at_tsn; -__u16 list_of_streams[0 ]; +__be32 request_seq; +__be32 response_seq; +__be32 send_reset_at_tsn; +__be16 list_of_streams[0 ]; };struct sctp_strreset_inreq {struct sctp_paramhdr param_hdr; -__u32 request_seq; -__u16 list_of_streams[0 ]; +__be32 request_seq; +__be16 list_of_streams[0 ]; };struct sctp_strreset_tsnreq {struct sctp_paramhdr param_hdr; -__u32 request_seq; +__be32 request_seq; };struct sctp_strreset_addstrm {struct sctp_paramhdr param_hdr; -__u32 request_seq; -__u16 number_of_streams; -__u16 reserved; +__be32 request_seq; +__be16 number_of_streams; +__be16 reserved; };enum {
@@ -752,16 +752,16 @@ enum {struct sctp_strreset_resp {struct sctp_paramhdr param_hdr; -__u32 response_seq; -__u32 result; +__be32 response_seq; +__be32 result; };struct sctp_strreset_resptsn {struct sctp_paramhdr param_hdr; -__u32 response_seq; -__u32 result; -__u32 senders_next_tsn; -__u32 receivers_next_tsn; +__be32 response_seq; +__be32 result; +__be32 senders_next_tsn; +__be32 receivers_next_tsn; };#endif /* __LINUX_SCTP_H__ */
include/net/sctp/sm.h +1 -1
@@ -261,7 +261,7 @@ struct sctp_chunk *sctp_make_fwdtsn(const struct sctp_association *asoc, struct sctp_fwdtsn_skip *skiplist);struct sctp_chunk *sctp_make_auth (const struct sctp_association *asoc);struct sctp_chunk *sctp_make_strreset_req (const struct sctp_association *asoc, -__u16 stream_num,__u16 *stream_list, +__u16 stream_num,__be16 *stream_list,bool out,bool in);struct sctp_chunk *sctp_make_strreset_tsnreq (const struct sctp_association *asoc);
include/net/sctp/ulpevent.h +1 -1
@@ -130,7 +130,7 @@ struct sctp_ulpevent *sctp_ulpevent_make_sender_dry_event(struct sctp_ulpevent *sctp_ulpevent_make_stream_reset_event (const struct sctp_association *asoc,__u16 flags, -__u16 stream_num,__u16 *stream_list,gfp_t gfp); +__u16 stream_num,__be16 *stream_list,gfp_t gfp); struct sctp_ulpevent *sctp_ulpevent_make_assoc_reset_event( const struct sctp_association *asoc, __u16 flags,
net/sctp/sm_make_chunk.c +3 -2
@@ -3591,7 +3591,7 @@ static struct sctp_chunk *sctp_make_reconf(const struct sctp_association *asoc,*/ struct sctp_chunk *sctp_make_strreset_req (const struct sctp_association *asoc, -__u16 stream_num,__u16 *stream_list, +__u16 stream_num,__be16 *stream_list,bool out,bool in) {struct sctp_strreset_outreq outreq;
@@ -3788,7 +3788,8 @@ bool sctp_verify_reconf(const struct sctp_association *asoc, {struct sctp_reconf_chunk *hdr;union sctp_params param; -__u16 last =0 , cnt =0 ; +__be16 last =0 ; +__u16 cnt =0 ; hdr = (struct sctp_reconf_chunk *)chunk->chunk_hdr; sctp_walk_params(param, hdr, params) {
net/sctp/stream.c +17 -9
@@ -118,6 +118,7 @@ int sctp_send_reset_streams(struct sctp_association *asoc,__u16 i, str_nums, *str_list;struct sctp_chunk *chunk;int retval = -EINVAL ; +__be16 *nstr_list;bool out, in; if (!asoc->peer.reconf_capable ||
@@ -148,13 +149,18 @@ int sctp_send_reset_streams(struct sctp_association *asoc,if (str_list[i] >= stream->incnt) goto out; + nstr_list =kcalloc (str_nums,sizeof (__be16),GFP_KERNEL ); +if (!nstr_list) { + retval = -ENOMEM ; + goto out; + } +for (i =0 ; i < str_nums; i++) - str_list[i] =htons (str_list[i]); + nstr_list[i] =htons (str_list[i]); - chunk =sctp_make_strreset_req (asoc, str_nums, str_list, out, in); + chunk =sctp_make_strreset_req (asoc, str_nums, nstr_list, out, in); -for (i =0 ; i < str_nums; i++) - str_list[i] =ntohs (str_list[i]); +kfree (nstr_list);if (!chunk) { retval = -ENOMEM ;
@@ -305,7 +311,7 @@ int sctp_send_add_streams(struct sctp_association *asoc, }static struct sctp_paramhdr *sctp_chunk_lookup_strreset_param ( -struct sctp_association *asoc,__u32 resp_seq, +struct sctp_association *asoc,__be32 resp_seq,__be16 type) {struct sctp_chunk *chunk = asoc->strreset_chunk;
@@ -345,8 +351,9 @@ struct sctp_chunk *sctp_process_strreset_outreq( {struct sctp_strreset_outreq *outreq = param.v;struct sctp_stream *stream = &asoc->stream; -__u16 i, nums, flags =0 , *str_p =NULL ;__u32 result =SCTP_STRRESET_DENIED ; +__u16 i, nums, flags =0 ; +__be16 *str_p =NULL ;__u32 request_seq; request_seq =ntohl (outreq->request_seq);
@@ -439,8 +446,9 @@ struct sctp_chunk *sctp_process_strreset_inreq(struct sctp_stream *stream = &asoc->stream;__u32 result =SCTP_STRRESET_DENIED ;struct sctp_chunk *chunk =NULL ; -__u16 i, nums, *str_p;__u32 request_seq; +__u16 i, nums; +__be16 *str_p; request_seq =ntohl (inreq->request_seq); if (TSN_lt(asoc->strreset_inseq, request_seq) ||
@@ -769,7 +777,7 @@ struct sctp_chunk *sctp_process_strreset_resp(if (req->type =SCTP_PARAM_RESET_OUT_REQUEST ) {struct sctp_strreset_outreq *outreq; -__u16 *str_p; +__be16 *str_p; outreq = (struct sctp_strreset_outreq *)req; str_p = outreq->list_of_streams;
@@ -794,7 +802,7 @@ struct sctp_chunk *sctp_process_strreset_resp( nums, str_p, GFP_ATOMIC); }else if (req->type =SCTP_PARAM_RESET_IN_REQUEST ) {struct sctp_strreset_inreq *inreq; -__u16 *str_p; +__be16 *str_p;/* if the result is performed, it's impossible for inreq */ if (result =SCTP_STRRESET_PERFORMED )
net/sctp/ulpevent.c +1 -1
@@ -847,7 +847,7 @@ struct sctp_ulpevent *sctp_ulpevent_make_sender_dry_event(struct sctp_ulpevent *sctp_ulpevent_make_stream_reset_event (const struct sctp_association *asoc,__u16 flags,__u16 stream_num, -__u16 *stream_list,gfp_t gfp) +__be16 *stream_list,gfp_t gfp) {struct sctp_stream_reset_event *sreset;struct sctp_ulpevent *event;
Signature · 1 line
2.1.0
Xin Long
maintainerPATCH 2/4 sctp: fix some type cast warnings introduced by transport rhashtable
These warnings were found by running 'make C=2 M=net/sctp/'.
They are introduced by not aware of Endian for the port when coding transport rhashtable patches.
--- net/sctp/input.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
net/sctp/input.c +11 -11
@@ -794,7 +794,7 @@ static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(struct net *net,struct sctp_hash_cmp_arg {const union sctp_addr *paddr;const struct net *net; -u16 lport; +__be16 lport; }; static inline int sctp_hash_cmp(struct rhashtable_compare_arg *arg,
@@ -820,37 +820,37 @@ static inline int sctp_hash_cmp(struct rhashtable_compare_arg *arg,return err; } -static inline u32 sctp_hash_obj (const void *data,u32 len,u32 seed) +static inline __u32 sctp_hash_obj (const void *data,u32 len,u32 seed) {const struct sctp_transport *t = data;const union sctp_addr *paddr = &t->ipaddr;const struct net *net =sock_net (t->asoc->base.sk); -u16 lport =htons (t->asoc->base.bind_addr.port); -u32 addr; +__be16 lport =htons (t->asoc->base.bind_addr.port); +__u32 addr;if (paddr->sa.sa_family =AF_INET6 ) addr =jhash (&paddr->v6.sin6_addr,16 , seed);else - addr = paddr->v4.sin_addr.s_addr; + addr = (__force __u32 )paddr->v4.sin_addr.s_addr; -return jhash_3words (addr, ((__u32 )paddr->v4.sin_port) <<16 | +return jhash_3words (addr, ((__force __u32 )paddr->v4.sin_port) <<16 | (__force __u32 )lport,net_hash_mix (net), seed); } -static inline u32 sctp_hash_key (const void *data,u32 len,u32 seed) +static inline __u32 sctp_hash_key (const void *data,u32 len,u32 seed) {const struct sctp_hash_cmp_arg *x = data;const union sctp_addr *paddr = x->paddr;const struct net *net = x->net; -u16 lport = x->lport; -u32 addr; +__be16 lport = x->lport; +__u32 addr;if (paddr->sa.sa_family =AF_INET6 ) addr =jhash (&paddr->v6.sin6_addr,16 , seed);else - addr = paddr->v4.sin_addr.s_addr; + addr = (__force __u32 )paddr->v4.sin_addr.s_addr; -return jhash_3words (addr, ((__u32 )paddr->v4.sin_port) <<16 | +return jhash_3words (addr, ((__force __u32 )paddr->v4.sin_port) <<16 | (__force __u32 )lport,net_hash_mix (net), seed); }
Signature · 1 line
2.1.0
Xin Long
maintainerPATCH 3/4 sctp: fix a type cast warnings that causes a_rwnd gets the wrong value
These warnings were found by running 'make C=2 M=net/sctp/'.
Commit d4d6fb5787a6 ("sctp: Try not to change a_rwnd when faking a SACK from SHUTDOWN.") expected to use the peers old rwnd and add our flight size to the a_rwnd. But with the wrong Endian, it may not work as well as expected.
So fix it by converting to the right value.
--- net/sctp/sm_sideeffect.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
net/sctp/sm_sideeffect.c +2 -2
@@ -1680,8 +1680,8 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,case SCTP_CMD_PROCESS_CTSN :/* Dummy up a SACK for processing. */ sackh.cum_tsn_ack = cmd->obj.be32; - sackh.a_rwnd = asoc->peer.rwnd + - asoc->outqueue.outstanding_bytes; + sackh.a_rwnd =htonl (asoc->peer.rwnd + + asoc->outqueue.outstanding_bytes); sackh.num_gap_ack_blocks =0 ; sackh.num_dup_tsns =0 ; chunk->subh.sack_hdr = &sackh;
Signature · 1 line
2.1.0
Xin Long
maintainerPATCH 4/4 sctp: fix some type cast warnings introduced since very beginning
These warnings were found by running 'make C=2 M=net/sctp/'. They are there since very beginning.
Note after this patch, there still one warning left in sctp_outq_flush():
sctp_chunk_fail(chunk, SCTP_ERROR_INV_STRM)
Since it has been moved to sctp_stream_outq_migrate on net-next, to avoid the extra job when merging net-next to net, I will post the fix for it after the merging is done.
--- include/linux/sctp.h | 2 +- include/uapi/linux/sctp.h | 2 +- net/sctp/ipv6.c | 2 +- net/sctp/sm_make_chunk.c | 4 ++-- net/sctp/sm_sideeffect.c | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-)
include/linux/sctp.h +1 -1
@@ -231,7 +231,7 @@ struct sctp_datahdr {__be32 tsn;__be16 stream;__be16 ssn; -__be32 ppid; +__u32 ppid;__u8 payload[0 ]; };
include/uapi/linux/sctp.h +1 -1
@@ -376,7 +376,7 @@ struct sctp_remote_error {__u16 sre_type;__u16 sre_flags;__u32 sre_length; -__u16 sre_error; +__be16 sre_error;sctp_assoc_t sre_assoc_id;__u8 sre_data[0 ]; };
net/sctp/ipv6.c +1 -1
@@ -738,7 +738,7 @@ static int sctp_v6_skb_iif(const struct sk_buff *skb)/* Was this packet marked by Explicit Congestion Notification? */ static int sctp_v6_is_ce (const struct sk_buff *skb) { -return *((__u32 *)(ipv6_hdr (skb))) &htonl (1 <<20 ); +return *((__u32 *)(ipv6_hdr (skb))) & (__force __u32 )htonl (1 <<20 ); }/* Dump the v6 addr to the seq file. */
net/sctp/sm_make_chunk.c +2 -2
@@ -2854,7 +2854,7 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *asoc, addr_param_len = af->to_addr_param (addr, &addr_param); param.param_hdr.type = flags; param.param_hdr.length =htons (paramlen + addr_param_len); - param.crr_id = i; + param.crr_id =htonl (i);sctp_addto_chunk (retval, paramlen, ¶m);sctp_addto_chunk (retval, addr_param_len, &addr_param);
@@ -2867,7 +2867,7 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *asoc, addr_param_len = af->to_addr_param (addr, &addr_param); param.param_hdr.type =SCTP_PARAM_DEL_IP ; param.param_hdr.length =htons (paramlen + addr_param_len); - param.crr_id = i; + param.crr_id =htonl (i);sctp_addto_chunk (retval, paramlen, ¶m);sctp_addto_chunk (retval, addr_param_len, &addr_param);
net/sctp/sm_sideeffect.c +2 -2
@@ -1607,12 +1607,12 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,break ;case SCTP_CMD_INIT_FAILED : -sctp_cmd_init_failed (commands, asoc, cmd->obj.err); +sctp_cmd_init_failed (commands, asoc, cmd->obj.u32);break ;case SCTP_CMD_ASSOC_FAILED :sctp_cmd_assoc_failed (commands, asoc, event_type, - subtype, chunk, cmd->obj.err); + subtype, chunk, cmd->obj.u32);break ;case SCTP_CMD_INIT_COUNTER_INC :
Signature · 1 line
2.1.0
RE: [PATCH net 4/4] sctp: fix some type cast warnings introduced since very beginning
From: Xin Long
Quoted 3 lines
Sent: 28 October 2017 12:44
These warnings were found by running 'make C=2 M=net/sctp/'. They are there since very beginning.
...
Review
Quoted 2 lines
(quoted diff) +1
-1- param.crr_id = i; + param.crr_id = htonl(i);
...
Isn't this a bug fix, not just removing a warning??
David
Xin Long
maintainerRe: [PATCH net 4/4] sctp: fix some type cast warnings introduced since very beginning
Quoted 10 lines · David Laight
From: Xin Long
Quoted 3 lines
Sent: 28 October 2017 12:44
These warnings were found by running 'make C=2 M=net/sctp/'. They are there since very beginning....
Review
Quoted 2 lines
(quoted diff) +1
-1- param.crr_id = i; + param.crr_id = htonl(i);...
Isn't this a bug fix, not just removing a warning??
It's actually fiine,crr_id is just an identifier, and receiver will only pack the same value to the asconf_ack chunk, and then sender checks with:
if (asconf_ack_param->crr_id = asconf_param->crr_id)
...
So no issue could be caused by this, I didn't count it as a bug.
David Miller
maintainerFrom: Xin Long <[email protected]>
Date: Sat, 28 Oct 2017 19:43:53 +0800
Quoted 9 lines
As Eric noticed, when running 'make C=2 M=net/sctp/', a plenty of warnings or errors checked by sparse appear. They are all problems about Endian and type cast.
Most of them are just warnings by which no issues could be caused while some might be bugs.
This patchset fixes them with four patches basically according to how they are introduced.
Series applied, thanks.