[PATCH] vxlan: Wrong type passed to %pIS

[PATCH] vxlan: Wrong type passed to %pIS

[Rasmus Villemoes]

src_ip is a pointer to a union vxlan_addr, one member of which is a
struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
the value of src_ip itself. Since %pIS formally expects something of
type struct sockaddr*, let's pass a pointer to the appropriate union
member, though this of course doesn't change the generated code.

Fixes: e4c7ed415387 ("vxlan: add ipv6 support")
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/net/vxlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index a8c755dcab14..11defbb24183 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -991,7 +991,7 @@ static bool vxlan_snoop(struct net_device *dev,
 		if (net_ratelimit())
 			netdev_info(dev,
 				    "%pM migrated from %pIS to %pIS\n",
-				    src_mac, &rdst->remote_ip, &src_ip);
+				    src_mac, &rdst->remote_ip.sa, &src_ip->sa);
 
 		rdst->remote_ip = *src_ip;
 		f->updated = jiffies;
-- 
2.1.3

Re: [PATCH] vxlan: Wrong type passed to %pIS

[Joe Perches]

On Sat, 2015-02-07 at 03:17 +0100, Rasmus Villemoes wrote:
> src_ip is a pointer to a union vxlan_addr, one member of which is a
> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
> the value of src_ip itself. Since %pIS formally expects something of
> type struct sockaddr*, let's pass a pointer to the appropriate union
> member, though this of course doesn't change the generated code.

Hello Rasmus

Are you finding these mismatches by hand or
are you using some tool?

Re: [PATCH] vxlan: Wrong type passed to %pIS

[Rasmus Villemoes]

On Sat, Feb 07 2015, Joe Perches <joe@perches.com> wrote:

> On Sat, 2015-02-07 at 03:17 +0100, Rasmus Villemoes wrote:
>> src_ip is a pointer to a union vxlan_addr, one member of which is a
>> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
>> the value of src_ip itself. Since %pIS formally expects something of
>> type struct sockaddr*, let's pass a pointer to the appropriate union
>> member, though this of course doesn't change the generated code.
>
> Hello Rasmus
>
> Are you finding these mismatches by hand or
> are you using some tool?

I've extended smatch to do format checking (mostly for the %p
extensions; gcc already does all the other stuff just fine). I'll try
and see if I can get it on github sometime in the coming week.

Rasmus

Re: [PATCH] vxlan: Wrong type passed to %pIS

[Cong Wang]

On Fri, Feb 6, 2015 at 6:17 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> src_ip is a pointer to a union vxlan_addr, one member of which is a
> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
> the value of src_ip itself. Since %pIS formally expects something of
> type struct sockaddr*, let's pass a pointer to the appropriate union
> member, though this of course doesn't change the generated code.
>


It is a union, this doesn't harm.

> Fixes: e4c7ed415387 ("vxlan: add ipv6 support")
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  drivers/net/vxlan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index a8c755dcab14..11defbb24183 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -991,7 +991,7 @@ static bool vxlan_snoop(struct net_device *dev,
>                 if (net_ratelimit())
>                         netdev_info(dev,
>                                     "%pM migrated from %pIS to %pIS\n",
> -                                   src_mac, &rdst->remote_ip, &src_ip);
> +                                   src_mac, &rdst->remote_ip.sa, &src_ip->sa);
>
>                 rdst->remote_ip = *src_ip;
>                 f->updated = jiffies;

Since you are on it, there is another similar place in vxlan too.

Re: [PATCH] vxlan: Wrong type passed to %pIS

[Rasmus Villemoes]

On Sat, Feb 07 2015, Cong Wang <xiyou.wangcong@gmail.com> wrote:

> On Fri, Feb 6, 2015 at 6:17 PM, Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>> src_ip is a pointer to a union vxlan_addr, one member of which is a
>> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
>> the value of src_ip itself. Since %pIS formally expects something of
>> type struct sockaddr*, let's pass a pointer to the appropriate union
>> member, though this of course doesn't change the generated code.
>>
>
>
> It is a union, this doesn't harm.
>

Just to be clear: This fixes a real bug. The minimal fix had been

-                     src_mac, &rdst->remote_ip, &src_ip);
+                     src_mac, &rdst->remote_ip, src_ip);

but I through in the cosmetic improvements while the line needed
changing anyway.

> Since you are on it, there is another similar place in vxlan too.

... which is why I didn't change that other occurrence.

Rasmus

Re: [PATCH] vxlan: Wrong type passed to %pIS

[Cong Wang]

On Sat, Feb 7, 2015 at 4:34 AM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Sat, Feb 07 2015, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
>> On Fri, Feb 6, 2015 at 6:17 PM, Rasmus Villemoes
>> <linux@rasmusvillemoes.dk> wrote:
>>> src_ip is a pointer to a union vxlan_addr, one member of which is a
>>> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
>>> the value of src_ip itself. Since %pIS formally expects something of
>>> type struct sockaddr*, let's pass a pointer to the appropriate union
>>> member, though this of course doesn't change the generated code.
>>>
>>
>>
>> It is a union, this doesn't harm.
>>
>
> Just to be clear: This fixes a real bug. The minimal fix had been
>
> -                     src_mac, &rdst->remote_ip, &src_ip);
> +                     src_mac, &rdst->remote_ip, src_ip);
>
> but I through in the cosmetic improvements while the line needed
> changing anyway.
>

Ah, I misread the patch.

Acked-by: Cong Wang <xiyou.wangcong@gmail.com>

Thanks!

Re: [PATCH] vxlan: Wrong type passed to %pIS

[David Miller]

From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Date: Sat,  7 Feb 2015 03:17:31 +0100

> src_ip is a pointer to a union vxlan_addr, one member of which is a
> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
> the value of src_ip itself. Since %pIS formally expects something of
> type struct sockaddr*, let's pass a pointer to the appropriate union
> member, though this of course doesn't change the generated code.
> 
> Fixes: e4c7ed415387 ("vxlan: add ipv6 support")
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Applied, thanks.