From: Oliver Kurth Date: Fri, 15 Sep 2017 18:22:56 +0000 (-0700) Subject: Hgfs Fuse Client: fix for backdoor not available X-Git-Tag: stable-10.2.0~609 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=675c07bb089fb4cfd07932563da21b251d12c70b;p=thirdparty%2Fopen-vm-tools.git Hgfs Fuse Client: fix for backdoor not available The client dropped errors for failing to open the backdoor returning FALSE, when the HGFS server was not enabled which migrated to a protocol error. This could mislead users when mounting the FUSE file system when the feature was disabled into thinking there was a different issue when actually enabling the feature was required before mounting. It was not a protocol mismatch between the client and server. The main entry point now logs a message for the user if the transport init fails and so the mount does not occur. i The main issue to prevent the mount being created when the feature is disabled is addressed by the transport init function now trying to open the transport and generate an initial connection. This fails when the HGFS server is disabled because the feature is disabled. Hence, there will not be a mount made and an error is reported to users. If a mount is created when the feature is enabled and then later disabled via the UI settings, the mount will still exist as there is no umount made at that point. This will be addressed in later fixes. However, it should be noted that the leftover mount which is now not accessible reports that the "Transport endpoint is not connected" instead of the previous error of "protocol error". --- diff --git a/open-vm-tools/vmhgfs-fuse/bdhandler.c b/open-vm-tools/vmhgfs-fuse/bdhandler.c index 62c489fd7..968e52af8 100644 --- a/open-vm-tools/vmhgfs-fuse/bdhandler.c +++ b/open-vm-tools/vmhgfs-fuse/bdhandler.c @@ -45,7 +45,7 @@ static HgfsTransportChannel bdChannel; * Open the backdoor in an idempotent way. * * Results: - * TRUE on success, FALSE on failure. + * Existing or updated channel status, HGFS_CHANNEL_CONNECTED on success. * * Side effects: * None @@ -53,36 +53,35 @@ static HgfsTransportChannel bdChannel; *----------------------------------------------------------------------------- */ -static Bool +static HgfsChannelStatus HgfsBdChannelOpen(HgfsTransportChannel *channel) // IN: Channel { - Bool ret; - pthread_mutex_lock(&channel->connLock); switch (channel->status) { case HGFS_CHANNEL_UNINITIALIZED: - ret = FALSE; + LOG(8, ("Backdoor uninitialized.\n")); break; case HGFS_CHANNEL_CONNECTED: - ret = TRUE; + LOG(8, ("Backdoor already connected.\n")); break; case HGFS_CHANNEL_NOTCONNECTED: if (HgfsBd_OpenBackdoor((RpcOut **)&channel->priv)) { - LOG(8, ("Backdoor opened.\n")); - bdChannel.status = HGFS_CHANNEL_CONNECTED; - ret = TRUE; + LOG(8, ("Backdoor opened and connected.\n")); + channel->status = HGFS_CHANNEL_CONNECTED; ASSERT(channel->priv != NULL); } else { - ret = FALSE; + LOG(8, ("ERROR: Backdoor cannot connect.\n")); } break; default: ASSERT(0); /* Not reached. */ - ret = FALSE; + LOG(2, ("ERROR: Backdoor status %d is unknown resetting.\n", + channel->status)); + channel->status = HGFS_CHANNEL_UNINITIALIZED; } pthread_mutex_unlock(&channel->connLock); - return ret; + return channel->status; } diff --git a/open-vm-tools/vmhgfs-fuse/main.c b/open-vm-tools/vmhgfs-fuse/main.c index 241ce68a7..26698b690 100644 --- a/open-vm-tools/vmhgfs-fuse/main.c +++ b/open-vm-tools/vmhgfs-fuse/main.c @@ -1371,7 +1371,7 @@ main(int argc, //IN: Argument count HgfsResetOps(); res = HgfsTransportInit(); if (res != 0) { - LOG(4, ("Main: Error in HgfsTransportInit %d\n", res)); + fprintf(stderr, "Error %d cannot open connection!\n", res); return res; } HgfsInitCache(); diff --git a/open-vm-tools/vmhgfs-fuse/transport.c b/open-vm-tools/vmhgfs-fuse/transport.c index 4878d3689..9f3f4325b 100644 --- a/open-vm-tools/vmhgfs-fuse/transport.c +++ b/open-vm-tools/vmhgfs-fuse/transport.c @@ -40,9 +40,11 @@ static HgfsTransportChannel *gHgfsActiveChannel; /* Current active channel. */ static pthread_mutex_t gHgfsActiveChannelLock; /* Current active channel lock. */ +static Bool gHgfsActiveChannelLockInited; static struct list_head gHgfsPendingRequests; /* Pending requests queue. */ static pthread_mutex_t gHgfsPendingRequestsLock; /* Pending requests queue lock. */ +static Bool gHgfsPendingRequestsLockInited; #define HgfsRequestId(req) ((HgfsRequest *)req)->id @@ -61,7 +63,7 @@ static void HgfsTransportChannelClose(HgfsTransportChannel **channel); * Open a new workable channel. * * Results: - * TRUE on success and the new channel, otherwise FALSE and NULL. + * 0 on success and the new channel, otherwise -ENOTCONN and NULL. * * Side effects: * None @@ -69,17 +71,18 @@ static void HgfsTransportChannelClose(HgfsTransportChannel **channel); *---------------------------------------------------------------------- */ -static Bool +static int HgfsTransportChannelOpen(HgfsTransportChannel **channel) // IN: active channel { - Bool result = FALSE; + int result = 0; *channel = HgfsBdChannelInit(); if (NULL != *channel) { - if ((*channel)->ops.open(*channel)) { - result = TRUE; - } else { + HgfsChannelStatus status = (*channel)->ops.open(*channel); + if (status != HGFS_CHANNEL_CONNECTED) { HgfsTransportChannelClose(channel); + result = -ENOTCONN; + *channel = NULL; } } @@ -138,9 +141,14 @@ static Bool HgfsTransportChannelReset(HgfsTransportChannel **channel) // IN: active channel { Bool ret = FALSE; + int openResult; + HgfsTransportChannelClose(channel); - ret = HgfsTransportChannelOpen(channel); - LOG(8, ("Result: %s.\n", ret ? "TRUE" : "FALSE")); + openResult = HgfsTransportChannelOpen(channel); + if (openResult == 0) { + ret = TRUE; + } + LOG(8, ("Result: %d: %s.\n",openResult, ret ? "TRUE" : "FALSE")); return ret; } @@ -321,10 +329,11 @@ HgfsTransportSendRequest(HgfsReq *req) // IN: Request to send pthread_mutex_lock(&gHgfsActiveChannelLock); /* Try opening the channel. */ - if (NULL == gHgfsActiveChannel && - !HgfsTransportChannelOpen(&gHgfsActiveChannel)) { - pthread_mutex_unlock(&gHgfsActiveChannelLock); - return -EPROTO; + if (NULL == gHgfsActiveChannel) { + ret = HgfsTransportChannelOpen(&gHgfsActiveChannel); + if (ret != 0) { + goto exit; + } } ASSERT(gHgfsActiveChannel->ops.send); @@ -335,12 +344,12 @@ HgfsTransportSendRequest(HgfsReq *req) // IN: Request to send if (ret < 0) { LOG(4, ("Send failed, status = %d. Try reopening the channel ...\n", ret)); - if (gHgfsActiveChannel->ops.open(gHgfsActiveChannel) && - HgfsTransportChannelReset(&gHgfsActiveChannel)) { + if (HgfsTransportChannelReset(&gHgfsActiveChannel)) { ret = gHgfsActiveChannel->ops.send(gHgfsActiveChannel, req); } } +exit: ASSERT(req->state == HGFS_REQ_STATE_COMPLETED || req->state == HGFS_REQ_STATE_SUBMITTED || req->state == HGFS_REQ_STATE_UNSENT); @@ -366,7 +375,7 @@ HgfsTransportSendRequest(HgfsReq *req) // IN: Request to send * connected socket. * * Results: - * Zero on success and negative one on failure. + * Zero on success and negative error on failure. * * Side effects: * None @@ -378,17 +387,33 @@ int HgfsTransportInit(void) { int res; + + gHgfsActiveChannel = NULL; + gHgfsPendingRequestsLockInited = FALSE; + gHgfsActiveChannelLockInited = FALSE; INIT_LIST_HEAD(&gHgfsPendingRequests); + res = pthread_mutex_init(&gHgfsPendingRequestsLock, NULL); - if( res != 0) { - return -1; + if (res != 0) { + res = -res; + goto exit; } + gHgfsPendingRequestsLockInited = TRUE; + res = pthread_mutex_init(&gHgfsActiveChannelLock, NULL); - if( res != 0) { - return -1; + if (res != 0) { + res = -res; + goto exit; } - gHgfsActiveChannel = NULL; - return 0; + gHgfsActiveChannelLockInited = TRUE; + + res = HgfsTransportChannelOpen(&gHgfsActiveChannel); + +exit: + if (res != 0) { + HgfsTransportExit(); + } + return res; } @@ -412,10 +437,21 @@ void HgfsTransportExit(void) { LOG(8, ("Entered.\n")); - pthread_mutex_lock(&gHgfsActiveChannelLock); - HgfsTransportChannelClose(&gHgfsActiveChannel); - pthread_mutex_unlock(&gHgfsActiveChannelLock); + + if (gHgfsActiveChannelLockInited) { + pthread_mutex_lock(&gHgfsActiveChannelLock); + HgfsTransportChannelClose(&gHgfsActiveChannel); + pthread_mutex_unlock(&gHgfsActiveChannelLock); + + pthread_mutex_destroy(&gHgfsActiveChannelLock); + gHgfsActiveChannelLockInited = FALSE; + } ASSERT(list_empty(&gHgfsPendingRequests)); + + if (gHgfsPendingRequestsLockInited) { + pthread_mutex_destroy(&gHgfsPendingRequestsLock); + gHgfsPendingRequestsLockInited = FALSE; + } LOG(8, ("Exited.\n")); } diff --git a/open-vm-tools/vmhgfs-fuse/transport.h b/open-vm-tools/vmhgfs-fuse/transport.h index f623fe18c..e8d92e05b 100644 --- a/open-vm-tools/vmhgfs-fuse/transport.h +++ b/open-vm-tools/vmhgfs-fuse/transport.h @@ -26,24 +26,24 @@ #include "request.h" #include +typedef enum { + HGFS_CHANNEL_UNINITIALIZED, + HGFS_CHANNEL_NOTCONNECTED, + HGFS_CHANNEL_CONNECTED, +} HgfsChannelStatus; + /* * There are the operations a channel should implement. */ struct HgfsTransportChannel; typedef struct HgfsTransportChannelOps { - Bool (*open)(struct HgfsTransportChannel *); + HgfsChannelStatus (*open)(struct HgfsTransportChannel *); void (*close)(struct HgfsTransportChannel *); int (*send)(struct HgfsTransportChannel *, HgfsReq *); int (*recv)(struct HgfsTransportChannel *, char **, size_t *); void (*exit)(struct HgfsTransportChannel *); } HgfsTransportChannelOps; -typedef enum { - HGFS_CHANNEL_UNINITIALIZED, - HGFS_CHANNEL_NOTCONNECTED, - HGFS_CHANNEL_CONNECTED, -} HgfsChannelStatus; - typedef struct HgfsTransportChannel { const char *name; /* Channel name. */ HgfsTransportChannelOps ops; /* Channel ops. */