ipv4: icmp: validate reply type before using icmp_pointers
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 ipv4: icmp: validate reply type before using icmp_pointers
From: Ruide Cao <[email protected]>
Extended echo replies use ICMP_EXT_ECHOREPLY as the outbound reply type. That value is outside the range covered by icmp_pointers[], which only describes the traditional ICMP types up to NR_ICMP_TYPES.
Avoid consulting icmp_pointers[] for reply types outside that range and keep the existing behavior for normal ICMP replies unchanged.
--- net/ipv4/icmp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv4/icmp.c +2 -1
@@ -373,7 +373,8 @@ static int icmp_glue_bits(void *from, char *to, int offset, int len, int odd, to, len); skb->csum =csum_block_add (skb->csum, csum, odd); -if (icmp_pointers[icmp_param->data.icmph.type].error) +if (icmp_param->data.icmph.type <=NR_ICMP_TYPES && + icmp_pointers[icmp_param->data.icmph.type].error)nf_ct_attach (skb, icmp_param->skb);return 0 ; }
Signature · 1 line
2.34.1
Quoted 27 lines · Ren Wei
From: Ruide Cao <[email protected]>
Extended echo replies use
ICMP_EXT_ECHOREPLYas the outbound reply type. That value is outside the range covered by icmp_pointers[], which only describes the traditional ICMP types up toNR_ICMP_TYPES.Avoid consulting icmp_pointers[] for reply types outside that range and keep the existing behavior for normal ICMP replies unchanged.
Fixes d329ea5bd884 ("icmp: add response to RFC 8335 PROBE messages")Reported-by Yuan Tan <[email protected]>Reported-by Yifan Wu <[email protected]>Reported-by Juefei Pu <[email protected]>Reported-by Xin Liu <[email protected]>Signed-off-by Ruide Cao <[email protected]>Signed-off-by Ren Wei <[email protected]>--- net/ipv4/icmp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)net/ipv4/icmp.c +0
-0
net/ipv4/icmp.c static int icmp_glue_bits(void *from, char *to, int offset, int len, int odd, · line 379
Quoted 8 lines
net/ipv4/icmp.c +2
-1@@ -373,7 +373,8 @@ static int icmp_glue_bits(void *from, char *to, int offset, int len, int odd, to, len); skb->csum =csum_block_add (skb->csum, csum, odd); -if (icmp_pointers[icmp_param->data.icmph.type].error) +if (icmp_param->data.icmph.type <=NR_ICMP_TYPES && + icmp_pointers[icmp_param->data.icmph.type].error)nf_ct_attach (skb, icmp_param->skb);return 0 ;
Pedantic mode: Perhaps also use array_index_nospec()
Quoted 9 lines · Ren Wei
From: Ruide Cao <[email protected]>
Extended echo replies use
ICMP_EXT_ECHOREPLYas the outbound reply type. That value is outside the range covered by icmp_pointers[], which only describes the traditional ICMP types up toNR_ICMP_TYPES.Avoid consulting icmp_pointers[] for reply types outside that range and keep the existing behavior for normal ICMP replies unchanged.
Quoted 8 lines
Fixes d329ea5bd884 ("icmp: add response to RFC 8335 PROBE messages")Reported-by Yuan Tan <[email protected]>Reported-by Yifan Wu <[email protected]>Reported-by Juefei Pu <[email protected]>Reported-by Xin Liu <[email protected]>Signed-off-by Ruide Cao <[email protected]>Signed-off-by Ren Wei <[email protected]>
Sashiko has generated a review of this patch. I do not believe the problems flagged there need to be addressed as the array indexes used in those cases have are always valid. However, I would appreciate it if you could look over this.
Simon Horman
reviewerQuoted 19 lines · Simon Horman
Quoted 9 lines · Ren Wei
From: Ruide Cao <[email protected]>
Extended echo replies use
ICMP_EXT_ECHOREPLYas the outbound reply type. That value is outside the range covered by icmp_pointers[], which only describes the traditional ICMP types up toNR_ICMP_TYPES.Avoid consulting icmp_pointers[] for reply types outside that range and keep the existing behavior for normal ICMP replies unchanged.
Quoted 8 lines
Fixes d329ea5bd884 ("icmp: add response to RFC 8335 PROBE messages")Reported-by Yuan Tan <[email protected]>Reported-by Yifan Wu <[email protected]>Reported-by Juefei Pu <[email protected]>Reported-by Xin Liu <[email protected]>Signed-off-by Ruide Cao <[email protected]>Signed-off-by Ren Wei <[email protected]>
Quoted 6 lines
Reviewed-by Simon Horman <[email protected]>Sashiko has generated a review of this patch. I do not believe the problems flagged there need to be addressed as the array indexes used in those cases have are always valid. However, I would appreciate it if you could look over this.
Sorry, I meant that to be a reply to v2.
I will respond there too.