diff mbox series

[1/3] selinux: Check address length before reading address family

Message ID 1555066776-9758-1-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

Commit Message

Tetsuo Handa April 12, 2019, 10:59 a.m. UTC
KMSAN will complain if valid address length passed to bind()/connect() is
shorter than sizeof("struct sockaddr"->sa_family) bytes.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 security/selinux/hooks.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Paul Moore April 15, 2019, 4:46 p.m. UTC | #1
On Fri, Apr 12, 2019 at 7:00 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> KMSAN will complain if valid address length passed to bind()/connect() is
> shorter than sizeof("struct sockaddr"->sa_family) bytes.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  security/selinux/hooks.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

A reminder to please CC the SELinux mailing list on patches such as these.

This looks good to me, and since it's specific to SELinux, I've gone
ahead and merged it into the selinux/next branch.  Thanks for your
help on this.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d5fdcb0d26fe..c61787b15f27 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4512,7 +4512,7 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>                 struct lsm_network_audit net = {0,};
>                 struct sockaddr_in *addr4 = NULL;
>                 struct sockaddr_in6 *addr6 = NULL;
> -               u16 family_sa = address->sa_family;
> +               u16 family_sa;
>                 unsigned short snum;
>                 u32 sid, node_perm;
>
> @@ -4522,6 +4522,9 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>                  * need to check address->sa_family as it is possible to have
>                  * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>                  */
> +               if (addrlen < offsetofend(struct sockaddr, sa_family))
> +                       return -EINVAL;
> +               family_sa = address->sa_family;
>                 switch (family_sa) {
>                 case AF_UNSPEC:
>                 case AF_INET:
> @@ -4654,6 +4657,8 @@ static int selinux_socket_connect_helper(struct socket *sock,
>                  * need to check address->sa_family as it is possible to have
>                  * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>                  */
> +               if (addrlen < offsetofend(struct sockaddr, sa_family))
> +                       return -EINVAL;
>                 switch (address->sa_family) {
>                 case AF_INET:
>                         addr4 = (struct sockaddr_in *)address;
> --
> 2.16.5
diff mbox series

Patch

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d5fdcb0d26fe..c61787b15f27 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4512,7 +4512,7 @@  static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
 		struct lsm_network_audit net = {0,};
 		struct sockaddr_in *addr4 = NULL;
 		struct sockaddr_in6 *addr6 = NULL;
-		u16 family_sa = address->sa_family;
+		u16 family_sa;
 		unsigned short snum;
 		u32 sid, node_perm;
 
@@ -4522,6 +4522,9 @@  static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
 		 * need to check address->sa_family as it is possible to have
 		 * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
 		 */
+		if (addrlen < offsetofend(struct sockaddr, sa_family))
+			return -EINVAL;
+		family_sa = address->sa_family;
 		switch (family_sa) {
 		case AF_UNSPEC:
 		case AF_INET:
@@ -4654,6 +4657,8 @@  static int selinux_socket_connect_helper(struct socket *sock,
 		 * need to check address->sa_family as it is possible to have
 		 * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
 		 */
+		if (addrlen < offsetofend(struct sockaddr, sa_family))
+			return -EINVAL;
 		switch (address->sa_family) {
 		case AF_INET:
 			addr4 = (struct sockaddr_in *)address;