linux-kernel - RE: [PATCH v2 3/4] hv_utils: Support host-initiated hibernation request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <HK0P153MB014846CCBBA8C989EEC8819BBF0F0@HK0P153MB0148.APCP153.PROD.OUTLOOK.COM>
Date:   Thu, 23 Jan 2020 08:11:19 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Michael Kelley <mikelley@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "sashal@...nel.org" <sashal@...nel.org>,
        Sasha Levin <Alexander.Levin@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        vkuznets <vkuznets@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 3/4] hv_utils: Support host-initiated hibernation
 request

> From: Michael Kelley <mikelley@...rosoft.com>
> > --- a/drivers/hv/hv_util.c
> > +++ b/drivers/hv/hv_util.c
> > ...
> >  static const int sd_versions[] = {
> >  	SD_VERSION_3_1,
> > +	SD_VERSION_3_2,
> 
> I think these versions need to listed in descending order, so the new
> SD_VERSION_3_2 should be listed first.  Otherwise a Hyper-V host that
> supports both 3.1 and 3.2 might match on 3.1 first without ever checking
> for a match with 3.2.

Thanks! This is a rebasing mistake. Will fix it.

> > @@ -187,6 +226,17 @@ static void shutdown_onchannelcallback(void
> *context)
> >
> >  				schedule_work(&restart_work);
> >  				break;
> > +			case 4:
> > +			case 5:
> 
> As before, I'm wondering about the interpretation of these numbers.

Will add a comment.
 
> > +				pr_info("Hibernation request received\n");
> > +
> > +				if (execute_hibernate) {
> > +					icmsghdrp->status = HV_S_OK;
> > +					schedule_work(&hibernate_context.work);
> 
> Same comment here about the ordering of the schedule_work() call and the
> sending of the response message.  Seems like the code should be consistent
> for all three cases -- shutdown, restart, and hibernate.

I agree. Will fix this.

Thanks,
-- Dexuan