Message ID | 20181027195853.30243-1-tomasbortoli@gmail.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Series | sctp: socket.c validate sprstat_policy | expand |
Hi Tomas, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on net-next/master] [also build test WARNING on v4.19 next-20181019] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Tomas-Bortoli/sctp-socket-c-validate-sprstat_policy/20181028-040051 config: i386-randconfig-x075-201843 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): In file included from arch/x86/include/asm/atomic.h:5:0, from include/linux/atomic.h:7, from include/linux/crypto.h:20, from include/crypto/hash.h:16, from net/sctp/socket.c:55: net/sctp/socket.c: In function 'sctp_getsockopt_pr_assocstatus': net/sctp/socket.c:7086:25: error: called object is not a function or function pointer if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/compiler.h:58:30: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^~~~ >> net/sctp/socket.c:7086:2: note: in expansion of macro 'if' if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) ^~ net/sctp/socket.c:7086:25: error: called object is not a function or function pointer if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/compiler.h:58:42: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^~~~ >> net/sctp/socket.c:7086:2: note: in expansion of macro 'if' if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) ^~ net/sctp/socket.c:7086:25: error: called object is not a function or function pointer if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/compiler.h:69:16: note: in definition of macro '__trace_if' ______r = !!(cond); \ ^~~~ >> net/sctp/socket.c:7086:2: note: in expansion of macro 'if' if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) ^~ vim +/if +7086 net/sctp/socket.c 7066 7067 static int sctp_getsockopt_pr_assocstatus(struct sock *sk, int len, 7068 char __user *optval, 7069 int __user *optlen) 7070 { 7071 struct sctp_prstatus params; 7072 struct sctp_association *asoc; 7073 int policy; 7074 int retval = -EINVAL; 7075 7076 if (len < sizeof(params)) 7077 goto out; 7078 7079 len = sizeof(params); 7080 if (copy_from_user(¶ms, optval, len)) { 7081 retval = -EFAULT; 7082 goto out; 7083 } 7084 7085 policy = params.sprstat_policy; > 7086 if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) 7087 __SCTP_PR_INDEX(policy) > SCTP_PR_INDEX(MAX)) 7088 goto out; 7089 7090 asoc = sctp_id2assoc(sk, params.sprstat_assoc_id); 7091 if (!asoc) 7092 goto out; 7093 7094 if (policy & SCTP_PR_SCTP_ALL) { 7095 params.sprstat_abandoned_unsent = 0; 7096 params.sprstat_abandoned_sent = 0; 7097 for (policy = 0; policy <= SCTP_PR_INDEX(MAX); policy++) { 7098 params.sprstat_abandoned_unsent += 7099 asoc->abandoned_unsent[policy]; 7100 params.sprstat_abandoned_sent += 7101 asoc->abandoned_sent[policy]; 7102 } 7103 } else { 7104 params.sprstat_abandoned_unsent = 7105 asoc->abandoned_unsent[__SCTP_PR_INDEX(policy)]; 7106 params.sprstat_abandoned_sent = 7107 asoc->abandoned_sent[__SCTP_PR_INDEX(policy)]; 7108 } 7109 7110 if (put_user(len, optlen)) { 7111 retval = -EFAULT; 7112 goto out; 7113 } 7114 7115 if (copy_to_user(optval, ¶ms, len)) { 7116 retval = -EFAULT; 7117 goto out; 7118 } 7119 7120 retval = 0; 7121 7122 out: 7123 return retval; 7124 } 7125 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Tomas, Thank you for the patch! Yet something to improve: [auto build test ERROR on net-next/master] [also build test ERROR on v4.19 next-20181019] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Tomas-Bortoli/sctp-socket-c-validate-sprstat_policy/20181028-040051 config: i386-randconfig-x077-201843 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): net//sctp/socket.c: In function 'sctp_getsockopt_pr_assocstatus': >> net//sctp/socket.c:7086:25: error: called object is not a function or function pointer if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +7086 net//sctp/socket.c 7066 7067 static int sctp_getsockopt_pr_assocstatus(struct sock *sk, int len, 7068 char __user *optval, 7069 int __user *optlen) 7070 { 7071 struct sctp_prstatus params; 7072 struct sctp_association *asoc; 7073 int policy; 7074 int retval = -EINVAL; 7075 7076 if (len < sizeof(params)) 7077 goto out; 7078 7079 len = sizeof(params); 7080 if (copy_from_user(¶ms, optval, len)) { 7081 retval = -EFAULT; 7082 goto out; 7083 } 7084 7085 policy = params.sprstat_policy; > 7086 if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) 7087 __SCTP_PR_INDEX(policy) > SCTP_PR_INDEX(MAX)) 7088 goto out; 7089 7090 asoc = sctp_id2assoc(sk, params.sprstat_assoc_id); 7091 if (!asoc) 7092 goto out; 7093 7094 if (policy & SCTP_PR_SCTP_ALL) { 7095 params.sprstat_abandoned_unsent = 0; 7096 params.sprstat_abandoned_sent = 0; 7097 for (policy = 0; policy <= SCTP_PR_INDEX(MAX); policy++) { 7098 params.sprstat_abandoned_unsent += 7099 asoc->abandoned_unsent[policy]; 7100 params.sprstat_abandoned_sent += 7101 asoc->abandoned_sent[policy]; 7102 } 7103 } else { 7104 params.sprstat_abandoned_unsent = 7105 asoc->abandoned_unsent[__SCTP_PR_INDEX(policy)]; 7106 params.sprstat_abandoned_sent = 7107 asoc->abandoned_sent[__SCTP_PR_INDEX(policy)]; 7108 } 7109 7110 if (put_user(len, optlen)) { 7111 retval = -EFAULT; 7112 goto out; 7113 } 7114 7115 if (copy_to_user(optval, ¶ms, len)) { 7116 retval = -EFAULT; 7117 goto out; 7118 } 7119 7120 retval = 0; 7121 7122 out: 7123 return retval; 7124 } 7125 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/net/sctp/socket.c b/net/sctp/socket.c index fc0386e8ff23..5290b8bd40c8 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -7083,7 +7083,8 @@ static int sctp_getsockopt_pr_assocstatus(struct sock *sk, int len, } policy = params.sprstat_policy; - if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL))) + if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) + __SCTP_PR_INDEX(policy) > SCTP_PR_INDEX(MAX)) goto out; asoc = sctp_id2assoc(sk, params.sprstat_assoc_id); @@ -7142,7 +7143,8 @@ static int sctp_getsockopt_pr_streamstatus(struct sock *sk, int len, } policy = params.sprstat_policy; - if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL))) + if (!policy || (policy & ~(SCTP_PR_SCTP_MASK | SCTP_PR_SCTP_ALL)) || + __SCTP_PR_INDEX(policy) > SCTP_PR_INDEX(MAX)) goto out; asoc = sctp_id2assoc(sk, params.sprstat_assoc_id);
It is possible to perform out-of-bound reads on sctp_getsockopt_pr_streamstatus() and on sctp_getsockopt_pr_assocstatus() by passing from userspace a sprstat_policy that overflows the abandoned_sent/abandoned_unsent fixed length arrays. The over-read data are directly copied/leaked to userspace. Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com> Reported-by: syzbot+5da0d0a72a9e7d791748@syzkaller.appspotmail.com --- net/sctp/socket.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)