2020-12-01 10:11:11

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v6 06/18] virt: acrn: Introduce VM management interfaces

On Tue, Dec 01, 2020 at 05:38:41PM +0800, [email protected] wrote:
> + vm_param->reserved0 = 0;
> + vm_param->reserved1 = 0;

NO!

This means that userspace can put whatever crud they want in those
fields, and you will happily zero it out. Then, when those reserved
fields are wanted to be used in the future, you will take those values
from userspace and accept them as a valid value. But, since userspace
was sending crud before, now you will take that crud and do something
with it.

TEST IT to verify that it is zero, that way userspace gets it right the
first time, and you don't get it wrong later, as you can not change it
later.

thaqnks,

greg k-h


2020-12-02 02:17:32

by Shuo Liu

[permalink] [raw]
Subject: Re: [PATCH v6 06/18] virt: acrn: Introduce VM management interfaces

On Tue 1.Dec'20 at 11:09:47 +0100, Greg Kroah-Hartman wrote:
>On Tue, Dec 01, 2020 at 05:38:41PM +0800, [email protected] wrote:
>> + vm_param->reserved0 = 0;
>> + vm_param->reserved1 = 0;
>
>NO!
>
>This means that userspace can put whatever crud they want in those
>fields, and you will happily zero it out. Then, when those reserved
>fields are wanted to be used in the future, you will take those values
>from userspace and accept them as a valid value. But, since userspace
>was sending crud before, now you will take that crud and do something
>with it.
>
>TEST IT to verify that it is zero, that way userspace gets it right the
>first time, and you don't get it wrong later, as you can not change it
>later.

OK. Thanks for the elaboration. I will test it and return -EINVAL if
it is not zero.

2020-12-15 09:58:02

by Shuo Liu

[permalink] [raw]
Subject: Re: [PATCH v6 06/18] virt: acrn: Introduce VM management interfaces

On Wed 2.Dec'20 at 10:14:29 +0800, Shuo A Liu wrote:
>On Tue 1.Dec'20 at 11:09:47 +0100, Greg Kroah-Hartman wrote:
>>On Tue, Dec 01, 2020 at 05:38:41PM +0800, [email protected] wrote:
>>>+ vm_param->reserved0 = 0;
>>>+ vm_param->reserved1 = 0;
>>
>>NO!
>>
>>This means that userspace can put whatever crud they want in those
>>fields, and you will happily zero it out. Then, when those reserved
>>fields are wanted to be used in the future, you will take those values
>>from userspace and accept them as a valid value. But, since userspace
>>was sending crud before, now you will take that crud and do something
>>with it.
>>
>>TEST IT to verify that it is zero, that way userspace gets it right the
>>first time, and you don't get it wrong later, as you can not change it
>>later.
>
>OK. Thanks for the elaboration. I will test it and return -EINVAL if
>it is not zero.
>

Hi Greg,

Would you like to review other patches in this series on this version?

This version got only this one comment, so doesn't have too much update.
I can send out a new version if you prefer to.

Thanks for review.

shuo

2020-12-15 10:03:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v6 06/18] virt: acrn: Introduce VM management interfaces

On Tue, Dec 15, 2020 at 05:52:59PM +0800, Shuo A Liu wrote:
> On Wed 2.Dec'20 at 10:14:29 +0800, Shuo A Liu wrote:
> > On Tue 1.Dec'20 at 11:09:47 +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2020 at 05:38:41PM +0800, [email protected] wrote:
> > > > + vm_param->reserved0 = 0;
> > > > + vm_param->reserved1 = 0;
> > >
> > > NO!
> > >
> > > This means that userspace can put whatever crud they want in those
> > > fields, and you will happily zero it out. Then, when those reserved
> > > fields are wanted to be used in the future, you will take those values
> > > from userspace and accept them as a valid value. But, since userspace
> > > was sending crud before, now you will take that crud and do something
> > > with it.
> > >
> > > TEST IT to verify that it is zero, that way userspace gets it right the
> > > first time, and you don't get it wrong later, as you can not change it
> > > later.
> >
> > OK. Thanks for the elaboration. I will test it and return -EINVAL if
> > it is not zero.
> >
>
> Hi Greg,
>
> Would you like to review other patches in this series on this version?

Nope, it's the middle of the merge window, I can't do anything with any
new patches until after 5.11-rc1 is out. So I suggest you fix up the
current issues and send a new patch series once 5.11-rc1 is released.

thanks,

greg k-h

2020-12-15 10:06:42

by Shuo Liu

[permalink] [raw]
Subject: Re: [PATCH v6 06/18] virt: acrn: Introduce VM management interfaces

