Re-work identity stealing code

The change introduced by patch 04c391d9 "Replace copying network
settings with identity stealing" had a flaw...

If a PV device that aliases an emulated device is installed then it
will only steal linkage if that emulated device is operational at the
time. This works on initial driver install, but not if the PV network
device driver is removed and then re-installed without an interim
reboot (which would make the emulated device operational). Instead,
after re-installation, the PV device has a brand new stack binding and
thus any network settings changes that were previously made are
apparently lost.

To fix this problem, this patch breaks the process of stealing the
emulated device's stack binding into two phases:

1) Discovery: When the PV device driver is installed, if an aliasing
   emulated device is operational, a DWORD value called "VIF" is
   created under the emulated devices software key. The value is set
   to the PV device instance.

2) Theft: When the PV device is being brought online, the registry is
   checked to see if any network device has a software key containing
   a "VIF" value that patches the PV device instance.
   If such an entry is found then the stack binding is stolen from
   that device. Since the "VIF" value is in the *emulated* device's
   software key, it will not be removed if the PV device driver is
   removed and re-installed and so the problem described above will
   not occur.

Because this patch avoids stealing the emulated device's stack bindings
if it is actually operational (i.e. we're in phase 1) this results in the
class installer building a new set of stack bindings for the PV device
during initial installation.
These are then restored any time a PV device with a stolen stack binding
is offlined. So, when a PV device is offline there is no evidence that
any stack binding has been stolen and hence, during driver removal, there
is no risk of an emulated device's stack binding being torn down.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

diff --git a/src/xenvif/frontend.c b/src/xenvif/frontend.c
index 7d682abca69dcf18a8e99fc6c02ec1cfde3592d2..d21c798e5055d1ce107603ccaaaa2bc141a8217b 100644 (file)
--- a/src/xenvif/frontend.c
+++ b/src/xenvif/frontend.c
@@ -498,9 +498,6 @@ FrontendProcessInterfaceTable(
         if (!Row->InterfaceAndOperStatusFlags.ConnectorPresent)
             continue;
 
-        if (Row->OperStatus != IfOperStatusUp)
-            continue;
-
         if (Row->PhysicalAddressLength != sizeof (ETHERNET_ADDRESS))
             continue;
 
@@ -509,6 +506,9 @@ FrontendProcessInterfaceTable(
                    sizeof (ETHERNET_ADDRESS)) != 0)
             continue;
 
+        if (Row->OperStatus != IfOperStatusUp)
+            continue;
+
         goto found;
     }
 

diff --git a/src/xenvif/pdo.c b/src/xenvif/pdo.c
index 6c4c9bd246587720ec57708d1716f44f4c5f9652..5fefaebac0f18bf22ce895b29a47f7314b3f8e0a 100644 (file)
--- a/src/xenvif/pdo.c
+++ b/src/xenvif/pdo.c
@@ -98,7 +98,9 @@ struct _XENVIF_PDO {
     PXENVIF_VIF_CONTEXT         VifContext;
     XENVIF_VIF_INTERFACE        VifInterface;
 
+    ULONG                       Number;
     BOOLEAN                     HasAlias;
+    BOOLEAN                     HasStolenLinkage;
 };
 
 static FORCEINLINE PVOID
@@ -298,6 +300,23 @@ PdoGetName(
     return __PdoGetName(Pdo);
 }
 
+static FORCEINLINE VOID
+__PdoSetNumber(
+    IN  PXENVIF_PDO Pdo,
+    IN  ULONG       Number
+    )
+{
+    Pdo->Number = Number;
+}
+
+static FORCEINLINE ULONG
+__PdoGetNumber(
+    IN  PXENVIF_PDO Pdo
+    )
+{
+    return Pdo->Number;
+}
+
 static FORCEINLINE BOOLEAN
 __PdoSetEjectRequested(
     IN  PXENVIF_PDO Pdo
@@ -1257,9 +1276,6 @@ PdoStartDevice(
         if (!Row->InterfaceAndOperStatusFlags.ConnectorPresent)
             continue;
 
-        if (Row->OperStatus != IfOperStatusUp)
-            continue;
-
         if (Row->PhysicalAddressLength != sizeof (ETHERNET_ADDRESS))
             continue;
 
@@ -1268,14 +1284,28 @@ PdoStartDevice(
                    sizeof (ETHERNET_ADDRESS)) != 0)
             continue;
 
+        if (Row->OperStatus != IfOperStatusUp)
+            continue;
+
+        (VOID) SettingsSetAlias(Row->Alias,
+                                Row->Description,
+                                &Row->InterfaceGuid,
+                                __PdoGetNumber(Pdo));
+
         Pdo->HasAlias = TRUE;
+    }
 
