diff mbox

[1/7] IB/rxe: Allocate enough space for an IPv6 addr

Message ID 975a89bc-033e-14ab-72f2-4244c0205e59@mellanox.com (mailing list archive)
State Superseded
Headers show

Commit Message

yonatan cohen Nov. 22, 2016, 3:21 p.m. UTC 
On 11/18/2016 4:36 PM, Andrew Boyer wrote:
> Avoid smashing the stack when an ICRC error occurs on an IPv6 network.
>
> Signed-off-by: Andrew Boyer <andrew.boyer@dell.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_recv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
> index 46f0628..b40ab8d 100644
> --- a/drivers/infiniband/sw/rxe/rxe_recv.c
> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> @@ -391,7 +391,7 @@ int rxe_rcv(struct sk_buff *skb)
>  			     payload_size(pkt));
>  	calc_icrc = cpu_to_be32(~calc_icrc);
>  	if (unlikely(calc_icrc != pack_icrc)) {
> -		char saddr[sizeof(struct in6_addr)];
> +		char saddr[64];
>
>  		if (skb->protocol == htons(ETH_P_IPV6))
>  			sprintf(saddr, "%pI6", &ipv6_hdr(skb)->saddr);
>
you fixed a bug here but i think the following would be better
than hard coding 64 bytes on the stack

         }

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Bart Van Assche Nov. 22, 2016, 11:28 p.m. UTC | #1 
On 11/22/2016 07:21 AM, Yonatan Cohen wrote:
> On 11/18/2016 4:36 PM, Andrew Boyer wrote:
>> Avoid smashing the stack when an ICRC error occurs on an IPv6 network.
>>
>> Signed-off-by: Andrew Boyer <andrew.boyer@dell.com>
>> ---
>>  drivers/infiniband/sw/rxe/rxe_recv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c
>> b/drivers/infiniband/sw/rxe/rxe_recv.c
>> index 46f0628..b40ab8d 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_recv.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
>> @@ -391,7 +391,7 @@ int rxe_rcv(struct sk_buff *skb)
>>                   payload_size(pkt));
>>      calc_icrc = cpu_to_be32(~calc_icrc);
>>      if (unlikely(calc_icrc != pack_icrc)) {
>> -        char saddr[sizeof(struct in6_addr)];
>> +        char saddr[64];
>>
>>          if (skb->protocol == htons(ETH_P_IPV6))
>>              sprintf(saddr, "%pI6", &ipv6_hdr(skb)->saddr);
>>
> you fixed a bug here but i think the following would be better
> than hard coding 64 bytes on the stack
>
> --- a/drivers/infiniband/sw/rxe/rxe_recv.c
> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> @@ -391,16 +391,14 @@ int rxe_rcv(struct sk_buff *skb)
>                              payload_size(pkt));
>         calc_icrc = cpu_to_be32(~calc_icrc);
>         if (unlikely(calc_icrc != pack_icrc)) {
> -               char saddr[sizeof(struct in6_addr)];
>
>                 if (skb->protocol == htons(ETH_P_IPV6))
> -                       sprintf(saddr, "%pI6", &ipv6_hdr(skb)->saddr);
> +                       pr_warn_ratelimited("bad ICRC from %pI6\n",
> &ipv6_hdr(skb)->saddr);
>                 else if (skb->protocol == htons(ETH_P_IP))
> -                       sprintf(saddr, "%pI4", &ip_hdr(skb)->saddr);
> +                       pr_warn_ratelimited("bad ICRC from %pI4\n",
> &ip_hdr(skb)->saddr);
>                 else
> -                       sprintf(saddr, "unknown");
> +                       pr_warn_ratelimited("bad ICRC from unknown\n");
>
> -               pr_warn_ratelimited("bad ICRC from %s\n", saddr);
>                 goto drop;
>         }

Hello Yonatan,

Apparently our e-mails crossed. Anyway, have you considered to use %pIS 
instead of %pI4 / %pI6? See also 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=1067964305df.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- a/drivers/infiniband/sw/rxe/rxe_recv.c
+++ b/drivers/infiniband/sw/rxe/rxe_recv.c
@@ -391,16 +391,14 @@  int rxe_rcv(struct sk_buff *skb)
                              payload_size(pkt));
         calc_icrc = cpu_to_be32(~calc_icrc);
         if (unlikely(calc_icrc != pack_icrc)) {
-               char saddr[sizeof(struct in6_addr)];

                 if (skb->protocol == htons(ETH_P_IPV6))
-                       sprintf(saddr, "%pI6", &ipv6_hdr(skb)->saddr);
+                       pr_warn_ratelimited("bad ICRC from %pI6\n", 
&ipv6_hdr(skb)->saddr);
                 else if (skb->protocol == htons(ETH_P_IP))
-                       sprintf(saddr, "%pI4", &ip_hdr(skb)->saddr);
+                       pr_warn_ratelimited("bad ICRC from %pI4\n", 
&ip_hdr(skb)->saddr);
                 else
-                       sprintf(saddr, "unknown");
+                       pr_warn_ratelimited("bad ICRC from unknown\n");

-               pr_warn_ratelimited("bad ICRC from %s\n", saddr);
                 goto drop;