On 25.03.21 09:13, Kurt Van Dijck wrote:

On Wed, 24 Mar 2021 22:54:42 +0100, Oliver Hartkopp wrote:

quoted

Since commit f5223e9eee65 ("can: extend sockaddr_can to include j1939
members") the sockaddr_can has been extended in size and a new
CAN_REQUIRED_SIZE macro has been introduced to calculate the protocol
specific needed size.

The ABI for the msg_name and msg_namelen has not been adapted to the
new CAN_REQUIRED_SIZE macro which leads to a problem when an existing
binary reads the (increased) struct sockaddr_can in msg_name.

Fixes: f5223e9eee65 ("can: extend sockaddr_can to include j1939 members")
Link: https://lore.kernel.org/linux-can/1135648123.112255.1616613706554.JavaMail.zimbra@nod.at/T/#t (local)
Reported-by: Richard Weinberger <richard@nod.at>
Suggested-by: Kurt Van Dijck <redacted>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
  net/can/bcm.c   | 14 ++++++++++----
  net/can/isotp.c | 14 ++++++++++----
  net/can/raw.c   | 17 +++++++++++------
  3 files changed, 31 insertions(+), 14 deletions(-)

...

quoted

@@ -808,10 +810,13 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
  	int noblock;
  
  	noblock = flags & MSG_DONTWAIT;
  	flags &= ~MSG_DONTWAIT;
  
+	if (msg->msg_name && msg->msg_namelen < RAW_MIN_NAMELEN)
+		return -EINVAL;
+
  	if (flags & MSG_ERRQUEUE)
  		return sock_recv_errqueue(sk, msg, size,
  					  SOL_CAN_RAW, SCM_CAN_RAW_ERRQUEUE);
  
  	skb = skb_recv_datagram(sk, flags, noblock, &err);

@@ -830,12 +835,12 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
  	}
  
  	sock_recv_ts_and_drops(msg, sk, skb);
  
  	if (msg->msg_name) {
-		__sockaddr_check_size(sizeof(struct sockaddr_can));
-		msg->msg_namelen = sizeof(struct sockaddr_can);
+		__sockaddr_check_size(RAW_MIN_NAMELEN);
+		msg->msg_namelen = RAW_MIN_NAMELEN;

Why not fill up to MIN(msg->msg_namelen, sizeof(struct sockaddr_can))?

I checked that in j1939/socket.c and there the content is also reduced 
to the minimum size.

At least that makes sense to me to have that consistent.

Regards,
Oliver

quoted

  		memcpy(msg->msg_name, skb->cb, msg->msg_namelen);
  	}
  
  	/* assign the flags that have been recorded in raw_rcv() */
  	msg->msg_flags |= *(raw_flags(skb));
-- 
2.30.2