+    if (Pdo->HasAlias) {
         PdoUnplugRequest(Pdo, TRUE);
 
         status = STATUS_UNSUCCESSFUL;
         goto fail9;
     }
 
+    status = SettingsStealAliasLinkage(__PdoGetSoftwareKey(Pdo),
+                                       __PdoGetNumber(Pdo));
+    Pdo->HasStolenLinkage = (NT_SUCCESS(status)) ? TRUE : FALSE;
+
     StackLocation = IoGetCurrentIrpStackLocation(Irp);
 
     status = PdoD3ToD0(Pdo);
@@ -1296,6 +1326,11 @@ PdoStartDevice(
 fail10:
     Error("fail10\n");
 
+    if (Pdo->HasStolenLinkage) {
+        (VOID) SettingsRestoreLinkage(__PdoGetSoftwareKey(Pdo));
+        Pdo->HasStolenLinkage = FALSE;
+    }
+
     __FreeMibTable(Table);
 
     goto fail6;
@@ -1303,11 +1338,6 @@ fail10:
 fail9:
     Error("fail9\n");
 
-    (VOID) SettingsStealIdentity(__PdoGetSoftwareKey(Pdo),
-                                 Row->Alias,
-                                 Row->Description,
-                                 &Row->InterfaceGuid);
-
     DriverRequestReboot();
     __FreeMibTable(Table);
 
@@ -1398,6 +1428,11 @@ PdoStopDevice(
 
     PdoD0ToD3(Pdo);
 
+    if (Pdo->HasStolenLinkage) {
+        (VOID) SettingsRestoreLinkage(__PdoGetSoftwareKey(Pdo));
+        Pdo->HasStolenLinkage = FALSE;
+    }
+
 done:
     RtlZeroMemory(&Pdo->CurrentAddress, sizeof (ETHERNET_ADDRESS));
 
@@ -1489,6 +1524,11 @@ PdoRemoveDevice(
 
     PdoD0ToD3(Pdo);
 
+    if (Pdo->HasStolenLinkage) {
+        (VOID) SettingsRestoreLinkage(__PdoGetSoftwareKey(Pdo));
+        Pdo->HasStolenLinkage = FALSE;
+    }
+
 done:
     RtlZeroMemory(&Pdo->CurrentAddress, sizeof (ETHERNET_ADDRESS));
 
@@ -2654,6 +2694,7 @@ PdoCreate(
         goto fail4;
 
     __PdoSetName(Pdo, Number);
+    __PdoSetNumber(Pdo, Number);
 
     status = __PdoSetPermanentAddress(Pdo, Address);
     if (!NT_SUCCESS(status))
@@ -2757,6 +2798,8 @@ fail4:
 fail3:
     Error("fail3\n");
 
+    Pdo->Number = 0;
+
     Pdo->Fdo = NULL;
     Pdo->Dx = NULL;
 
@@ -2783,9 +2826,9 @@ PdoDestroy(
     PDEVICE_OBJECT  PhysicalDeviceObject = Dx->DeviceObject;
     PXENVIF_FDO     Fdo = __PdoGetFdo(Pdo);
 
-    ASSERT(!Pdo->UnplugRequested);
     ASSERT3U(__PdoGetDevicePnpState(Pdo), ==, Deleted);
 
+    Pdo->UnplugRequested = FALSE;
     Pdo->HasAlias = FALSE;
 
     ASSERT(__PdoIsMissing(Pdo));
@@ -2831,6 +2874,8 @@ PdoDestroy(
     ThreadJoin(Pdo->SystemPowerThread);
     Pdo->SystemPowerThread = NULL;
 
+    Pdo->Number = 0;
+
     Pdo->Fdo = NULL;
     Pdo->Dx = NULL;
 

diff --git a/src/xenvif/settings.c b/src/xenvif/settings.c
index e684b82a146c86dbc0a18b75ea96e71b8a79eef5..dcfeb60c56ed0f408193b61844c01f259011f904 100644 (file)
--- a/src/xenvif/settings.c
+++ b/src/xenvif/settings.c
@@ -374,7 +374,7 @@ fail1:
 }
 
 static NTSTATUS
-SettingsGetAliasNetInstance(
+SettingsGetNetInstance(
     IN  LPGUID                                      NetCfgInstanceID,
     OUT PANSI_STRING                                SubKeyName
     )
@@ -411,7 +411,7 @@ SettingsGetAliasNetInstance(
     if (Parameters.SubKeyName.Length == 0)
         goto fail5;
 
-    Info("%Z\n", &Parameters.SubKeyName);
+    Trace("%Z\n", &Parameters.SubKeyName);
 
     *SubKeyName = Parameters.SubKeyName;
 
@@ -447,6 +447,199 @@ fail1:
     return status;
 }
 
+typedef struct _SETTINGS_MATCH_NUMBER_PARAMETERS {
+    ULONG       Number;
+    ANSI_STRING SubKeyName;
+} SETTINGS_MATCH_NUMBER_PARAMETERS, *PSETTINGS_MATCH_NUMBER_PARAMETERS;
+
+static NTSTATUS
+SettingsMatchNumber(
+    IN  PVOID                           Context,
+    IN  HANDLE                          Key,
+    IN  PANSI_STRING                    SubKeyName
+    )
+{
+    PSETTINGS_MATCH_NUMBER_PARAMETERS   Parameters = Context;
+    HANDLE                              SubKey;
+    ANSI_STRING                         Ansi;
+    ULONG                               Value;
+    NTSTATUS                            status;
+
+    Trace("====> (%Z)\n", SubKeyName);
+
+    if (Parameters->SubKeyName.Length != 0)
+        goto done;
+
+    RtlInitAnsiString(&Ansi, "Properties");
+
+    if (RtlCompareString(&Ansi, SubKeyName, TRUE) == 0)
+        goto done;
+
+    status = RegistryOpenSubKey(Key,
+                                SubKeyName->Buffer,
+                                KEY_READ,
+                                &SubKey);
+    if (!NT_SUCCESS(status))
+        goto fail1;
+
+    status = RegistryQueryDwordValue(SubKey,
+                                     "VIF",
+                                     &Value);
+    if (NT_SUCCESS(status) &&
+        Parameters->Number == Value) {
+        Parameters->SubKeyName.MaximumLength = SubKeyName->MaximumLength;
+        Parameters->SubKeyName.Buffer = __SettingsAllocate(Parameters->SubKeyName.MaximumLength);
+
+        status = STATUS_NO_MEMORY;
+        if (Parameters->SubKeyName.Buffer == NULL)
+            goto fail2;
+
+        RtlCopyMemory(Parameters->SubKeyName.Buffer,
+                      SubKeyName->Buffer,
+                      SubKeyName->Length);
+
+        Parameters->SubKeyName.Length = SubKeyName->Length;
+    }
+
+    RegistryCloseKey(SubKey);
+
+done:
+    Trace("<====\n");
+
+    return STATUS_SUCCESS;
+
+fail2:
+    Error("fail2\n");
+
+    RegistryCloseKey(SubKey);
+
+fail1:
+    Error("fail1 (%08x)\n", status);
+
+    return status;
+}
+
+static NTSTATUS
+SettingsGetAliasNetInstance(
+    IN  ULONG                          Number,
+    OUT PANSI_STRING                   SubKeyName
+    )
+{
+    HANDLE                             NetKey;
+    SETTINGS_MATCH_NUMBER_PARAMETERS   Parameters;
+    NTSTATUS                           status;
+
+    status = SettingsOpenNetKey(KEY_READ, &NetKey);
+    if (!NT_SUCCESS(status))
+        goto fail1;
+
+    RtlZeroMemory(&Parameters, sizeof (Parameters));
+
+    Parameters.Number = Number;
+
+    status = RegistryEnumerateSubKeys(NetKey,
+                                      SettingsMatchNumber,
+                                      &Parameters);
+    if (!NT_SUCCESS(status))
+        goto fail2;
+
+    status = STATUS_UNSUCCESSFUL;
+    if (Parameters.SubKeyName.Length == 0)
+        goto fail3;
+
+    Trace("%Z\n", &Parameters.SubKeyName);
+
+    *SubKeyName = Parameters.SubKeyName;
+
+    RegistryCloseKey(NetKey);
+
+    return STATUS_SUCCESS;
+
+fail3:
+    Error("fail3\n");
+
+fail2:
+    Error("fail2\n");
+
+    RegistryCloseKey(NetKey);
+
+fail1:
+    Error("fail1 (%08x)\n", status);
+
+    return status;
+}
+
+NTSTATUS
+SettingsSetAlias(
+    IN  PWCHAR      Alias,
+    IN  PWCHAR      Description,
+    IN  LPGUID      NetCfgInstanceID,
+    IN  ULONG       Number
+    )
+{
+    ANSI_STRING     SubKeyName;
+    HANDLE          NetKey;
+    HANDLE          SubKey;
+    NTSTATUS        status;
+
+    Trace("====>\n");
+
+    Info("%ws (%ws)\n", Alias, Description);
+
+    status = SettingsGetNetInstance(NetCfgInstanceID,
+                                    &SubKeyName);
+    if (!NT_SUCCESS(status))
+        goto fail1;
+
+    status = SettingsOpenNetKey(KEY_READ,
+                                &NetKey);
+    if (!NT_SUCCESS(status))
+        goto fail2;
+
+    status = RegistryOpenSubKey(NetKey,
+                                SubKeyName.Buffer,
+                                KEY_READ,
+                                &SubKey);
+    if (!NT_SUCCESS(status))
+        goto fail3;
+
+    status = RegistryUpdateDwordValue(SubKey,
+                                      "VIF",
+                                      Number);
+    if (!NT_SUCCESS(status))
+        goto fail4;
+
+    RegistryCloseKey(SubKey);
+
+    RegistryCloseKey(NetKey);
+
+    __SettingsFree(SubKeyName.Buffer);
+
+    Trace("<====\n");
+
+    return STATUS_SUCCESS;
+
+fail4:
+    Error("fail4\n");
+
+    RegistryCloseKey(SubKey);
+
+fail3:
+    Error("fail3\n");
+
+    RegistryCloseKey(NetKey);
+
+fail2:
+    Error("fail2\n");
+
+    __SettingsFree(SubKeyName.Buffer);
+
+fail1:
+    Error("fail1 (%08x)\n", status);
+
+    return status;
+}
+
 static NTSTATUS
 SettingsCopyLinkage(
     IN HANDLE       DestinationKey,
@@ -494,69 +687,87 @@ fail1:
 }
 
 NTSTATUS
-SettingsStealIdentity(
-    IN HANDLE   SoftwareKey,
-    IN PWCHAR   Alias,
-    IN PWCHAR   Description,
-    IN LPGUID   NetCfgInstanceID
+SettingsStealAliasLinkage(
+    IN  HANDLE      SoftwareKey,
+    IN  ULONG       Number
     )
 {
-    ANSI_STRING SubKeyName;
-    HANDLE      NetKey;
-    HANDLE      SubKey;
-    NTSTATUS    status;
+    ANSI_STRING     SubKeyName;
+    HANDLE          BackupKey;
+    HANDLE          NetKey;
+    HANDLE          SubKey;
+    NTSTATUS        status;
 
-    Info("%ws (%ws)\n", Alias, Description);
+    Trace("====>\n");
 
-    status = SettingsGetAliasNetInstance(NetCfgInstanceID,
+    status = SettingsGetAliasNetInstance(Number,
                                          &SubKeyName);
     if (!NT_SUCCESS(status))
         goto fail1;
 
-    status = RegistryUpdateSzValue(SoftwareKey,
-                                   "AliasNetInstance",
-                                   REG_SZ,
-                                   &SubKeyName);
+    Info("FROM %s\n", SubKeyName);
+
+    status = RegistryCreateSubKey(SoftwareKey,
+                                  "Backup",
+                                  REG_OPTION_NON_VOLATILE,
+                                  &BackupKey);
     if (!NT_SUCCESS(status))
         goto fail2;
 
-    status = SettingsOpenNetKey(KEY_READ, &NetKey);
+    status = SettingsCopyLinkage(BackupKey,
+                                 SoftwareKey);
     if (!NT_SUCCESS(status))
         goto fail3;
 
+    status = SettingsOpenNetKey(KEY_READ,
+                                &NetKey);
+    if (!NT_SUCCESS(status))
+        goto fail4;
+
     status = RegistryOpenSubKey(NetKey,
                                 SubKeyName.Buffer,
                                 KEY_READ,
                                 &SubKey);
     if (!NT_SUCCESS(status))
-        goto fail4;
+        goto fail5;
 
     status = SettingsCopyLinkage(SoftwareKey,
                                  SubKey);
     if (!NT_SUCCESS(status))
-        goto fail5;
+        goto fail6;
 
     RegistryCloseKey(SubKey);
 
     RegistryCloseKey(NetKey);
 
+    RegistryCloseKey(BackupKey);
+
     __SettingsFree(SubKeyName.Buffer);
 
+    Trace("<====\n");
+
     return STATUS_SUCCESS;
 
+fail6:
+    Error("fail6\n");
+
+    RegistryCloseKey(SubKey);
+
 fail5:
     Error("fail5\n");
 
-    RegistryCloseKey(SubKey);
+    RegistryCloseKey(NetKey);
 
 fail4:
     Error("fail4\n");
 
-    RegistryCloseKey(NetKey);
-
 fail3:
     Error("fail3\n");
 
+    RegistryCloseKey(BackupKey);
+
+    (VOID) RegistryDeleteSubKey(SoftwareKey, "Backup");
+
 fail2:
     Error("fail2\n");
 
@@ -567,3 +778,44 @@ fail1:
 
     return status;
 }
+
+NTSTATUS
+SettingsRestoreLinkage(
+    IN  HANDLE  SoftwareKey
+    )
+{
+    HANDLE      BackupKey;
+    NTSTATUS    status;
+
+    Trace("====>\n");
+
+    status = RegistryOpenSubKey(SoftwareKey,
+                                "Backup",
+                                KEY_READ,
+                                &BackupKey);
+    if (!NT_SUCCESS(status))
+        goto fail1;
+
+    status = SettingsCopyLinkage(SoftwareKey,
+                                 BackupKey);
+    if (!NT_SUCCESS(status))
+        goto fail2;
+
+    RegistryCloseKey(BackupKey);
+
+    (VOID) RegistryDeleteSubKey(SoftwareKey, "Backup");
+
+    Trace("<====\n");
+
+    return STATUS_SUCCESS;
+
+fail2:
+    Error("fail2\n");
+
+    RegistryCloseKey(BackupKey);
+
+fail1:
+    Error("fail1 (%08x)\n", status);
+
+    return status;
+}

diff --git a/src/xenvif/settings.h b/src/xenvif/settings.h
index 08221f296e9cfa6e97add2de509d953cc69bfa8e..eb77ec8fd2ff6164f896275429454b3c12b04be3 100644 (file)
--- a/src/xenvif/settings.h
+++ b/src/xenvif/settings.h
@@ -33,11 +33,22 @@
 #define _XENVIF_SETTINGS_H
 
 extern NTSTATUS
-SettingsStealIdentity(
-     IN HANDLE      SoftwareKey,
-     IN PWCHAR      Alias,
-     IN PWCHAR      Description,
-     IN LPGUID      InterfaceGuid
+SettingsSetAlias(
+     IN PWCHAR  Alias,
+     IN PWCHAR  Description,
+     IN LPGUID  InterfaceGuid,
+     IN ULONG   Number
+     );
+
+extern NTSTATUS
+SettingsStealAliasLinkage(
+     IN HANDLE  SoftwareKey,
+     IN ULONG   Number
+     );
+
+extern NTSTATUS
+SettingsRestoreLinkage(
+     IN HANDLE  SoftwareKey
      );
 
 #endif  // _XENVIF_SETTINGS_H