On Tue 15.Dec'20 at 11:00:57 +0100, Greg Kroah-Hartman wrote:
>On Tue, Dec 15, 2020 at 05:52:59PM +0800, Shuo A Liu wrote:
>> On Wed 2.Dec'20 at 10:14:29 +0800, Shuo A Liu wrote:
>> > On Tue 1.Dec'20 at 11:09:47 +0100, Greg Kroah-Hartman wrote:
>> > > On Tue, Dec 01, 2020 at 05:38:41PM +0800, [email protected] wrote:
>> > > > + vm_param->reserved0 = 0;
>> > > > + vm_param->reserved1 = 0;
>> > >
>> > > NO!
>> > >
>> > > This means that userspace can put whatever crud they want in those
>> > > fields, and you will happily zero it out. Then, when those reserved
>> > > fields are wanted to be used in the future, you will take those values
>> > > from userspace and accept them as a valid value. But, since userspace
>> > > was sending crud before, now you will take that crud and do something
>> > > with it.
>> > >
>> > > TEST IT to verify that it is zero, that way userspace gets it right the
>> > > first time, and you don't get it wrong later, as you can not change it
>> > > later.
>> >
>> > OK. Thanks for the elaboration. I will test it and return -EINVAL if
>> > it is not zero.
>> >
>>
>> Hi Greg,
>>
>> Would you like to review other patches in this series on this version?
>
>Nope, it's the middle of the merge window, I can't do anything with any
>new patches until after 5.11-rc1 is out. So I suggest you fix up the
>current issues and send a new patch series once 5.11-rc1 is released.

Got it, thanks!

2021-01-05 14:03:50

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v6 06/18] virt: acrn: Introduce VM management interfaces

On Tue, Dec 15, 2020 at 06:02:51PM +0800, Shuo A Liu wrote:
> On Tue 15.Dec'20 at 11:00:57 +0100, Greg Kroah-Hartman wrote:
> > On Tue, Dec 15, 2020 at 05:52:59PM +0800, Shuo A Liu wrote:
> > > On Wed 2.Dec'20 at 10:14:29 +0800, Shuo A Liu wrote:
> > > > On Tue 1.Dec'20 at 11:09:47 +0100, Greg Kroah-Hartman wrote:
> > > > > On Tue, Dec 01, 2020 at 05:38:41PM +0800, [email protected] wrote:
> > > > > > + vm_param->reserved0 = 0;
> > > > > > + vm_param->reserved1 = 0;
> > > > >
> > > > > NO!
> > > > >
> > > > > This means that userspace can put whatever crud they want in those
> > > > > fields, and you will happily zero it out. Then, when those reserved
> > > > > fields are wanted to be used in the future, you will take those values
> > > > > from userspace and accept them as a valid value. But, since userspace
> > > > > was sending crud before, now you will take that crud and do something
> > > > > with it.
> > > > >
> > > > > TEST IT to verify that it is zero, that way userspace gets it right the
> > > > > first time, and you don't get it wrong later, as you can not change it
> > > > > later.
> > > >
> > > > OK. Thanks for the elaboration. I will test it and return -EINVAL if
> > > > it is not zero.
> > > >
> > >
> > > Hi Greg,
> > >
> > > Would you like to review other patches in this series on this version?
> >
> > Nope, it's the middle of the merge window, I can't do anything with any
> > new patches until after 5.11-rc1 is out. So I suggest you fix up the
> > current issues and send a new patch series once 5.11-rc1 is released.
>
> Got it, thanks!

Did this ever happen? I don't see a new series anywhere, do you have a
lore.kernel.org link?

thanks,

greg k-h

2021-01-06 07:58:05

by Shuo Liu

[permalink] [raw]
Subject: Re: [PATCH v6 06/18] virt: acrn: Introduce VM management interfaces

On Tue 5.Jan'21 at 15:03:06 +0100, Greg Kroah-Hartman wrote:
>On Tue, Dec 15, 2020 at 06:02:51PM +0800, Shuo A Liu wrote:
>> On Tue 15.Dec'20 at 11:00:57 +0100, Greg Kroah-Hartman wrote:
>> > On Tue, Dec 15, 2020 at 05:52:59PM +0800, Shuo A Liu wrote:
>> > > On Wed 2.Dec'20 at 10:14:29 +0800, Shuo A Liu wrote:
>> > > > On Tue 1.Dec'20 at 11:09:47 +0100, Greg Kroah-Hartman wrote:
>> > > > > On Tue, Dec 01, 2020 at 05:38:41PM +0800, [email protected] wrote:
>> > > > > > + vm_param->reserved0 = 0;
>> > > > > > + vm_param->reserved1 = 0;
>> > > > >
>> > > > > NO!
>> > > > >
>> > > > > This means that userspace can put whatever crud they want in those
>> > > > > fields, and you will happily zero it out. Then, when those reserved
>> > > > > fields are wanted to be used in the future, you will take those values
>> > > > > from userspace and accept them as a valid value. But, since userspace
>> > > > > was sending crud before, now you will take that crud and do something
>> > > > > with it.
>> > > > >
>> > > > > TEST IT to verify that it is zero, that way userspace gets it right the
>> > > > > first time, and you don't get it wrong later, as you can not change it
>> > > > > later.
>> > > >
>> > > > OK. Thanks for the elaboration. I will test it and return -EINVAL if
>> > > > it is not zero.
>> > > >
>> > >
>> > > Hi Greg,
>> > >
>> > > Would you like to review other patches in this series on this version?
>> >
>> > Nope, it's the middle of the merge window, I can't do anything with any
>> > new patches until after 5.11-rc1 is out. So I suggest you fix up the
>> > current issues and send a new patch series once 5.11-rc1 is released.
>>
>> Got it, thanks!
>
>Did this ever happen? I don't see a new series anywhere, do you have a
>lore.kernel.org link?

Just back to work. :)

Sent new series already. The link is
https://lore.kernel.org/lkml/[email protected]/

Thanks
shuo