* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180427154502.GA22544@la.guarana.org> @ 2018-04-27 16:05 ` Michael S. Tsirkin [not found] ` <20180427185501-mutt-send-email-mst@kernel.org> ` (4 subsequent siblings) 5 siblings, 0 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-04-27 16:05 UTC (permalink / raw) To: Kevin Easton; +Cc: kvm, netdev, syzkaller-bugs, linux-kernel, virtualization On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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. > --- > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180427185501-mutt-send-email-mst@kernel.org>]
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180427185501-mutt-send-email-mst@kernel.org> @ 2018-04-27 16:11 ` Dmitry Vyukov via Virtualization 2018-04-27 16:15 ` Michael S. Tsirkin ` (3 more replies) [not found] ` <20180428010756.GA27341@la.guarana.org> 1 sibling, 4 replies; 21+ messages in thread From: Dmitry Vyukov via Virtualization @ 2018-04-27 16:11 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Kevin Easton, KVM list, netdev, syzkaller-bugs, LKML, virtualization On Fri, Apr 27, 2018 at 6:05 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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. >> --- >> 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 > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20180427185501-mutt-send-email-mst%40kernel.org. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node 2018-04-27 16:11 ` Dmitry Vyukov via Virtualization @ 2018-04-27 16:15 ` Michael S. Tsirkin [not found] ` <20180427191430-mutt-send-email-mst@kernel.org> ` (2 subsequent siblings) 3 siblings, 0 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-04-27 16:15 UTC (permalink / raw) To: Dmitry Vyukov Cc: Kevin Easton, KVM list, netdev, syzkaller-bugs, LKML, virtualization On Fri, Apr 27, 2018 at 06:11:31PM +0200, Dmitry Vyukov wrote: > On Fri, Apr 27, 2018 at 6:05 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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? > >> --- > >> 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 > > > > -- > > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20180427185501-mutt-send-email-mst%40kernel.org. > > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180427191430-mutt-send-email-mst@kernel.org>]
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180427191430-mutt-send-email-mst@kernel.org> @ 2018-04-27 16:25 ` Dmitry Vyukov via Virtualization [not found] ` <CACT4Y+bzWiPvV+pVvys4v8CwUhF7iYVskxn_yeo6ztN5uKA0VA@mail.gmail.com> 1 sibling, 0 replies; 21+ messages in thread From: Dmitry Vyukov via Virtualization @ 2018-04-27 16:25 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Kevin Easton, KVM list, netdev, syzkaller-bugs, LKML, virtualization On Fri, Apr 27, 2018 at 6:15 PM, Michael S. Tsirkin <mst@redhat.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. >> >> --- >> >> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <CACT4Y+bzWiPvV+pVvys4v8CwUhF7iYVskxn_yeo6ztN5uKA0VA@mail.gmail.com>]
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <CACT4Y+bzWiPvV+pVvys4v8CwUhF7iYVskxn_yeo6ztN5uKA0VA@mail.gmail.com> @ 2018-04-27 16:29 ` Dmitry Vyukov via Virtualization 0 siblings, 0 replies; 21+ messages in thread From: Dmitry Vyukov via Virtualization @ 2018-04-27 16:29 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Kevin Easton, KVM list, netdev, syzkaller-bugs, LKML, virtualization 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node 2018-04-27 16:11 ` Dmitry Vyukov via Virtualization 2018-04-27 16:15 ` Michael S. Tsirkin [not found] ` <20180427191430-mutt-send-email-mst@kernel.org> @ 2018-04-27 19:36 ` Michael S. Tsirkin [not found] ` <20180427223636-mutt-send-email-mst@kernel.org> 3 siblings, 0 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-04-27 19:36 UTC (permalink / raw) To: Dmitry Vyukov Cc: Kevin Easton, KVM list, netdev, syzkaller-bugs, LKML, virtualization On Fri, Apr 27, 2018 at 06:11:31PM +0200, Dmitry Vyukov wrote: > On Fri, Apr 27, 2018 at 6:05 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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. How about a Tested-by tag then? > >> --- > >> 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 > > > > -- > > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20180427185501-mutt-send-email-mst%40kernel.org. > > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180427223636-mutt-send-email-mst@kernel.org>]
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180427223636-mutt-send-email-mst@kernel.org> @ 2018-04-29 8:10 ` Dmitry Vyukov via Virtualization 0 siblings, 0 replies; 21+ messages in thread From: Dmitry Vyukov via Virtualization @ 2018-04-29 8:10 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Kevin Easton, KVM list, netdev, syzkaller-bugs, LKML, virtualization On Fri, Apr 27, 2018 at 9:36 PM, Michael S. Tsirkin <mst@redhat.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. > > How about a Tested-by tag then? I didn't test either patch. >> >> --- >> >> 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 >> > >> > -- >> > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. >> > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. >> > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20180427185501-mutt-send-email-mst%40kernel.org. >> > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180428010756.GA27341@la.guarana.org>]
[parent not found: <20180428015106.GA27738@la.guarana.org>]
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180428015106.GA27738@la.guarana.org> @ 2018-04-28 2:23 ` Jason Wang 0 siblings, 0 replies; 21+ messages in thread From: Jason Wang @ 2018-04-28 2:23 UTC (permalink / raw) To: Kevin Easton, Michael S. Tsirkin Cc: netdev, syzkaller-bugs, linux-kernel, kvm, virtualization On 2018年04月28日 09:51, Kevin Easton wrote: > On Fri, Apr 27, 2018 at 09:07:56PM -0400, Kevin Easton wrote: >> On Fri, Apr 27, 2018 at 07:05:45PM +0300, Michael S. Tsirkin wrote: >>> On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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. >> No, I don't believe that is sufficient to fix the problem. > Scratch that, somehow I missed the "..and then we init just the > relevant field" part, sorry. > > There's still the padding after the vhost_iotlb_msg to consider. It's > named in the union but I don't think that's guaranteed to be initialised > when the iotlb member of the union is used to initialise things. > >> I didn't name the padding in my original patch because I wasn't sure >> if the padding actually exists on 32 bit architectures? > This might still be a conce Yes. print &((struct vhost_msg *)0)->iotlb $3 = (struct vhost_iotlb_msg *) 0x4 > > At the end of the day, zeroing 96 bytes (the full size of vhost_msg_node) > is pretty quick. > > - Kevin Right, and even if it may be used heavily in the data-path, zeroing is not the main delay in that path. Thanks _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180427154502.GA22544@la.guarana.org> 2018-04-27 16:05 ` [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node Michael S. Tsirkin [not found] ` <20180427185501-mutt-send-email-mst@kernel.org> @ 2018-05-07 13:03 ` Michael S. Tsirkin [not found] ` <20180507155534-mutt-send-email-mst@kernel.org> ` (2 subsequent siblings) 5 siblings, 0 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-05-07 13:03 UTC (permalink / raw) To: Kevin Easton; +Cc: kvm, netdev, syzkaller-bugs, linux-kernel, virtualization On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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 > --- > 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; Let's just init the msg though. OK it seems this is the best we can do for now, we need a new feature bit to fix it for 32 bit userspace on 64 bit kernels. Does the following help? Signed-off-by: Michael S. Tsirkin <mst@redhat.com> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index f3bd8e9..58d9aec 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2342,6 +2342,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); if (!node) return NULL; + + /* Make sure all padding within the structure is initialized. */ + memset(&node->msg, 0, sizeof node->msg); node->vq = vq; node->msg.type = type; return node; ^ permalink raw reply related [flat|nested] 21+ messages in thread
[parent not found: <20180507155534-mutt-send-email-mst@kernel.org>]
* Re: [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180507155534-mutt-send-email-mst@kernel.org> @ 2018-05-07 13:12 ` Dmitry Vyukov via Virtualization 0 siblings, 0 replies; 21+ messages in thread From: Dmitry Vyukov via Virtualization @ 2018-05-07 13:12 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Kevin Easton, KVM list, netdev, syzkaller-bugs, LKML, virtualization On Mon, May 7, 2018 at 3:03 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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 >> --- >> 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; > > > Let's just init the msg though. > > OK it seems this is the best we can do for now, > we need a new feature bit to fix it for 32 bit > userspace on 64 bit kernels. > > Does the following help? Hi Michael, You can ask reporter (syzbot) to test: https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches https://github.com/google/syzkaller/blob/master/docs/syzbot.md#kmsan-bugs > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index f3bd8e9..58d9aec 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2342,6 +2342,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); > if (!node) > return NULL; > + > + /* Make sure all padding within the structure is initialized. */ > + memset(&node->msg, 0, sizeof node->msg); > node->vq = vq; > node->msg.type = type; > return node; > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20180507155534-mutt-send-email-mst%40kernel.org. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180427154502.GA22544@la.guarana.org> ` (3 preceding siblings ...) [not found] ` <20180507155534-mutt-send-email-mst@kernel.org> @ 2018-05-29 22:19 ` Guenter Roeck [not found] ` <20180529221908.GA22742@roeck-us.net> 5 siblings, 0 replies; 21+ messages in thread From: Guenter Roeck @ 2018-05-29 22:19 UTC (permalink / raw) To: Kevin Easton Cc: kvm, Michael S. Tsirkin, netdev, syzkaller-bugs, linux-kernel, virtualization On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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 Is this patch going anywhere ? The patch fixes CVE-2018-1118. It would be useful to understand if and when this problem is going to be fixed. Thanks, Guenter > --- > 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; ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180529221908.GA22742@roeck-us.net>]
* Re: [net] vhost: Use kzalloc() to allocate vhost_msg_node [not found] ` <20180529221908.GA22742@roeck-us.net> @ 2018-05-30 3:01 ` Michael S. Tsirkin 2018-05-30 3:42 ` Guenter Roeck 2018-06-04 12:34 ` Dmitry Vyukov via Virtualization 0 siblings, 2 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-05-30 3:01 UTC (permalink / raw) To: Guenter Roeck Cc: Kevin Easton, kvm, netdev, syzkaller-bugs, linux-kernel, virtualization On Tue, May 29, 2018 at 03:19:08PM -0700, Guenter Roeck wrote: > On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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 > > Is this patch going anywhere ? > > The patch fixes CVE-2018-1118. It would be useful to understand if and when > this problem is going to be fixed. > > Thanks, > Guenter > > --- > > 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; As I pointed out, we don't need to init the whole structure. The proper fix is thus (I think) below. Could you use your testing infrastructure to confirm this fixes the issue? Thanks! Signed-off-by: Michael S. Tsirkin <mst@redhat.com> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index f3bd8e941224..58d9aec90afb 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2342,6 +2342,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); if (!node) return NULL; + + /* Make sure all padding within the structure is initialized. */ + memset(&node->msg, 0, sizeof node->msg); node->vq = vq; node->msg.type = type; return node; ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [net] vhost: Use kzalloc() to allocate vhost_msg_node 2018-05-30 3:01 ` Michael S. Tsirkin @ 2018-05-30 3:42 ` Guenter Roeck 2018-06-04 12:34 ` Dmitry Vyukov via Virtualization 1 sibling, 0 replies; 21+ messages in thread From: Guenter Roeck @ 2018-05-30 3:42 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Kevin Easton, kvm, netdev, syzkaller-bugs, linux-kernel, virtualization On 05/29/2018 08:01 PM, Michael S. Tsirkin wrote: > On Tue, May 29, 2018 at 03:19:08PM -0700, Guenter Roeck wrote: >> On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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 >> >> Is this patch going anywhere ? >> >> The patch fixes CVE-2018-1118. It would be useful to understand if and when >> this problem is going to be fixed. >> >> Thanks, >> Guenter >>> --- >>> 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; > > As I pointed out, we don't need to init the whole structure. The proper > fix is thus (I think) below. > > Could you use your testing infrastructure to confirm this fixes the issue? > Sorry, I don't have the means to test the fix. Guenter > Thanks! > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index f3bd8e941224..58d9aec90afb 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2342,6 +2342,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); > if (!node) > return NULL; > + > + /* Make sure all padding within the structure is initialized. */ > + memset(&node->msg, 0, sizeof node->msg); > node->vq = vq; > node->msg.type = type; > return node; > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net] vhost: Use kzalloc() to allocate vhost_msg_node 2018-05-30 3:01 ` Michael S. Tsirkin 2018-05-30 3:42 ` Guenter Roeck @ 2018-06-04 12:34 ` Dmitry Vyukov via Virtualization 1 sibling, 0 replies; 21+ messages in thread From: Dmitry Vyukov via Virtualization @ 2018-06-04 12:34 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Kevin Easton, KVM list, netdev, syzkaller-bugs, LKML, virtualization, Guenter Roeck On Wed, May 30, 2018 at 5:01 AM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Tue, May 29, 2018 at 03:19:08PM -0700, Guenter Roeck wrote: >> On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton 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 >> >> Is this patch going anywhere ? >> >> The patch fixes CVE-2018-1118. It would be useful to understand if and when >> this problem is going to be fixed. >> >> Thanks, >> Guenter >> > --- >> > 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; > > As I pointed out, we don't need to init the whole structure. The proper > fix is thus (I think) below. > > Could you use your testing infrastructure to confirm this fixes the issue? Hi Michael, syzbot is self-service, see: https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches > Thanks! > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index f3bd8e941224..58d9aec90afb 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2342,6 +2342,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); > if (!node) > return NULL; > + > + /* Make sure all padding within the structure is initialized. */ > + memset(&node->msg, 0, sizeof node->msg); > node->vq = vq; > node->msg.type = type; > return node; > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20180530055704-mutt-send-email-mst%40kernel.org. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <000000000000cf4578056ab12452@google.com>]
* Re: KMSAN: uninit-value in _copy_to_iter (2) [not found] ` <000000000000cf4578056ab12452@google.com> @ 2018-06-07 15:38 ` Michael S. Tsirkin 2018-06-07 16:25 ` Dmitry Vyukov via Virtualization ` (2 more replies) 2018-06-07 17:10 ` Michael S. Tsirkin 1 sibling, 3 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-06-07 15:38 UTC (permalink / raw) To: syzbot Cc: willemb, avagin, kvm, netdev, matthew, linux-kernel, mingo, syzkaller-bugs, edumazet, viro, dingtianhong, pabeni, virtualization, davem, elena.reshetova #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617 Subject: vhost: fix info leak Fixes: CVE-2018-1118 Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index f0be5f35ab28..9beefa6ed1ce 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); if (!node) return NULL; + + /* Make sure all padding within the structure is initialized. */ + memset(&node->msg, 0, sizeof node->msg); node->vq = vq; node->msg.type = type; return node; ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: KMSAN: uninit-value in _copy_to_iter (2) 2018-06-07 15:38 ` KMSAN: uninit-value in _copy_to_iter (2) Michael S. Tsirkin @ 2018-06-07 16:25 ` Dmitry Vyukov via Virtualization 2018-06-07 17:43 ` Al Viro [not found] ` <20180607174355.GR30522@ZenIV.linux.org.uk> 2 siblings, 0 replies; 21+ messages in thread From: Dmitry Vyukov via Virtualization @ 2018-06-07 16:25 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Willem de Bruijn, avagin, KVM list, netdev, Matthew Dawson, LKML, Ingo Molnar, syzkaller-bugs, Eric Dumazet, Al Viro, Ding Tianhong, syzbot, Paolo Abeni, virtualization, David Miller, Reshetova, Elena [-- Attachment #1: Type: text/plain, Size: 1599 bytes --] On Thu, Jun 7, 2018 at 5:38 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617 Hi Michael, We need: #syz test: https://github.com/google/kmsan.git master here. Please see https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches for more info. Please also add the Reported-by tag when mailing the patch for review. Thanks > Subject: vhost: fix info leak > > Fixes: CVE-2018-1118 > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index f0be5f35ab28..9beefa6ed1ce 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); > if (!node) > return NULL; > + > + /* Make sure all padding within the structure is initialized. */ > + memset(&node->msg, 0, sizeof node->msg); > node->vq = vq; > node->msg.type = type; > return node; > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20180607183627-mutt-send-email-mst%40kernel.org. > For more options, visit https://groups.google.com/d/optout. [-- Attachment #2: patch --] [-- Type: application/octet-stream, Size: 518 bytes --] diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index f0be5f35ab28..9beefa6ed1ce 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); if (!node) return NULL; + + /* Make sure all padding within the structure is initialized. */ + memset(&node->msg, 0, sizeof node->msg); node->vq = vq; node->msg.type = type; return node; [-- Attachment #3: Type: text/plain, Size: 183 bytes --] _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: KMSAN: uninit-value in _copy_to_iter (2) 2018-06-07 15:38 ` KMSAN: uninit-value in _copy_to_iter (2) Michael S. Tsirkin 2018-06-07 16:25 ` Dmitry Vyukov via Virtualization @ 2018-06-07 17:43 ` Al Viro [not found] ` <20180607174355.GR30522@ZenIV.linux.org.uk> 2 siblings, 0 replies; 21+ messages in thread From: Al Viro @ 2018-06-07 17:43 UTC (permalink / raw) To: Michael S. Tsirkin Cc: willemb, avagin, kvm, netdev, matthew, linux-kernel, mingo, syzkaller-bugs, edumazet, dingtianhong, syzbot, pabeni, virtualization, davem, elena.reshetova On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote: > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617 > > Subject: vhost: fix info leak > > Fixes: CVE-2018-1118 > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index f0be5f35ab28..9beefa6ed1ce 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); > if (!node) > return NULL; > + > + /* Make sure all padding within the structure is initialized. */ > + memset(&node->msg, 0, sizeof node->msg); Umm... Maybe kzalloc(), then? You have struct vhost_msg_node { struct vhost_msg msg; struct vhost_virtqueue *vq; struct list_head node; }; and that's what, 68 bytes in msg, then either 4 bytes pointer or 4 bytes padding + 8 bytes pointer, then two pointers? How much does explicit partial memset() save you here? > node->vq = vq; > node->msg.type = type; > return node; ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180607174355.GR30522@ZenIV.linux.org.uk>]
* Re: KMSAN: uninit-value in _copy_to_iter (2) [not found] ` <20180607174355.GR30522@ZenIV.linux.org.uk> @ 2018-06-07 17:59 ` Michael S. Tsirkin [not found] ` <20180607205611-mutt-send-email-mst@kernel.org> 1 sibling, 0 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-06-07 17:59 UTC (permalink / raw) To: Al Viro Cc: willemb, avagin, kvm, netdev, matthew, linux-kernel, mingo, syzkaller-bugs, edumazet, dingtianhong, syzbot, pabeni, virtualization, davem, elena.reshetova On Thu, Jun 07, 2018 at 06:43:55PM +0100, Al Viro wrote: > On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote: > > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617 > > > > Subject: vhost: fix info leak > > > > Fixes: CVE-2018-1118 > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > index f0be5f35ab28..9beefa6ed1ce 100644 > > --- a/drivers/vhost/vhost.c > > +++ b/drivers/vhost/vhost.c > > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) > > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); > > if (!node) > > return NULL; > > + > > + /* Make sure all padding within the structure is initialized. */ > > + memset(&node->msg, 0, sizeof node->msg); > > Umm... Maybe kzalloc(), then? You have > > struct vhost_msg_node { > struct vhost_msg msg; > struct vhost_virtqueue *vq; > struct list_head node; > }; > > and that's what, 68 bytes in msg, then either 4 bytes pointer or > 4 bytes padding + 8 bytes pointer, then two pointers? How much > does explicit partial memset() save you here? Yes but 0 isn't a nop here so if this struct is used without a sensible initialization, it will crash elsewhere. I prefer KASAN to catch such uses. > > node->vq = vq; > > node->msg.type = type; > > return node; ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180607205611-mutt-send-email-mst@kernel.org>]
* Re: KMSAN: uninit-value in _copy_to_iter (2) [not found] ` <20180607205611-mutt-send-email-mst@kernel.org> @ 2018-06-07 18:04 ` Al Viro [not found] ` <20180607180449.GS30522@ZenIV.linux.org.uk> 1 sibling, 0 replies; 21+ messages in thread From: Al Viro @ 2018-06-07 18:04 UTC (permalink / raw) To: Michael S. Tsirkin Cc: willemb, avagin, kvm, netdev, matthew, linux-kernel, mingo, syzkaller-bugs, edumazet, dingtianhong, syzbot, pabeni, virtualization, davem, elena.reshetova On Thu, Jun 07, 2018 at 08:59:06PM +0300, Michael S. Tsirkin wrote: > On Thu, Jun 07, 2018 at 06:43:55PM +0100, Al Viro wrote: > > On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote: > > > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617 > > > > > > Subject: vhost: fix info leak > > > > > > Fixes: CVE-2018-1118 > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > --- > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > > index f0be5f35ab28..9beefa6ed1ce 100644 > > > --- a/drivers/vhost/vhost.c > > > +++ b/drivers/vhost/vhost.c > > > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) > > > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); > > > if (!node) > > > return NULL; > > > + > > > + /* Make sure all padding within the structure is initialized. */ > > > + memset(&node->msg, 0, sizeof node->msg); > > > > Umm... Maybe kzalloc(), then? You have > > > > struct vhost_msg_node { > > struct vhost_msg msg; > > struct vhost_virtqueue *vq; > > struct list_head node; > > }; > > > > and that's what, 68 bytes in msg, then either 4 bytes pointer or > > 4 bytes padding + 8 bytes pointer, then two pointers? How much > > does explicit partial memset() save you here? > > Yes but 0 isn't a nop here so if this struct is used without > a sensible initialization, it will crash elsewhere. > I prefer KASAN to catch such uses. > > > > > node->vq = vq; > > > node->msg.type = type; IDGI - what would your variant catch that kzalloc + 2 assignments won't? Accesses to uninitialized ->node? Because that's the only difference in what is and is not initialized between those variants... ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20180607180449.GS30522@ZenIV.linux.org.uk>]
* Re: KMSAN: uninit-value in _copy_to_iter (2) [not found] ` <20180607180449.GS30522@ZenIV.linux.org.uk> @ 2018-06-07 19:29 ` Michael S. Tsirkin 0 siblings, 0 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-06-07 19:29 UTC (permalink / raw) To: Al Viro Cc: willemb, avagin, kvm, netdev, matthew, linux-kernel, mingo, syzkaller-bugs, edumazet, dingtianhong, syzbot, pabeni, virtualization, davem, elena.reshetova On Thu, Jun 07, 2018 at 07:04:49PM +0100, Al Viro wrote: > On Thu, Jun 07, 2018 at 08:59:06PM +0300, Michael S. Tsirkin wrote: > > On Thu, Jun 07, 2018 at 06:43:55PM +0100, Al Viro wrote: > > > On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote: > > > > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617 > > > > > > > > Subject: vhost: fix info leak > > > > > > > > Fixes: CVE-2018-1118 > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > --- > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > > > index f0be5f35ab28..9beefa6ed1ce 100644 > > > > --- a/drivers/vhost/vhost.c > > > > +++ b/drivers/vhost/vhost.c > > > > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) > > > > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); > > > > if (!node) > > > > return NULL; > > > > + > > > > + /* Make sure all padding within the structure is initialized. */ > > > > + memset(&node->msg, 0, sizeof node->msg); > > > > > > Umm... Maybe kzalloc(), then? You have > > > > > > struct vhost_msg_node { > > > struct vhost_msg msg; > > > struct vhost_virtqueue *vq; > > > struct list_head node; > > > }; > > > > > > and that's what, 68 bytes in msg, then either 4 bytes pointer or > > > 4 bytes padding + 8 bytes pointer, then two pointers? How much > > > does explicit partial memset() save you here? > > > > Yes but 0 isn't a nop here so if this struct is used without > > a sensible initialization, it will crash elsewhere. > > I prefer KASAN to catch such uses. > > > > > > > > node->vq = vq; > > > > node->msg.type = type; > > IDGI - what would your variant catch that kzalloc + 2 assignments won't? > Accesses to uninitialized ->node? Because that's the only difference in > what is and is not initialized between those variants... For now yes but we'll likely add more fields in this structure down the road, which is where I'd expect new bugs to come from. -- MST ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: KMSAN: uninit-value in _copy_to_iter (2) [not found] ` <000000000000cf4578056ab12452@google.com> 2018-06-07 15:38 ` KMSAN: uninit-value in _copy_to_iter (2) Michael S. Tsirkin @ 2018-06-07 17:10 ` Michael S. Tsirkin 1 sibling, 0 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2018-06-07 17:10 UTC (permalink / raw) To: syzbot Cc: willemb, avagin, kvm, netdev, matthew, linux-kernel, mingo, syzkaller-bugs, edumazet, viro, dingtianhong, pabeni, virtualization, davem, elena.reshetova #syz test: https://github.com/google/kmsan.git master Subject: vhost: fix info leak Fixes: CVE-2018-1118 Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index f0be5f35ab28..9beefa6ed1ce 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type) struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL); if (!node) return NULL; + + /* Make sure all padding within the structure is initialized. */ + memset(&node->msg, 0, sizeof node->msg); node->vq = vq; node->msg.type = type; return node; ^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-06-07 19:29 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <000000000000a5b2b1056a86e98c@google.com>
[not found] ` <20180427154502.GA22544@la.guarana.org>
2018-04-27 16:05 ` [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node Michael S. Tsirkin
[not found] ` <20180427185501-mutt-send-email-mst@kernel.org>
2018-04-27 16:11 ` Dmitry Vyukov via Virtualization
2018-04-27 16:15 ` Michael S. Tsirkin
[not found] ` <20180427191430-mutt-send-email-mst@kernel.org>
2018-04-27 16:25 ` Dmitry Vyukov via Virtualization
[not found] ` <CACT4Y+bzWiPvV+pVvys4v8CwUhF7iYVskxn_yeo6ztN5uKA0VA@mail.gmail.com>
2018-04-27 16:29 ` Dmitry Vyukov via Virtualization
2018-04-27 19:36 ` Michael S. Tsirkin
[not found] ` <20180427223636-mutt-send-email-mst@kernel.org>
2018-04-29 8:10 ` Dmitry Vyukov via Virtualization
[not found] ` <20180428010756.GA27341@la.guarana.org>
[not found] ` <20180428015106.GA27738@la.guarana.org>
2018-04-28 2:23 ` Jason Wang
2018-05-07 13:03 ` Michael S. Tsirkin
[not found] ` <20180507155534-mutt-send-email-mst@kernel.org>
2018-05-07 13:12 ` Dmitry Vyukov via Virtualization
2018-05-29 22:19 ` [net] " Guenter Roeck
[not found] ` <20180529221908.GA22742@roeck-us.net>
2018-05-30 3:01 ` Michael S. Tsirkin
2018-05-30 3:42 ` Guenter Roeck
2018-06-04 12:34 ` Dmitry Vyukov via Virtualization
[not found] ` <000000000000cf4578056ab12452@google.com>
2018-06-07 15:38 ` KMSAN: uninit-value in _copy_to_iter (2) Michael S. Tsirkin
2018-06-07 16:25 ` Dmitry Vyukov via Virtualization
2018-06-07 17:43 ` Al Viro
[not found] ` <20180607174355.GR30522@ZenIV.linux.org.uk>
2018-06-07 17:59 ` Michael S. Tsirkin
[not found] ` <20180607205611-mutt-send-email-mst@kernel.org>
2018-06-07 18:04 ` Al Viro
[not found] ` <20180607180449.GS30522@ZenIV.linux.org.uk>
2018-06-07 19:29 ` Michael S. Tsirkin
2018-06-07 17:10 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).