aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Borkmann <dborkman@redhat.com>2013-09-07 20:51:21 +0200
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2013-10-13 16:08:28 -0700
commit8ee62e8c9495a6d0f8d227bbe0f02e1b288bac30 (patch)
treec27076b072d1bdc083766ac058ebd89a4f2927d4
parent8b1ada497191ab2be429c74817a8de63f9aa2bf6 (diff)
net: sctp: fix smatch warning in sctp_send_asconf_del_ip
[ Upstream commit 88362ad8f9a6cea787420b57cc27ccacef000dbe ] This was originally reported in [1] and posted by Neil Horman [2], he said: Fix up a missed null pointer check in the asconf code. If we don't find a local address, but we pass in an address length of more than 1, we may dereference a NULL laddr pointer. Currently this can't happen, as the only users of the function pass in the value 1 as the addrcnt parameter, but its not hot path, and it doesn't hurt to check for NULL should that ever be the case. The callpath from sctp_asconf_mgmt() looks okay. But this could be triggered from sctp_setsockopt_bindx() call with SCTP_BINDX_REM_ADDR and addrcnt > 1 while passing all possible addresses from the bind list to SCTP_BINDX_REM_ADDR so that we do *not* find a single address in the association's bind address list that is not in the packed array of addresses. If this happens when we have an established association with ASCONF-capable peers, then we could get a NULL pointer dereference as we only check for laddr == NULL && addrcnt == 1 and call later sctp_make_asconf_update_ip() with NULL laddr. BUT: this actually won't happen as sctp_bindx_rem() will catch such a case and return with an error earlier. As this is incredably unintuitive and error prone, add a check to catch at least future bugs here. As Neil says, its not hot path. Introduced by 8a07eb0a5 ("sctp: Add ASCONF operation on the single-homed host"). [1] http://www.spinics.net/lists/linux-sctp/msg02132.html [2] http://www.spinics.net/lists/linux-sctp/msg02133.html Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: Daniel Borkmann <dborkman@redhat.com> Cc: Michio Honda <micchie@sfc.wide.ad.jp> Acked-By: Neil Horman <nhorman@tuxdriver.com> Acked-by: Vlad Yasevich <vyasevich@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--net/sctp/socket.c3
1 files changed, 3 insertions, 0 deletions
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index f160c53878a..79bc251042b 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -820,6 +820,9 @@ static int sctp_send_asconf_del_ip(struct sock *sk,
goto skip_mkasconf;
}
+ if (laddr == NULL)
+ return -EINVAL;
+
/* We do not need RCU protection throughout this loop
* because this is done under a socket lock from the
* setsockopt call.