LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr
@ 2019-05-14  0:55 Jason Gunthorpe
  2019-05-16 12:44 ` Simon Horman
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Gunthorpe @ 2019-05-14  0:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: David Miller, Doug Ledford, linux-rdma,
	Linux List Kernel Mailing, Netdev

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr
  2019-05-14  0:55 [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr Jason Gunthorpe
@ 2019-05-16 12:44 ` Simon Horman
  2019-05-16 15:21   ` Jason Gunthorpe
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Horman @ 2019-05-16 12:44 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Linus Torvalds, David Miller, Doug Ledford, linux-rdma,
	Linux List Kernel Mailing, Netdev

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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr
  2019-05-16 12:44 ` Simon Horman
@ 2019-05-16 15:21   ` Jason Gunthorpe
  2019-05-17 11:32     ` Simon Horman
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Gunthorpe @ 2019-05-16 15:21 UTC (permalink / raw)
  To: Simon Horman
  Cc: Linus Torvalds, David Miller, Doug Ledford, linux-rdma,
	Linux List Kernel Mailing, Netdev

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr
  2019-05-16 15:21   ` Jason Gunthorpe
@ 2019-05-17 11:32     ` Simon Horman
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2019-05-17 11:32 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Linus Torvalds, David Miller, Doug Ledford, linux-rdma,
	Linux List Kernel Mailing, Netdev

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-05-17 11:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-14  0:55 [PATCH v2] RDMA: Directly cast the sockaddr union to sockaddr Jason Gunthorpe
2019-05-16 12:44 ` Simon Horman
2019-05-16 15:21   ` Jason Gunthorpe
2019-05-17 11:32     ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).