[PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr

[PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr

[Jason Gunthorpe]

gcc 9 now does allocation size tracking and thinks that passing the member
of a union and then accessing beyond that member's bounds is an overflow.

Instead of using the union member, use the entire union with a cast to
get to the sockaddr. gcc will now know that the memory extends the full
size of the union.

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 drivers/infiniband/core/addr.c           | 16 ++++++++--------
 drivers/infiniband/hw/ocrdma/ocrdma_ah.c |  5 ++---
 drivers/infiniband/hw/ocrdma/ocrdma_hw.c |  5 ++---
 3 files changed, 12 insertions(+), 14 deletions(-)

I missed the ocrdma files in the v1

We can revisit what to do with that repetitive union after the merge
window, but this simple patch will eliminate the warnings for now.

Linus, I'll send this as a PR tomorrow - there is also a bug fix for
the rdma-netlink changes posted that should go too.

Thanks,
Jason

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index ba01b90c04e775..2f7d14159841f8 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -731,8 +731,8 @@ int roce_resolve_route_from_path(struct sa_path_rec *rec,
 	if (rec->roce.route_resolved)
 		return 0;
 
-	rdma_gid2ip(&sgid._sockaddr, &rec->sgid);
-	rdma_gid2ip(&dgid._sockaddr, &rec->dgid);
+	rdma_gid2ip((struct sockaddr *)&sgid, &rec->sgid);
+	rdma_gid2ip((struct sockaddr *)&dgid, &rec->dgid);
 
 	if (sgid._sockaddr.sa_family != dgid._sockaddr.sa_family)
 		return -EINVAL;
@@ -743,7 +743,7 @@ int roce_resolve_route_from_path(struct sa_path_rec *rec,
 	dev_addr.net = &init_net;
 	dev_addr.sgid_attr = attr;
 
-	ret = addr_resolve(&sgid._sockaddr, &dgid._sockaddr,
+	ret = addr_resolve((struct sockaddr *)&sgid, (struct sockaddr *)&dgid,
 			   &dev_addr, false, true, 0);
 	if (ret)
 		return ret;
@@ -815,22 +815,22 @@ int rdma_addr_find_l2_eth_by_grh(const union ib_gid *sgid,
 	struct rdma_dev_addr dev_addr;
 	struct resolve_cb_context ctx;
 	union {
-		struct sockaddr     _sockaddr;
 		struct sockaddr_in  _sockaddr_in;
 		struct sockaddr_in6 _sockaddr_in6;
 	} sgid_addr, dgid_addr;
 	int ret;
 
-	rdma_gid2ip(&sgid_addr._sockaddr, sgid);
-	rdma_gid2ip(&dgid_addr._sockaddr, dgid);
+	rdma_gid2ip((struct sockaddr *)&sgid_addr, sgid);
+	rdma_gid2ip((struct sockaddr *)&dgid_addr, dgid);
 
 	memset(&dev_addr, 0, sizeof(dev_addr));
 	dev_addr.net = &init_net;
 	dev_addr.sgid_attr = sgid_attr;
 
 	init_completion(&ctx.comp);
-	ret = rdma_resolve_ip(&sgid_addr._sockaddr, &dgid_addr._sockaddr,
-			      &dev_addr, 1000, resolve_cb, true, &ctx);
+	ret = rdma_resolve_ip((struct sockaddr *)&sgid_addr,
+			      (struct sockaddr *)&dgid_addr, &dev_addr, 1000,
+			      resolve_cb, true, &ctx);
 	if (ret)
 		return ret;
 
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
index 1d4ea135c28f2a..8d3e36d548aae9 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
@@ -83,7 +83,6 @@ static inline int set_av_attr(struct ocrdma_dev *dev, struct ocrdma_ah *ah,
 	struct iphdr ipv4;
 	const struct ib_global_route *ib_grh;
 	union {
-		struct sockaddr     _sockaddr;
 		struct sockaddr_in  _sockaddr_in;
 		struct sockaddr_in6 _sockaddr_in6;
 	} sgid_addr, dgid_addr;
@@ -133,9 +132,9 @@ static inline int set_av_attr(struct ocrdma_dev *dev, struct ocrdma_ah *ah,
 		ipv4.tot_len = htons(0);
 		ipv4.ttl = ib_grh->hop_limit;
 		ipv4.protocol = nxthdr;
-		rdma_gid2ip(&sgid_addr._sockaddr, sgid);
+		rdma_gid2ip((struct sockaddr *)&sgid_addr, sgid);
 		ipv4.saddr = sgid_addr._sockaddr_in.sin_addr.s_addr;
-		rdma_gid2ip(&dgid_addr._sockaddr, &ib_grh->dgid);
+		rdma_gid2ip((struct sockaddr*)&dgid_addr, &ib_grh->dgid);
 		ipv4.daddr = dgid_addr._sockaddr_in.sin_addr.s_addr;
 		memcpy((u8 *)ah->av + eth_sz, &ipv4, sizeof(struct iphdr));
 	} else {
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
index 32674b291f60da..5127e2ea4bdd2d 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
@@ -2499,7 +2499,6 @@ static int ocrdma_set_av_params(struct ocrdma_qp *qp,
 	u16 vlan_id = 0xFFFF;
 	u8 mac_addr[6], hdr_type;
 	union {
-		struct sockaddr     _sockaddr;
 		struct sockaddr_in  _sockaddr_in;
 		struct sockaddr_in6 _sockaddr_in6;
 	} sgid_addr, dgid_addr;
@@ -2542,8 +2541,8 @@ static int ocrdma_set_av_params(struct ocrdma_qp *qp,
 
 	hdr_type = rdma_gid_attr_network_type(sgid_attr);
 	if (hdr_type == RDMA_NETWORK_IPV4) {
-		rdma_gid2ip(&sgid_addr._sockaddr, &sgid_attr->gid);
-		rdma_gid2ip(&dgid_addr._sockaddr, &grh->dgid);
+		rdma_gid2ip((struct sockaddr *)&sgid_addr, &sgid_attr->gid);
+		rdma_gid2ip((struct sockaddr *)&dgid_addr, &grh->dgid);
 		memcpy(&cmd->params.dgid[0],
 		       &dgid_addr._sockaddr_in.sin_addr.s_addr, 4);
 		memcpy(&cmd->params.sgid[0],
-- 
2.21.0

Re: [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr

[Simon Horman]

On Mon, May 13, 2019 at 09:55:21PM -0300, Jason Gunthorpe wrote:
> gcc 9 now does allocation size tracking and thinks that passing the member
> of a union and then accessing beyond that member's bounds is an overflow.
> 
> Instead of using the union member, use the entire union with a cast to
> get to the sockaddr. gcc will now know that the memory extends the full
> size of the union.
> 
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> ---
>  drivers/infiniband/core/addr.c           | 16 ++++++++--------
>  drivers/infiniband/hw/ocrdma/ocrdma_ah.c |  5 ++---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c |  5 ++---
>  3 files changed, 12 insertions(+), 14 deletions(-)
> 
> I missed the ocrdma files in the v1
> 
> We can revisit what to do with that repetitive union after the merge
> window, but this simple patch will eliminate the warnings for now.
> 
> Linus, I'll send this as a PR tomorrow - there is also a bug fix for
> the rdma-netlink changes posted that should go too.

<2c>
I would be very happy to see this revisited in such a way
that some use is made of the C type system (instead of casts).
</2c>

Re: [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr

[Jason Gunthorpe]

On Thu, May 16, 2019 at 02:44:28PM +0200, Simon Horman wrote:
> On Mon, May 13, 2019 at 09:55:21PM -0300, Jason Gunthorpe wrote:
> > gcc 9 now does allocation size tracking and thinks that passing the member
> > of a union and then accessing beyond that member's bounds is an overflow.
> > 
> > Instead of using the union member, use the entire union with a cast to
> > get to the sockaddr. gcc will now know that the memory extends the full
> > size of the union.
> > 
> > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> >  drivers/infiniband/core/addr.c           | 16 ++++++++--------
> >  drivers/infiniband/hw/ocrdma/ocrdma_ah.c |  5 ++---
> >  drivers/infiniband/hw/ocrdma/ocrdma_hw.c |  5 ++---
> >  3 files changed, 12 insertions(+), 14 deletions(-)
> > 
> > I missed the ocrdma files in the v1
> > 
> > We can revisit what to do with that repetitive union after the merge
> > window, but this simple patch will eliminate the warnings for now.
> > 
> > Linus, I'll send this as a PR tomorrow - there is also a bug fix for
> > the rdma-netlink changes posted that should go too.
> 
> <2c>
> I would be very happy to see this revisited in such a way
> that some use is made of the C type system (instead of casts).
> </2c>

Well, I was thinking of swapping the union to sockaddr_storage ..

Do you propose to add a union to the kernel's sockaddr storage?

Jason

Re: [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr

[Simon Horman]

On Thu, May 16, 2019 at 12:21:48PM -0300, Jason Gunthorpe wrote:
> On Thu, May 16, 2019 at 02:44:28PM +0200, Simon Horman wrote:
> > On Mon, May 13, 2019 at 09:55:21PM -0300, Jason Gunthorpe wrote:
> > > gcc 9 now does allocation size tracking and thinks that passing the member
> > > of a union and then accessing beyond that member's bounds is an overflow.
> > > 
> > > Instead of using the union member, use the entire union with a cast to
> > > get to the sockaddr. gcc will now know that the memory extends the full
> > > size of the union.
> > > 
> > > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> > >  drivers/infiniband/core/addr.c           | 16 ++++++++--------
> > >  drivers/infiniband/hw/ocrdma/ocrdma_ah.c |  5 ++---
> > >  drivers/infiniband/hw/ocrdma/ocrdma_hw.c |  5 ++---
> > >  3 files changed, 12 insertions(+), 14 deletions(-)
> > > 
> > > I missed the ocrdma files in the v1
> > > 
> > > We can revisit what to do with that repetitive union after the merge
> > > window, but this simple patch will eliminate the warnings for now.
> > > 
> > > Linus, I'll send this as a PR tomorrow - there is also a bug fix for
> > > the rdma-netlink changes posted that should go too.
> > 
> > <2c>
> > I would be very happy to see this revisited in such a way
> > that some use is made of the C type system (instead of casts).
> > </2c>
> 
> Well, I was thinking of swapping the union to sockaddr_storage ..
> 
> Do you propose to add a union to the kernel's sockaddr storage?

I understand you have been down that rabbit hole before but,
yes, in an ideal world that would be my preference.