Skip to content

Commit c981f25

Browse files
author
Al Viro
committed
sctp: use vmemdup_user() rather than badly open-coding memdup_user()
Signed-off-by: Al Viro <[email protected]>
1 parent 59aeaf3 commit c981f25

File tree

1 file changed

+11
-48
lines changed

1 file changed

+11
-48
lines changed

net/sctp/socket.c

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -970,13 +970,6 @@ int sctp_asconf_mgmt(struct sctp_sock *sp, struct sctp_sockaddr_entry *addrw)
970970
* This is used for tunneling the sctp_bindx() request through sctp_setsockopt()
971971
* from userspace.
972972
*
973-
* We don't use copy_from_user() for optimization: we first do the
974-
* sanity checks (buffer size -fast- and access check-healthy
975-
* pointer); if all of those succeed, then we can alloc the memory
976-
* (expensive operation) needed to copy the data to kernel. Then we do
977-
* the copying without checking the user space area
978-
* (__copy_from_user()).
979-
*
980973
* On exit there is no need to do sockfd_put(), sys_setsockopt() does
981974
* it.
982975
*
@@ -1006,25 +999,15 @@ static int sctp_setsockopt_bindx(struct sock *sk,
1006999
if (unlikely(addrs_size <= 0))
10071000
return -EINVAL;
10081001

1009-
/* Check the user passed a healthy pointer. */
1010-
if (unlikely(!access_ok(VERIFY_READ, addrs, addrs_size)))
1011-
return -EFAULT;
1012-
1013-
/* Alloc space for the address array in kernel memory. */
1014-
kaddrs = kmalloc(addrs_size, GFP_USER | __GFP_NOWARN);
1015-
if (unlikely(!kaddrs))
1016-
return -ENOMEM;
1017-
1018-
if (__copy_from_user(kaddrs, addrs, addrs_size)) {
1019-
kfree(kaddrs);
1020-
return -EFAULT;
1021-
}
1002+
kaddrs = vmemdup_user(addrs, addrs_size);
1003+
if (unlikely(IS_ERR(kaddrs)))
1004+
return PTR_ERR(kaddrs);
10221005

10231006
/* Walk through the addrs buffer and count the number of addresses. */
10241007
addr_buf = kaddrs;
10251008
while (walk_size < addrs_size) {
10261009
if (walk_size + sizeof(sa_family_t) > addrs_size) {
1027-
kfree(kaddrs);
1010+
kvfree(kaddrs);
10281011
return -EINVAL;
10291012
}
10301013

@@ -1035,7 +1018,7 @@ static int sctp_setsockopt_bindx(struct sock *sk,
10351018
* causes the address buffer to overflow return EINVAL.
10361019
*/
10371020
if (!af || (walk_size + af->sockaddr_len) > addrs_size) {
1038-
kfree(kaddrs);
1021+
kvfree(kaddrs);
10391022
return -EINVAL;
10401023
}
10411024
addrcnt++;
@@ -1065,7 +1048,7 @@ static int sctp_setsockopt_bindx(struct sock *sk,
10651048
}
10661049

10671050
out:
1068-
kfree(kaddrs);
1051+
kvfree(kaddrs);
10691052

10701053
return err;
10711054
}
@@ -1323,13 +1306,6 @@ static int __sctp_connect(struct sock *sk,
13231306
* land and invoking either sctp_connectx(). This is used for tunneling
13241307
* the sctp_connectx() request through sctp_setsockopt() from userspace.
13251308
*
1326-
* We don't use copy_from_user() for optimization: we first do the
1327-
* sanity checks (buffer size -fast- and access check-healthy
1328-
* pointer); if all of those succeed, then we can alloc the memory
1329-
* (expensive operation) needed to copy the data to kernel. Then we do
1330-
* the copying without checking the user space area
1331-
* (__copy_from_user()).
1332-
*
13331309
* On exit there is no need to do sockfd_put(), sys_setsockopt() does
13341310
* it.
13351311
*
@@ -1345,7 +1321,6 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
13451321
sctp_assoc_t *assoc_id)
13461322
{
13471323
struct sockaddr *kaddrs;
1348-
gfp_t gfp = GFP_KERNEL;
13491324
int err = 0;
13501325

13511326
pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
@@ -1354,24 +1329,12 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
13541329
if (unlikely(addrs_size <= 0))
13551330
return -EINVAL;
13561331

1357-
/* Check the user passed a healthy pointer. */
1358-
if (unlikely(!access_ok(VERIFY_READ, addrs, addrs_size)))
1359-
return -EFAULT;
1360-
1361-
/* Alloc space for the address array in kernel memory. */
1362-
if (sk->sk_socket->file)
1363-
gfp = GFP_USER | __GFP_NOWARN;
1364-
kaddrs = kmalloc(addrs_size, gfp);
1365-
if (unlikely(!kaddrs))
1366-
return -ENOMEM;
1367-
1368-
if (__copy_from_user(kaddrs, addrs, addrs_size)) {
1369-
err = -EFAULT;
1370-
} else {
1371-
err = __sctp_connect(sk, kaddrs, addrs_size, assoc_id);
1372-
}
1332+
kaddrs = vmemdup_user(addrs, addrs_size);
1333+
if (unlikely(IS_ERR(kaddrs)))
1334+
return PTR_ERR(kaddrs);
13731335

1374-
kfree(kaddrs);
1336+
err = __sctp_connect(sk, kaddrs, addrs_size, assoc_id);
1337+
kvfree(kaddrs);
13751338

13761339
return err;
13771340
}

0 commit comments

Comments
 (0)