sctp: check dst 'NULL' before use it in, sctp_packet_transmit
Subsystems (3)
PATCH 1/1 sctp: check dst 'NULL' before use it in, sctp_packet_transmit
skb_dst_set will use dst, if dst is NULL although is not a problem, then goto the no_route and free nskb, so do the skb_dst_set is pointless. so check dst before use it. Remove the unnecessary initialization as well.
--- net/sctp/output.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
net/sctp/output.c +2 -2
@@ -387,7 +387,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)int err =0 ;int padding;/* How much padding do we need? */ __u8 has_data =0 ; -struct dst_entry *dst = tp->dst; +struct dst_entry *dst;unsigned char *auth =NULL ;/* pointer to auth in skb data */ pr_debug ("%s: packet:%p\n" , __func__, packet);
@@ -420,9 +420,9 @@ int sctp_packet_transmit(struct sctp_packet *packet) } } dst =dst_clone (tp->dst); -skb_dst_set (nskb, dst);if (!dst) goto no_route; +skb_dst_set (nskb, dst);/* Build the SCTP header. */ sh = (struct sctphdr *)skb_push (nskb,sizeof (struct sctphdr ));
Signature · 1 line
1.7.12
Quoted 3 lines · Wang Weidong
skb_dst_setwill use dst, if dst is NULL although is not a problem, then goto theno_routeand free nskb, so do theskb_dst_setis pointless. so check dst before use it. Remove the unnecessary initialization as well.
Please also cc linux-sctp as you did before!
Just went through the code, only reading from your subject title it first sounded like a NULL pointer dereference, but in fact the code is fine and nothing is wrong with it. I'd suggest you should make the subject sound more "harmless" to not confuse people, imho, since all you do here is some cleanup and rearrangement. "Use" sounds to me as dereferencing dst that is NULL.
Quoted 19 lines
Signed-off-by Wang Weidong <[email protected]>--- net/sctp/output.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)net/sctp/output.c +1
-1@@ -387,7 +387,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)int err =0 ;int padding;/* How much padding do we need? */ __u8 has_data =0 ; -struct dst_entry *dst = tp->dst; +struct dst_entry *dst;unsigned char *auth =NULL ;/* pointer to auth in skb data */ pr_debug ("%s: packet:%p\n" , __func__, packet);
net/sctp/output.c int sctp_packet_transmit(struct sctp_packet *packet) · line 424
Quoted 6 lines
net/sctp/output.c +0
-1@@ -420,9 +420,9 @@ int sctp_packet_transmit(struct sctp_packet *packet) } } dst =dst_clone (tp->dst); -skb_dst_set (nskb, dst);if (!dst) goto no_route;
Nit: you should set a newline here.
Quoted 5 lines
net/sctp/output.c +1
-0+skb_dst_set (nskb, dst);/* Build the SCTP header. */ sh = (struct sctphdr *)skb_push (nskb,sizeof (struct sctphdr ));
Quoted 3 lines · Wang Weidong
skb_dst_setwill use dst, if dst is NULL although is not a problem, then goto theno_routeand free nskb, so do theskb_dst_setis pointless. so check dst before use it. Remove the unnecessary initialization as well.
Please also cc linux-sctp as you did before!
Just went through the code, only reading from your subject title it first sounded like a NULL pointer dereference, but in fact the code is fine and nothing is wrong with it. I'd suggest you should make the subject sound more "harmless" to not confuse people, imho, since all you do here is some cleanup and rearrangement. "Use" sounds to me as dereferencing dst that is NULL.
Quoted 19 lines
Signed-off-by Wang Weidong <[email protected]>--- net/sctp/output.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)net/sctp/output.c +1
-1@@ -387,7 +387,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)int err =0 ;int padding;/* How much padding do we need? */ __u8 has_data =0 ; -struct dst_entry *dst = tp->dst; +struct dst_entry *dst;unsigned char *auth =NULL ;/* pointer to auth in skb data */ pr_debug ("%s: packet:%p\n" , __func__, packet);
net/sctp/output.c int sctp_packet_transmit(struct sctp_packet *packet) · line 424
Quoted 6 lines
net/sctp/output.c +0
-1@@ -420,9 +420,9 @@ int sctp_packet_transmit(struct sctp_packet *packet) } } dst =dst_clone (tp->dst); -skb_dst_set (nskb, dst);if (!dst) goto no_route;
Nit: you should set a newline here.
Quoted 5 lines
net/sctp/output.c +1
-0+skb_dst_set (nskb, dst);/* Build the SCTP header. */ sh = (struct sctphdr *)skb_push (nskb,sizeof (struct sctphdr ));
From: Wang Weidong <[email protected]>
Quoted 5 lines · Daniel Borkmann
Quoted 3 lines · Wang Weidong
skb_dst_setwill use dst, if dst is NULL although is not a problem, then goto theno_routeand free nskb, so do theskb_dst_setis pointless. so check dst before use it. Remove the unnecessary initialization as well.
Quoted 9 lines
Please also cc linux-sctp as you did before!
Just went through the code, only reading from your subject title it first sounded like a NULL pointer dereference, but in fact the code is fine and nothing is wrong with it. I'd suggest you should make the subject sound more "harmless" to not confuse people, imho, since all you do here is some cleanup and rearrangement. "Use" sounds to me as dereferencing dst that is NULL.
Sorry for that. I will fix the subject title and cc to linux-sctp.
Regards,
Wang
Quoted 17 lines
Quoted 17 lines
Signed-off-by Wang Weidong <[email protected]>--- net/sctp/output.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)net/sctp/output.c +1
-1@@ -387,7 +387,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)int err =0 ;int padding;/* How much padding do we need? */ __u8 has_data =0 ; -struct dst_entry *dst = tp->dst; +struct dst_entry *dst;unsigned char *auth =NULL ;/* pointer to auth in skb data */
Quoted 11 lines
net/sctp/output.c int sctp_packet_transmit(struct sctp_packet *packet) · line 424
Quoted 8 lines
net/sctp/output.c +0
-1pr_debug ("%s: packet:%p\n" , __func__, packet);@@ -420,9 +420,9 @@ int sctp_packet_transmit(struct sctp_packet *packet) } } dst =dst_clone (tp->dst); -skb_dst_set (nskb, dst);if (!dst) goto no_route;Nit: you should set a newline here.
Nice. Thanks.
Quoted 9 lines
net/sctp/output.c int sctp_packet_transmit(struct sctp_packet *packet) · line 428
Quoted 5 lines
net/sctp/output.c +1
-0+skb_dst_set (nskb, dst);/* Build the SCTP header. */ sh = (struct sctphdr *)skb_push (nskb,sizeof (struct sctphdr ));--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 1/1 sctp: move skb_dst_set() a bit downwards in sctp_packet_transmit()
In reply to Wang Weidongskb_dst_set will use dst, if dst is NULL although is not a problem, then goto the 'no_route' and free nskb, so do the skb_dst_set is pointless. so move the skb_dst_set after dst check.
Remove the unnecessary initialization as well.
v2: fix the subject line because it would confuse people,
as pointed out by Daniel.
--- net/sctp/output.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
net/sctp/output.c +2 -2
@@ -387,7 +387,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)int err =0 ;int padding;/* How much padding do we need? */ __u8 has_data =0 ; -struct dst_entry *dst = tp->dst; +struct dst_entry *dst;unsigned char *auth =NULL ;/* pointer to auth in skb data */ pr_debug ("%s: packet:%p\n" , __func__, packet);
@@ -420,9 +420,9 @@ int sctp_packet_transmit(struct sctp_packet *packet) } } dst =dst_clone (tp->dst); -skb_dst_set (nskb, dst);if (!dst) goto no_route; +skb_dst_set (nskb, dst);/* Build the SCTP header. */ sh = (struct sctphdr *)skb_push (nskb,sizeof (struct sctphdr ));
Signature · 1 line
1.7.12
Re: [PATCH net-next v2] sctp: move skb_dst_set() a bit downwards in sctp_packet_transmit()
Quoted 9 lines · Wang Weidong
skb_dst_setwill use dst, if dst is NULL although is not a problem, then goto the 'no_route' and free nskb, so do theskb_dst_setis pointless. so move theskb_dst_setafter dst check.
Remove the unnecessary initialization as well.v2: fix the subject line because it would confuse people,
as pointed out by Daniel.Signed-off-by Wang Weidong <[email protected]>
Quoted 38 lines
--- net/sctp/output.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)net/sctp/output.c +2
-2@@ -387,7 +387,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)int err =0 ;int padding;/* How much padding do we need? */ __u8 has_data =0 ; -struct dst_entry *dst = tp->dst; +struct dst_entry *dst;unsigned char *auth =NULL ;/* pointer to auth in skb data */ pr_debug ("%s: packet:%p\n" , __func__, packet);@@ -420,9 +420,9 @@ int sctp_packet_transmit(struct sctp_packet *packet) } } dst =dst_clone (tp->dst); -skb_dst_set (nskb, dst);if (!dst) goto no_route; +skb_dst_set (nskb, dst);/* Build the SCTP header. */ sh = (struct sctphdr *)skb_push (nskb,sizeof (struct sctphdr ));Signature · 8 lines
1.7.12 -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html