| Message ID | 1555066776-9758-2-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] selinux: Check address length before reading address family | expand |
On 4/12/2019 3:59 AM, Tetsuo Handa wrote: > KMSAN will complain if valid address length passed to bind()/connect()/ > sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes. > > Also, since smk_ipv6_port_label()/smack_netlabel_send()/ > smack_ipv6host_label()/smk_ipv6_check()/smk_ipv6_port_check() are not > checking valid address length and/or address family, make sure we check > both. The minimal valid length in smack_socket_connect() is changed from > sizeof(struct sockaddr_in6) bytes to SIN6_LEN_RFC2133 bytes, for it seems > that Smack is not using "struct sockaddr_in6"->sin6_scope_id field. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Acked-by: Casey Schaufler <casey@schaufler-ca.com> > --- > security/smack/smack_lsm.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 5c1613519d5a..4b59e4519e0c 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -2805,13 +2805,17 @@ static int smack_socket_socketpair(struct socket *socka, > * > * Records the label bound to a port. > * > - * Returns 0 > + * Returns 0 on success, and error code otherwise > */ > static int smack_socket_bind(struct socket *sock, struct sockaddr *address, > int addrlen) > { > - if (sock->sk != NULL && sock->sk->sk_family == PF_INET6) > + if (sock->sk != NULL && sock->sk->sk_family == PF_INET6) { > + if (addrlen < SIN6_LEN_RFC2133 || > + address->sa_family != AF_INET6) > + return -EINVAL; > smk_ipv6_port_label(sock, address); > + } > return 0; > } > #endif /* SMACK_IPV6_PORT_LABELING */ > @@ -2847,12 +2851,13 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, > > switch (sock->sk->sk_family) { > case PF_INET: > - if (addrlen < sizeof(struct sockaddr_in)) > + if (addrlen < sizeof(struct sockaddr_in) || > + sap->sa_family != AF_INET) > return -EINVAL; > rc = smack_netlabel_send(sock->sk, (struct sockaddr_in *)sap); > break; > case PF_INET6: > - if (addrlen < sizeof(struct sockaddr_in6)) > + if (addrlen < SIN6_LEN_RFC2133 || sap->sa_family != AF_INET6) > return -EINVAL; > #ifdef SMACK_IPV6_SECMARK_LABELING > rsp = smack_ipv6host_label(sip); > @@ -3682,9 +3687,15 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg, > > switch (sock->sk->sk_family) { > case AF_INET: > + if (msg->msg_namelen < sizeof(struct sockaddr_in) || > + sip->sin_family != AF_INET) > + return -EINVAL; > rc = smack_netlabel_send(sock->sk, sip); > break; > case AF_INET6: > + if (msg->msg_namelen < SIN6_LEN_RFC2133 || > + sap->sin6_family != AF_INET6) > + return -EINVAL; > #ifdef SMACK_IPV6_SECMARK_LABELING > rsp = smack_ipv6host_label(sap); > if (rsp != NULL)
On Fri, 12 Apr 2019, Casey Schaufler wrote: > On 4/12/2019 3:59 AM, Tetsuo Handa wrote: > > KMSAN will complain if valid address length passed to bind()/connect()/ > > sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes. > > > > Also, since smk_ipv6_port_label()/smack_netlabel_send()/ > > smack_ipv6host_label()/smk_ipv6_check()/smk_ipv6_port_check() are not > > checking valid address length and/or address family, make sure we check > > both. The minimal valid length in smack_socket_connect() is changed from > > sizeof(struct sockaddr_in6) bytes to SIN6_LEN_RFC2133 bytes, for it seems > > that Smack is not using "struct sockaddr_in6"->sin6_scope_id field. > > > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > Acked-by: Casey Schaufler <casey@schaufler-ca.com> Casey: will you be taking this via your tree?
On 4/29/2019 12:58 PM, James Morris wrote: > On Fri, 12 Apr 2019, Casey Schaufler wrote: > >> On 4/12/2019 3:59 AM, Tetsuo Handa wrote: >>> KMSAN will complain if valid address length passed to bind()/connect()/ >>> sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes. >>> >>> Also, since smk_ipv6_port_label()/smack_netlabel_send()/ >>> smack_ipv6host_label()/smk_ipv6_check()/smk_ipv6_port_check() are not >>> checking valid address length and/or address family, make sure we check >>> both. The minimal valid length in smack_socket_connect() is changed from >>> sizeof(struct sockaddr_in6) bytes to SIN6_LEN_RFC2133 bytes, for it seems >>> that Smack is not using "struct sockaddr_in6"->sin6_scope_id field. >>> >>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> Acked-by: Casey Schaufler <casey@schaufler-ca.com> > Casey: will you be taking this via your tree? Sure. I will add it today.
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 5c1613519d5a..4b59e4519e0c 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2805,13 +2805,17 @@ static int smack_socket_socketpair(struct socket *socka, * * Records the label bound to a port. * - * Returns 0 + * Returns 0 on success, and error code otherwise */ static int smack_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen) { - if (sock->sk != NULL && sock->sk->sk_family == PF_INET6) + if (sock->sk != NULL && sock->sk->sk_family == PF_INET6) { + if (addrlen < SIN6_LEN_RFC2133 || + address->sa_family != AF_INET6) + return -EINVAL; smk_ipv6_port_label(sock, address); + } return 0; } #endif /* SMACK_IPV6_PORT_LABELING */ @@ -2847,12 +2851,13 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, switch (sock->sk->sk_family) { case PF_INET: - if (addrlen < sizeof(struct sockaddr_in)) + if (addrlen < sizeof(struct sockaddr_in) || + sap->sa_family != AF_INET) return -EINVAL; rc = smack_netlabel_send(sock->sk, (struct sockaddr_in *)sap); break; case PF_INET6: - if (addrlen < sizeof(struct sockaddr_in6)) + if (addrlen < SIN6_LEN_RFC2133 || sap->sa_family != AF_INET6) return -EINVAL; #ifdef SMACK_IPV6_SECMARK_LABELING rsp = smack_ipv6host_label(sip); @@ -3682,9 +3687,15 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg, switch (sock->sk->sk_family) { case AF_INET: + if (msg->msg_namelen < sizeof(struct sockaddr_in) || + sip->sin_family != AF_INET) + return -EINVAL; rc = smack_netlabel_send(sock->sk, sip); break; case AF_INET6: + if (msg->msg_namelen < SIN6_LEN_RFC2133 || + sap->sin6_family != AF_INET6) + return -EINVAL; #ifdef SMACK_IPV6_SECMARK_LABELING rsp = smack_ipv6host_label(sap); if (rsp != NULL)
KMSAN will complain if valid address length passed to bind()/connect()/ sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes. Also, since smk_ipv6_port_label()/smack_netlabel_send()/ smack_ipv6host_label()/smk_ipv6_check()/smk_ipv6_port_check() are not checking valid address length and/or address family, make sure we check both. The minimal valid length in smack_socket_connect() is changed from sizeof(struct sockaddr_in6) bytes to SIN6_LEN_RFC2133 bytes, for it seems that Smack is not using "struct sockaddr_in6"->sin6_scope_id field. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- security/smack/smack_lsm.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)