linux-kernel - RE: [PATCH v4 1/4] Tools: hv: Reopen the devices if read() or write() returns errors

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] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 26 Jan 2020 06:02:43 +0000
From:   Michael Kelley <mikelley@...rosoft.com>
To:     Dexuan Cui <decui@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "sashal@...nel.org" <sashal@...nel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        vkuznets <vkuznets@...hat.com>
Subject: RE: [PATCH v4 1/4] Tools: hv: Reopen the devices if read() or write()
 returns errors

From: Dexuan Cui <decui@...rosoft.com> Sent: Saturday, January 25, 2020 9:50 PM
> 
> The state machine in the hv_utils driver can run out of order in some
> corner cases, e.g. if the kvp daemon doesn't call write() fast enough
> due to some reason, kvp_timeout_func() can run first and move the state
> to HVUTIL_READY; next, when kvp_on_msg() is called it returns -EINVAL
> since kvp_transaction.state is smaller than HVUTIL_USERSPACE_REQ; later,
> the daemon's write() gets an error -EINVAL, and the daemon will exit().
> 
> We can reproduce the issue by sending a SIGSTOP signal to the daemon, wait
> for 1 minute, and send a SIGCONT signal to the daemon: the daemon will
> exit() quickly.
> 
> We can fix the issue by forcing a reset of the device (which means the
> daemon can close() and open() the device again) and doing extra necessary
> clean-up.
> 
> Signed-off-by: Dexuan Cui <decui@...rosoft.com>
> 
> ---
> Changes in v2:
>     This is actually a new patch that makes the daemons more robust.
> 
> Changes in v3 (I addressed Michael's comments):
>     Don't reset target_fd, since that's unnecessary.
>     Reset target_fname by: target_fname[0] = '\0';
>     Added the missing "fs_frozen = true;" in vss_operate().
>     Just after reopen_vss_fd: if vss_operate(VSS_OP_THAW) can not clear
>         fs_frozen due to an error, we just exit().
>     Added comments.
> 
> Changes in v4 (Thanks to Michael!):
>     Added the omitted "int fcopy_fd = -1" and
>     "
>      if (fcopy_fd != -1)
>                close(fcopy_fd);
>     "
> 
>  tools/hv/hv_fcopy_daemon.c | 37 ++++++++++++++++++++++++----
>  tools/hv/hv_kvp_daemon.c   | 36 ++++++++++++++++------------
>  tools/hv/hv_vss_daemon.c   | 49 +++++++++++++++++++++++++++++---------
>  3 files changed, 91 insertions(+), 31 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@...rosoft.com>