Re: [PATCH net-next 2/2] af_iucv: Move sockaddr length checks to before accessing sa_family in bind and connect handlers
flat view
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: 2017-06-23 17:36:17
Also in:
linux-s390
On 06/23/2017 07:32 PM, Julian Wiedmann wrote:
From: Mateusz Jurczyk <redacted> Verify that the caller-provided sockaddr structure is large enough to contain the sa_family field, before accessing it in bind() and connect() handlers of the AF_IUCV socket. Since neither syscall enforces a minimum size of the corresponding memory region, very short sockaddrs (zero or one byte long) result in operating on uninitialized memory while referencing .sa_family.
Won't it make sense to generally check the minimum length for .sa_family at a single point before fixing all called sites? Regards, Oliver
quoted hunk ↗ jump to hunk
Fixes: 52a82e23b9f2 ("af_iucv: Validate socket address length in iucv_sock_bind()") Signed-off-by: Mateusz Jurczyk <redacted> [jwi: removed unneeded null-check for addr] Signed-off-by: Julian Wiedmann <redacted> --- net/iucv/af_iucv.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c index 05112094d76b..ac033e413bc5 100644 --- a/net/iucv/af_iucv.c +++ b/net/iucv/af_iucv.c@@ -715,10 +715,8 @@ static int iucv_sock_bind(struct socket *sock, struct sockaddr *addr, char uid[9]; /* Verify the input sockaddr */ - if (!addr || addr->sa_family != AF_IUCV) - return -EINVAL; - - if (addr_len < sizeof(struct sockaddr_iucv)) + if (addr_len < sizeof(struct sockaddr_iucv) || + addr->sa_family != AF_IUCV) return -EINVAL; lock_sock(sk);@@ -862,7 +860,7 @@ static int iucv_sock_connect(struct socket *sock, struct sockaddr *addr, struct iucv_sock *iucv = iucv_sk(sk); int err; - if (addr->sa_family != AF_IUCV || alen < sizeof(struct sockaddr_iucv)) + if (alen < sizeof(struct sockaddr_iucv) || addr->sa_family != AF_IUCV) return -EINVAL; if (sk->sk_state != IUCV_OPEN && sk->sk_state != IUCV_BOUND)