All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	syzbot <syzbot+0049bebbf3042dbd2e8f@syzkaller.appspotmail.com>,
	syzbot <syzbot+915c9f99f3dbc4bd6cd1@syzkaller.appspotmail.com>
Subject: [PATCH] net: socket: Always initialize family field at move_addr_to_kernel().
Date: Mon,  1 Apr 2019 23:19:22 +0900	[thread overview]
Message-ID: <1554128362-12274-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> (raw)

syzbot is reporting uninitialized value at rds_connect [1] and
rds_bind [2]. This is because syzbot is passing ulen == 0 whereas
these functions expects that it is safe to access sockaddr->family field
in order to determine minimal ulen size for validation. I noticed that
the same problem also exists in tomoyo_check_inet_address() function.

Although the right fix might be to scatter around

  if (ulen < sizeof(__kernel_sa_family_t))
    return 0;

if the function wants to become no-op when the address is too short or

  if (ulen < sizeof(__kernel_sa_family_t))
    return -EINVAL;

if the function wants to reject when the address is too short, we can
avoid duplication (at e.g. LSM layer and protocol layer) if we make sure
that sockaddr->family field is always accessible.

[1] https://syzkaller.appspot.com/bug?id=f4e61c010416c1e6f0fa3ffe247561b60a50ad71
[2] https://syzkaller.appspot.com/bug?id=a4bf9e41b7e055c3823fdcd83e8c58ca7270e38f

Reported-by: syzbot <syzbot+0049bebbf3042dbd2e8f@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+915c9f99f3dbc4bd6cd1@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 net/socket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/socket.c b/net/socket.c
index 8255f5b..10a780b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -181,6 +181,7 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
 
 int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr)
 {
+	kaddr->ss_family = 0;
 	if (ulen < 0 || ulen > sizeof(struct sockaddr_storage))
 		return -EINVAL;
 	if (ulen == 0)
-- 
1.8.3.1


             reply	other threads:[~2019-04-01 14:19 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-01 14:19 Tetsuo Handa [this message]
2019-04-02 20:23 ` [PATCH] net: socket: Always initialize family field at move_addr_to_kernel() David Miller
2019-04-02 21:07   ` Tetsuo Handa
2019-04-04  4:49     ` David Miller
2019-04-11 11:31       ` Tetsuo Handa
2019-04-11 16:45         ` Casey Schaufler
2019-04-11 22:33         ` Paul Moore
2019-04-12  0:24           ` Tetsuo Handa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1554128362-12274-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=syzbot+0049bebbf3042dbd2e8f@syzkaller.appspotmail.com \
    --cc=syzbot+915c9f99f3dbc4bd6cd1@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.