From: Dmitry Vyukov via Virtualization <virtualization@lists.linux-foundation.org>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Kevin Easton <kevin@guarana.org>, KVM list <kvm@vger.kernel.org>,
netdev <netdev@vger.kernel.org>,
syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
LKML <linux-kernel@vger.kernel.org>,
virtualization@lists.linux-foundation.org
Subject: Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node
Date: Fri, 27 Apr 2018 18:29:20 +0200 [thread overview]
Message-ID: <CACT4Y+ZUoAB8ZORGnR+xhcg46XqP+HsfbeuQ+HNcRVE51xdH6A__35645.7237889574$1524846481$gmane$org@mail.gmail.com> (raw)
In-Reply-To: <CACT4Y+bzWiPvV+pVvys4v8CwUhF7iYVskxn_yeo6ztN5uKA0VA@mail.gmail.com>
On Fri, Apr 27, 2018 at 6:25 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> >> The struct vhost_msg within struct vhost_msg_node is copied to userspace,
>>> >> so it should be allocated with kzalloc() to ensure all structure padding
>>> >> is zeroed.
>>> >>
>>> >> Signed-off-by: Kevin Easton <kevin@guarana.org>
>>> >> Reported-by: syzbot+87cfa083e727a224754b@syzkaller.appspotmail.com
>>> >
>>> > Does it help if a patch naming the padding is applied,
>>> > and then we init just the relevant field?
>>> > Just curious.
>>>
>>> Yes, it would help.
>>
>> I think it's slightly better that way then. node has a lot of internal
>> stuff we don't care to init. Would you mind taking my patch and building
>> on top of that then?
>
>
> But it's asking for more information leaks in future. This looks like
> work for compiler.
Modern compilers are perfectly capable of doing this:
#include <memory.h>
#include <unistd.h>
int main()
{
int x[10];
memset(&x, 0, sizeof(x));
x[0] = 0;
x[2] = 2;
x[3] = 3;
x[4] = 4;
x[5] = 5;
x[6] = 6;
x[7] = 7;
x[8] = 8;
x[9] = 9;
write(0, x, sizeof(x));
return 0;
}
gcc 7.2 -O3
0000000000000540 <main>:
540: sub $0x38,%rsp
544: mov $0x28,%edx
549: xor %edi,%edi
54b: movdqa 0x1cd(%rip),%xmm0 # 720 <_IO_stdin_used+0x10>
553: mov %rsp,%rsi
556: movq $0x0,(%rsp)
55e: movups %xmm0,0x8(%rsp)
563: movdqa 0x1c5(%rip),%xmm0 # 730 <_IO_stdin_used+0x20>
56b: movups %xmm0,0x18(%rsp)
570: callq 520 <write@plt>
575: xor %eax,%eax
577: add $0x38,%rsp
57b: retq
57c: nopl 0x0(%rax)
But they will not put a security hole next time fields are shuffled.
>>> >> ---
>>> >> drivers/vhost/vhost.c | 2 +-
>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>> >> index f3bd8e9..1b84dcff 100644
>>> >> --- a/drivers/vhost/vhost.c
>>> >> +++ b/drivers/vhost/vhost.c
>>> >> @@ -2339,7 +2339,7 @@ EXPORT_SYMBOL_GPL(vhost_disable_notify);
>>> >> /* Create a new message. */
>>> >> struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
>>> >> {
>>> >> - struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
>>> >> + struct vhost_msg_node *node = kzalloc(sizeof *node, GFP_KERNEL);
>>> >> if (!node)
>>> >> return NULL;
>>> >> node->vq = vq;
>>> >> --
>>> >> 2.8.1
next prev parent reply other threads:[~2018-04-27 16:29 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-23 16:56 KMSAN: uninit-value in _copy_to_iter (2) syzbot
2018-04-25 19:19 ` syzbot
2018-06-07 15:38 ` Michael S. Tsirkin
2018-06-07 15:38 ` Michael S. Tsirkin
2018-06-07 15:38 ` syzbot
2018-06-07 16:25 ` Dmitry Vyukov
2018-06-07 16:25 ` Dmitry Vyukov via Virtualization
2018-06-07 17:04 ` syzbot
2018-06-07 17:43 ` Al Viro
2018-06-07 17:43 ` Al Viro
2018-06-07 17:59 ` Michael S. Tsirkin
2018-06-07 17:59 ` Michael S. Tsirkin
2018-06-07 18:04 ` Al Viro
2018-06-07 18:04 ` Al Viro
2018-06-07 19:29 ` Michael S. Tsirkin
2018-06-07 19:29 ` Michael S. Tsirkin
2018-06-07 17:10 ` Michael S. Tsirkin
2018-06-07 17:31 ` syzbot
2018-06-07 17:10 ` Michael S. Tsirkin
2018-04-27 15:45 ` [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node Kevin Easton
2018-04-27 16:05 ` Michael S. Tsirkin
2018-04-27 16:11 ` Dmitry Vyukov
2018-04-27 16:11 ` Dmitry Vyukov via Virtualization
2018-04-27 16:15 ` Michael S. Tsirkin
2018-04-27 16:15 ` Michael S. Tsirkin
2018-04-27 16:25 ` Dmitry Vyukov
2018-04-27 16:29 ` Dmitry Vyukov
2018-04-27 16:29 ` Dmitry Vyukov via Virtualization [this message]
2018-04-27 16:25 ` Dmitry Vyukov via Virtualization
2018-04-27 19:36 ` Michael S. Tsirkin
2018-04-27 19:36 ` Michael S. Tsirkin
2018-04-29 8:10 ` Dmitry Vyukov
2018-04-29 8:10 ` Dmitry Vyukov via Virtualization
2018-04-28 1:07 ` Kevin Easton
2018-04-28 1:51 ` Kevin Easton
2018-04-28 2:23 ` Jason Wang
2018-04-28 2:23 ` Jason Wang
2018-04-27 16:05 ` Michael S. Tsirkin
2018-05-07 13:03 ` Michael S. Tsirkin
2018-05-07 13:12 ` Dmitry Vyukov via Virtualization
2018-05-07 13:12 ` Dmitry Vyukov
2018-05-08 8:27 ` Kevin Easton
2018-05-07 13:03 ` Michael S. Tsirkin
2018-05-29 22:19 ` [net] " Guenter Roeck
2018-05-29 22:19 ` Guenter Roeck
2018-05-30 3:01 ` Michael S. Tsirkin
2018-05-30 3:01 ` Michael S. Tsirkin
2018-05-30 3:42 ` Guenter Roeck
2018-05-30 3:42 ` Guenter Roeck
2018-06-04 12:34 ` Dmitry Vyukov
2018-06-04 12:34 ` Dmitry Vyukov via Virtualization
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='CACT4Y+ZUoAB8ZORGnR+xhcg46XqP+HsfbeuQ+HNcRVE51xdH6A__35645.7237889574$1524846481$gmane$org@mail.gmail.com' \
--to=virtualization@lists.linux-foundation.org \
--cc=dvyukov@google.com \
--cc=kevin@guarana.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=syzkaller-bugs@googlegroups.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.