Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-07 Thread Corinna Vinschen
On Nov  7 15:30, Christian Franke wrote:
> Corinna Vinschen wrote:
> > ..
> > Looking forward to it. We'll just need an entry for the release text
> > in winsup/cygwin/release/3.5.0 and doc/new-features.xml in the end :)
> 
> Attached for now as implementing the remaining subdirs is not yet scheduled.
> Docbook formatting not tested.

Looks good, pushed.


Thanks,
Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-07 Thread Christian Franke

Corinna Vinschen wrote:

..
Looking forward to it. We'll just need an entry for the release text
in winsup/cygwin/release/3.5.0 and doc/new-features.xml in the end :)


Attached for now as implementing the remaining subdirs is not yet 
scheduled. Docbook formatting not tested.


Christian

From b07de21461207a2b57465d3dd8f7db2b36d886c0 Mon Sep 17 00:00:00 2001
From: Christian Franke 
Date: Tue, 7 Nov 2023 15:25:54 +0100
Subject: [PATCH] Cygwin: Document /dev/disk/by-id and /dev/disk/by-partuuid

Signed-off-by: Christian Franke 
---
 winsup/cygwin/release/3.5.0 |  6 ++
 winsup/doc/new-features.xml | 14 ++
 2 files changed, 20 insertions(+)

diff --git a/winsup/cygwin/release/3.5.0 b/winsup/cygwin/release/3.5.0
index dbbf8009d..2d59818b5 100644
--- a/winsup/cygwin/release/3.5.0
+++ b/winsup/cygwin/release/3.5.0
@@ -17,6 +17,12 @@ What's new:
   class expressions, and collating symbols in the search pattern, i.e.,
   [:alnum:], [=a=], [.aa.].
 
+- Introduce /dev/disk directory with subdirectories by-id and by-partuuid.
+  The by-id directory provides symlinks for each disk and its partitions:
+  BUSTYPE-[VENDOR_]PRODUCT_[SERIAL|HASH][-partN] -> ../../sdX[N].
+  The by-partuuid directory provides symlinks for each MBR and GPT disk
+  partition: MBR_SERIAL-OFFSET -> ../../sdXN, GPT_GUID -> ../../sdXN.
+
 - Introduce /proc/codesets and /proc/locales with information on
   supported codesets and locales for all interested parties.  Locale(1)
   opens these files and uses the info for printing locale info like any
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 78b2dbafd..a8e8a7991 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -34,6 +34,20 @@ class expressions, and collating symbols in the search 
pattern, i.e.,
 [:alnum:], [=a=], [.aa.].
 
 
+
+Introduce /dev/disk directory with subdirectories by-id and by-partuuid.
+The by-id directory provides symlinks for each disk and its partitions:
+  
+  BUSTYPE-[VENDOR_]PRODUCT_[SERIAL|0xHASH][-partN] -> ../../sdX[N]
+  
+The by-partuuid directory provides symlinks for each MBR and GPT disk
+partition:
+  
+  MBR_SERIAL-OFFSET -> ../../sdXN
+  GPT_GUID -> ../../sdXN
+  
+
+
 
 Introduce /proc/codesets and /proc/locales with information on supported
 codesets and locales for all interested parties.  Locale(1) opens these
-- 
2.42.1



Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-07 Thread Corinna Vinschen
On Nov  7 11:10, Christian Franke wrote:
> Corinna Vinschen wrote:
> > Hi Christian,
> > 
> > On Nov  5 16:45, Christian Franke wrote:
> > > ...
> > > Old IOCTL dropped and code simplified.
> > Great.  I pushed your patch.
> 
> Thanks.
> 
> 
> > ...
> > > > Last, but not least, do you see a chance to add any other /dev/disk
> > > > subdir?  by-partuuid, perhaps?
> > > Possibly, but not very soon. I'm not yet sure which API functions could be
> > > used.
> > > Some early draft ideas:
> > > 
> > > /dev/disk/by-partuid (Partition UUID -> device)
> > >    GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
> > >    MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)
> > That should only require IOCTL_DISK_GET_PARTITION_INFO_EX, I think.
> 
> Easier than expected: DRIVE_LAYOUT_INFORMATION_EX already contains
> PARTITION_INFORMATION_EX so existing scanning function could be enhanced.
> Patch attached.

Nice, pushed!

> > > /dev/disk/by-uuid (Windows Volume UUID -> device)
> > >    Vol_UUID1 -> ../../sdXN  (disk volume)
> > >    Vol_UUID2 -> ../../scd0  (CD/DVD drive volume)
> > >    Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID}  (others, e.g. VeraCrypt
> > > volume)
> > Yeah, tricky. These are not the partition GUIDs but the filesystem
> > GUIDs or serial numbers.  AFAICS, Windows filesystems (FAT*, NTFS)
> > don't maintain a filesystem GUID, as, e. g., ext4 or xfs, but only
> > serial numbers you can fetch via NtQueryVolumeInformationFile.
> > A Linux example of that is the serial number from a FAT32 filesytem
> > as the EFI boot partition in by-uuid:
> > 
> >lrwxrwxrwx 1 root root 10 Oct 30 10:20 DC38-0407 -> ../../sda1
> > 
> > On second thought, maybe that's sufficient for our by-uuid emulation.
> > 
> > > /dev/disk/by-drive (Cygwin specific: drive letter -> volume)
> > >    c -> ../by-uuid/UUID (if UUID available)
> > >    x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume
> > > Shadow Copy)
> > Ah, good idea. That's what my extension in /proc/partition already
> > provides, but a /dev/disk/by-drive sounds like a great idea.
> 
> Left for later :-)

Looking forward to it. We'll just need an entry for the release text
in winsup/cygwin/release/3.5.0 and doc/new-features.xml in the end :)


Thanks,
Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-07 Thread Christian Franke

Corinna Vinschen wrote:

Hi Christian,

On Nov  5 16:45, Christian Franke wrote:

...
Old IOCTL dropped and code simplified.

Great.  I pushed your patch.


Thanks.



...

Last, but not least, do you see a chance to add any other /dev/disk
subdir?  by-partuuid, perhaps?

Possibly, but not very soon. I'm not yet sure which API functions could be
used.
Some early draft ideas:

/dev/disk/by-partuid (Partition UUID -> device)
   GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
   MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)

That should only require IOCTL_DISK_GET_PARTITION_INFO_EX, I think.


Easier than expected: DRIVE_LAYOUT_INFORMATION_EX already contains 
PARTITION_INFORMATION_EX so existing scanning function could be 
enhanced. Patch attached.






/dev/disk/by-uuid (Windows Volume UUID -> device)
   Vol_UUID1 -> ../../sdXN  (disk volume)
   Vol_UUID2 -> ../../scd0  (CD/DVD drive volume)
   Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID}  (others, e.g. VeraCrypt
volume)

Yeah, tricky. These are not the partition GUIDs but the filesystem
GUIDs or serial numbers.  AFAICS, Windows filesystems (FAT*, NTFS)
don't maintain a filesystem GUID, as, e. g., ext4 or xfs, but only
serial numbers you can fetch via NtQueryVolumeInformationFile.
A Linux example of that is the serial number from a FAT32 filesytem
as the EFI boot partition in by-uuid:

   lrwxrwxrwx 1 root root 10 Oct 30 10:20 DC38-0407 -> ../../sda1

On second thought, maybe that's sufficient for our by-uuid emulation.


/dev/disk/by-drive (Cygwin specific: drive letter -> volume)
   c -> ../by-uuid/UUID (if UUID available)
   x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume
Shadow Copy)

Ah, good idea. That's what my extension in /proc/partition already
provides, but a /dev/disk/by-drive sounds like a great idea.


Left for later :-)

Christian

From e9c9d2a1a1df9ddecd815300c62321a480f0de9b Mon Sep 17 00:00:00 2001
From: Christian Franke 
Date: Tue, 7 Nov 2023 10:57:15 +0100
Subject: [PATCH] Cygwin: Add /dev/disk/by-partuuid symlinks

The new directory '/dev/disk/by-partuuid' provides symlinks for each
MBR or GPT disk partition:
'MBR_SERIAL-OFFSET' -> '../../sdXN'
'GPT_GUID' -> '../../sdXN'

Signed-off-by: Christian Franke 
---
 winsup/cygwin/fhandler/dev_disk.cc  | 173 
 winsup/cygwin/local_includes/fhandler.h |   7 +-
 2 files changed, 125 insertions(+), 55 deletions(-)

diff --git a/winsup/cygwin/fhandler/dev_disk.cc 
b/winsup/cygwin/fhandler/dev_disk.cc
index 9a1cae5eb..fcd0de651 100644
--- a/winsup/cygwin/fhandler/dev_disk.cc
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -176,9 +176,38 @@ by_id_realloc (by_id_entry *p, size_t n)
   return reinterpret_cast(p2);
 }
 
+static bool
+format_partuuid (char *name, const PARTITION_INFORMATION_EX *pix)
+{
+  const GUID *guid;
+  switch (pix->PartitionStyle)
+{
+  case PARTITION_STYLE_MBR: guid = >Mbr.PartitionId; break;
+  case PARTITION_STYLE_GPT: guid = >Gpt.PartitionId; break;
+  default: return false;
+}
+
+  if (pix->PartitionStyle == PARTITION_STYLE_MBR && !guid->Data2
+  && !guid->Data3 && !guid->Data4[6] && !guid->Data4[7])
+  /* MBR "GUID": SERIAL---00NN-NNN0
+Byte offset in LE order --^^^
+Print as: SERIAL-OFFSET */
+__small_sprintf(name,
+   "%08_x-%02_x%02_x%02_x%02_x%02_x%02_x",
+   guid->Data1, guid->Data4[5], guid->Data4[4], guid->Data4[3],
+   guid->Data4[2], guid->Data4[1], guid->Data4[0]);
+  else
+__small_sprintf(name,
+   
"%08_x-%04_x-%04_x-%02_x%02_x-%02_x%02_x%02_x%02_x%02_x%02_x",
+   guid->Data1, guid->Data2, guid->Data3,
+   guid->Data4[0], guid->Data4[1], guid->Data4[2], 
guid->Data4[3],
+   guid->Data4[4], guid->Data4[5], guid->Data4[6], 
guid->Data4[7]);
+   return true;
+}
+
 /* Create sorted name -> drive mapping table. Must be freed by caller. */
 static int
-get_by_id_table (by_id_entry * )
+get_by_id_table (by_id_entry * , fhandler_dev_disk::dev_disk_location 
loc)
 {
   table = nullptr;
 
@@ -282,25 +311,31 @@ get_by_id_table (by_id_entry * )
}
}
 
- /* Fetch storage properties and create the ID string. */
- int rc = storprop_to_id_name (devhdl, , ioctl_buf,
-   table[table_size].name);
- if (rc <= 0)
+ const char *drive_name = "";
+ if (loc == fhandler_dev_disk::disk_by_id)
{
- if (rc < 0)
-   errno_set = true;
- continue;
+ /* Fetch storage properties and create the ID string. */
+ int rc = storprop_to_id_name (devhdl, , ioctl_buf,
+   table[table_size].name);
+ if (rc <= 0)
+   {
+ if (rc < 0)
+   errno_set = true;
+ continue;
+   }
+  

Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-05 Thread Corinna Vinschen
Hi Christian,

On Nov  5 16:45, Christian Franke wrote:
> Corinna Vinschen wrote:
> > Do we really still need IOCTL_DISK_GET_DRIVE_LAYOUT w/o _EX?
> 
> Possibly not. I borrowed this code from code behind /proc/partitions.
> 
> > fhandler_proc still uses it, too, but the EX variation is available
> > since XP. I can't imagine that a fallback to the non-EX-version is still
> > necessary.  If you like you could drop it immediately, or we can drop
> > both usages as a followup patch.
> 
> Old IOCTL dropped and code simplified.

Great.  I pushed your patch.

However, while I was just happily removing the non-EX ioctl's from
fhandler/proc.cc I found that this isn't feasible:

The EX calls are not supported by the floppy driver.  D'uh.

So for /proc/partitions emulation we still need them.

> > Last, but not least, do you see a chance to add any other /dev/disk
> > subdir?  by-partuuid, perhaps?
> 
> Possibly, but not very soon. I'm not yet sure which API functions could be
> used.
> Some early draft ideas:
> 
> /dev/disk/by-partuid (Partition UUID -> device)
>   GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
>   MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)

That should only require IOCTL_DISK_GET_PARTITION_INFO_EX, I think.

> /dev/disk/by-uuid (Windows Volume UUID -> device)
>   Vol_UUID1 -> ../../sdXN  (disk volume)
>   Vol_UUID2 -> ../../scd0  (CD/DVD drive volume)
>   Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID}  (others, e.g. VeraCrypt
> volume)

Yeah, tricky. These are not the partition GUIDs but the filesystem
GUIDs or serial numbers.  AFAICS, Windows filesystems (FAT*, NTFS)
don't maintain a filesystem GUID, as, e. g., ext4 or xfs, but only
serial numbers you can fetch via NtQueryVolumeInformationFile.
A Linux example of that is the serial number from a FAT32 filesytem
as the EFI boot partition in by-uuid:

  lrwxrwxrwx 1 root root 10 Oct 30 10:20 DC38-0407 -> ../../sda1

On second thought, maybe that's sufficient for our by-uuid emulation.

> /dev/disk/by-drive (Cygwin specific: drive letter -> volume)
>   c -> ../by-uuid/UUID (if UUID available)
>   x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume
> Shadow Copy)

Ah, good idea. That's what my extension in /proc/partition already
provides, but a /dev/disk/by-drive sounds like a great idea.


Thanks,
Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-05 Thread Christian Franke

Hi Corinna,

Corinna Vinschen wrote:

Hi Christian,

patch looks pretty good to me.  Just two minor points:


+  /* Traverse \Device directory ... */
+  bool errno_set = false;
+  HANDLE devhdl = INVALID_HANDLE_VALUE;

INVALID_HANDLE_VALUE is a Win32 API concept and is only returned by
CreateFile and friends.  The native NT API doesn't know
INVALID_HANDLE_VALUE and, fortunately, only uses NULL.  I think it's
puzzeling to use INVALID_HANDLE_VALUE in a native NT context. So, would
you mind to just use NULL (or nullptr) instead?  As in...


+  if (devhdl != INVALID_HANDLE_VALUE)

  if (devhdl)

Sounds easier to me :)


Yes...done.



+ /* Fetch drive layout info to get size of all partitions on disk. */
+ DWORD part_cnt = 0;
+ PARTITION_INFORMATION_EX *pix = nullptr;
+ PARTITION_INFORMATION *pi = nullptr;
+ DWORD bytes_read;
+ if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 
0,
+  ioctl_buf, NT_MAX_PATH, _read, nullptr))
+   {
+ DRIVE_LAYOUT_INFORMATION_EX *pdlix =
+ reinterpret_cast(ioctl_buf);
+ part_cnt = pdlix->PartitionCount;
+ pix = pdlix->PartitionEntry;
+   }
+ else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT, 
nullptr,

Do we really still need IOCTL_DISK_GET_DRIVE_LAYOUT w/o _EX?


Possibly not. I borrowed this code from code behind /proc/partitions.



fhandler_proc still uses it, too, but the EX variation is available
since XP. I can't imagine that a fallback to the non-EX-version is still
necessary.  If you like you could drop it immediately, or we can drop
both usages as a followup patch.


Old IOCTL dropped and code simplified.



Last, but not least, do you see a chance to add any other /dev/disk
subdir?  by-partuuid, perhaps?


Possibly, but not very soon. I'm not yet sure which API functions could 
be used.

Some early draft ideas:

/dev/disk/by-partuid (Partition UUID -> device)
  GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
  MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)

/dev/disk/by-uuid (Windows Volume UUID -> device)
  Vol_UUID1 -> ../../sdXN  (disk volume)
  Vol_UUID2 -> ../../scd0  (CD/DVD drive volume)
  Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID}  (others, e.g. VeraCrypt 
volume)


/dev/disk/by-drive (Cygwin specific: drive letter -> volume)
  c -> ../by-uuid/UUID (if UUID available)
  x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume 
Shadow Copy)


Christian

From aa8c35e041ffe5a4b06104c122d9ba1fdc492683 Mon Sep 17 00:00:00 2001
From: Christian Franke 
Date: Sun, 5 Nov 2023 15:54:23 +0100
Subject: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

The new directory '/dev/disk/by-id' provides symlinks for each
disk and its partitions:
'BUSTYPE-[VENDOR_]PRODUCT_SERIAL[-partN]' -> '../../sdX[N]'.
This is based on strings provided by STORAGE_DEVICE_DESCRIPTOR.
If this information is too short, a 128-bit hash of the
STORAGE_DEVICE_UNIQUE_IDENTIFIER raw data is added.
Administrator privileges are not required.

Signed-off-by: Christian Franke 
---
 winsup/cygwin/Makefile.am   |   1 +
 winsup/cygwin/devices.in|   4 +
 winsup/cygwin/dtable.cc |   3 +
 winsup/cygwin/fhandler/dev_disk.cc  | 621 
 winsup/cygwin/local_includes/devices.h  |   6 +-
 winsup/cygwin/local_includes/fhandler.h |  47 ++
 winsup/cygwin/mount.cc  |  10 +
 7 files changed, 691 insertions(+), 1 deletion(-)
 create mode 100644 winsup/cygwin/fhandler/dev_disk.cc

diff --git a/winsup/cygwin/Makefile.am b/winsup/cygwin/Makefile.am
index 64b252a22..376c79fc3 100644
--- a/winsup/cygwin/Makefile.am
+++ b/winsup/cygwin/Makefile.am
@@ -84,6 +84,7 @@ FHANDLER_FILES= \
fhandler/console.cc \
fhandler/cygdrive.cc \
fhandler/dev.cc \
+   fhandler/dev_disk.cc \
fhandler/dev_fd.cc \
fhandler/disk_file.cc \
fhandler/dsp.cc \
diff --git a/winsup/cygwin/devices.in b/winsup/cygwin/devices.in
index 2545dd85e..48d3843fe 100644
--- a/winsup/cygwin/devices.in
+++ b/winsup/cygwin/devices.in
@@ -115,6 +115,9 @@ const _device dev_cygdrive_storage =
 const _device dev_fs_storage =
   {"", {FH_FS}, "", exists};
 
+const _device dev_dev_disk_storage =
+  {"", {FH_DEV_DISK}, "", exists};
+
 const _device dev_proc_storage =
   {"", {FH_PROC}, "", exists};
 
@@ -173,6 +176,7 @@ const _device dev_error_storage =
the POSIX namespace.  */
 %%
 "/dev", BRACK(FH_DEV), "", exists, S_IFDIR
+"/dev/disk", BRACK(FH_DEV_DISK), "", exists, S_IFDIR
 "/dev/tty", BRACK(FH_TTY), "/dev/tty", exists, S_IFCHR
 "/dev/pty%(0-127)d", BRACK(FHDEV(DEV_PTYS_MAJOR, {$1})), "/dev/pty{$1}", 
exists_pty, S_IFCHR, =ptys_dev
 ":p

Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-04 Thread Corinna Vinschen
Hi Christian,

patch looks pretty good to me.  Just two minor points:

> +  /* Traverse \Device directory ... */
> +  bool errno_set = false;
> +  HANDLE devhdl = INVALID_HANDLE_VALUE;

INVALID_HANDLE_VALUE is a Win32 API concept and is only returned by
CreateFile and friends.  The native NT API doesn't know
INVALID_HANDLE_VALUE and, fortunately, only uses NULL.  I think it's
puzzeling to use INVALID_HANDLE_VALUE in a native NT context. So, would
you mind to just use NULL (or nullptr) instead?  As in...

> +  if (devhdl != INVALID_HANDLE_VALUE)

 if (devhdl)

Sounds easier to me :)

> +   /* Fetch drive layout info to get size of all partitions on disk. */
> +   DWORD part_cnt = 0;
> +   PARTITION_INFORMATION_EX *pix = nullptr;
> +   PARTITION_INFORMATION *pi = nullptr;
> +   DWORD bytes_read;
> +   if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 
> 0,
> +ioctl_buf, NT_MAX_PATH, _read, nullptr))
> + {
> +   DRIVE_LAYOUT_INFORMATION_EX *pdlix =
> +   reinterpret_cast(ioctl_buf);
> +   part_cnt = pdlix->PartitionCount;
> +   pix = pdlix->PartitionEntry;
> + }
> +   else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT, 
> nullptr,

Do we really still need IOCTL_DISK_GET_DRIVE_LAYOUT w/o _EX?

fhandler_proc still uses it, too, but the EX variation is available
since XP. I can't imagine that a fallback to the non-EX-version is still
necessary.  If you like you could drop it immediately, or we can drop
both usages as a followup patch.

Last, but not least, do you see a chance to add any other /dev/disk
subdir?  by-partuuid, perhaps?


Thanks,
Corinna



Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-04 Thread Christian Franke

Corinna Vinschen wrote:

On Nov  4 10:34, Corinna Vinschen wrote:

On Nov  3 18:54, Christian Franke wrote:

...
Conclusion: The behavior of the current patch is compatible with Linux :-)

Ok, but with the DUID we have a workaround which makes it  work even
better than on Linux, so it would begreat if we used it, unless we find
out where the UUID in "\GLOBAL??\Disk{} comes from...

Given the size of the STORAGE_DEVICE_UNIQUE_IDENTIFIER struct, we could
even contemplate a 128 bit hash, just to be on the safe side.

Kind of like this

-  strcat (name, ioctl_buf + desc->SerialNumberOffset);
+  /* Use SerialNumber in the first place, if available */
+  if (desc->SerialNumberOffset && desc_buf[desc->SerialNumberOffset])
+strcat (name, desc_buf + desc->SerialNumberOffset);
+  else /* Utilize the DUID as defined by MSDN to generate a hash */
+{
+  union {
+   unsigned __int128 all;
+   struct {
+ unsigned long high;
+ unsigned long low;
+   };
+  } hash = { 0 };
+
+  for (ULONG i = 0; i < id->Size; ++i)
+   hash.all = ioctl_buf[i] + (hash.all << 6) + (hash.all << 16) - hash.all;
+  __small_sprintf (name + strlen (name), "%X%X", hash.high, hash.low);
+}



New patch attached. Only fhandler/dev_disk.cc changed, devices.cc again 
not included.


Christian

From 5db1cd009e8dc6ff01fabdeb8cddf65d609bfe2f Mon Sep 17 00:00:00 2001
From: Christian Franke 
Date: Sat, 4 Nov 2023 16:49:28 +0100
Subject: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

The new directory '/dev/disk/by-id' provides symlinks for each
disk and its partitions:
'BUSTYPE-[VENDOR_]PRODUCT_SERIAL[-partN]' -> '../../sdX[N]'.
This is based on strings provided by STORAGE_DEVICE_DESCRIPTOR.
If this information is too short, a 128-bit hash of the
STORAGE_DEVICE_UNIQUE_IDENTIFIER raw data is added.
Administrator privileges are not required.

Signed-off-by: Christian Franke 
---
 winsup/cygwin/Makefile.am   |   1 +
 winsup/cygwin/devices.in|   4 +
 winsup/cygwin/dtable.cc |   3 +
 winsup/cygwin/fhandler/dev_disk.cc  | 646 
 winsup/cygwin/local_includes/devices.h  |   6 +-
 winsup/cygwin/local_includes/fhandler.h |  47 ++
 winsup/cygwin/mount.cc  |  10 +
 7 files changed, 716 insertions(+), 1 deletion(-)
 create mode 100644 winsup/cygwin/fhandler/dev_disk.cc

diff --git a/winsup/cygwin/Makefile.am b/winsup/cygwin/Makefile.am
index 64b252a22..376c79fc3 100644
--- a/winsup/cygwin/Makefile.am
+++ b/winsup/cygwin/Makefile.am
@@ -84,6 +84,7 @@ FHANDLER_FILES= \
fhandler/console.cc \
fhandler/cygdrive.cc \
fhandler/dev.cc \
+   fhandler/dev_disk.cc \
fhandler/dev_fd.cc \
fhandler/disk_file.cc \
fhandler/dsp.cc \
diff --git a/winsup/cygwin/devices.in b/winsup/cygwin/devices.in
index 2545dd85e..48d3843fe 100644
--- a/winsup/cygwin/devices.in
+++ b/winsup/cygwin/devices.in
@@ -115,6 +115,9 @@ const _device dev_cygdrive_storage =
 const _device dev_fs_storage =
   {"", {FH_FS}, "", exists};
 
+const _device dev_dev_disk_storage =
+  {"", {FH_DEV_DISK}, "", exists};
+
 const _device dev_proc_storage =
   {"", {FH_PROC}, "", exists};
 
@@ -173,6 +176,7 @@ const _device dev_error_storage =
the POSIX namespace.  */
 %%
 "/dev", BRACK(FH_DEV), "", exists, S_IFDIR
+"/dev/disk", BRACK(FH_DEV_DISK), "", exists, S_IFDIR
 "/dev/tty", BRACK(FH_TTY), "/dev/tty", exists, S_IFCHR
 "/dev/pty%(0-127)d", BRACK(FHDEV(DEV_PTYS_MAJOR, {$1})), "/dev/pty{$1}", 
exists_pty, S_IFCHR, =ptys_dev
 ":ptym%(0-127)d", BRACK(FHDEV(DEV_PTYM_MAJOR, {$1})), "/dev/ptym{$1}", 
exists_internal, S_IFCHR, =ptym_dev
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 21d525389..9508f3e0b 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -585,6 +585,9 @@ fh_alloc (path_conv& pc)
case FH_DEV:
  fh = cnew (fhandler_dev);
  break;
+   case FH_DEV_DISK:
+ fh = cnew (fhandler_dev_disk);
+ break;
case FH_DEV_FD:
  fh = cnew (fhandler_dev_fd);
  break;
diff --git a/winsup/cygwin/fhandler/dev_disk.cc 
b/winsup/cygwin/fhandler/dev_disk.cc
new file mode 100644
index 0..2cbe5434e
--- /dev/null
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -0,0 +1,646 @@
+/* fhandler/dev_disk.cc: fhandler for the /dev/disk/by-id/... symlinks.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#include "winsup.h"
+#include "path.h"
+#include "fhandler.h"
+#include "tls_pbuf.h"
+#include 
+#include 
+#include 
+
+/* Replace no

Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-04 Thread Christian Franke

Corinna Vinschen wrote:

On Nov  4 10:34, Corinna Vinschen wrote:

On Nov  3 18:54, Christian Franke wrote:

...

MSDN claims:

If the storage device is SCSI-compliant, the port driver attempts to
extract the serial number from the optional Unit Serial Number page
(page 0x80) of the VPD.

Now I'm puzzled.

A quick test with a Debian 12 VM in VirtualBox with many virtual
controllers+drives shows the same problem:
Entries in /dev/disk/by-id appear only for virtual disks behind emulated
SATA and NVMe controllers, but not for SCSI and SAS controllers.
A test with "smartctl -i ..." with SCSI/SAS devices doesn't print a serial
number. In debug mode it prints "Vital Product Data (VPD) INQUIRY failed..."
and other messages that suggest limited/buggy support of optional SCSI
commands.

If a Win11 PE (from install ISO) is run in same VM, the
STORAGE_DEVICE_DESCRIPTOR only provides the serial number for SATA (NVMe
drives not detected), but not for SCSI.

Conclusion: The behavior of the current patch is compatible with Linux :-)

Ok, but with the DUID we have a workaround which makes it  work even
better than on Linux, so it would begreat if we used it, unless we find
out where the UUID in "\GLOBAL??\Disk{} comes from...

Given the size of the STORAGE_DEVICE_UNIQUE_IDENTIFIER struct, we could
even contemplate a 128 bit hash, just to be on the safe side.

Kind of like this

-  strcat (name, ioctl_buf + desc->SerialNumberOffset);
+  /* Use SerialNumber in the first place, if available */
+  if (desc->SerialNumberOffset && desc_buf[desc->SerialNumberOffset])
+strcat (name, desc_buf + desc->SerialNumberOffset);
+  else /* Utilize the DUID as defined by MSDN to generate a hash */
+{
+  union {
+   unsigned __int128 all;
+   struct {
+ unsigned long high;
+ unsigned long low;
+   };
+  } hash = { 0 };
+
+  for (ULONG i = 0; i < id->Size; ++i)
+   hash.all = ioctl_buf[i] + (hash.all << 6) + (hash.all << 16) - hash.all;
+  __small_sprintf (name + strlen (name), "%X%X", hash.high, hash.low);
+}



I agree and will provide a new patch soon.

Christian



Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-04 Thread Corinna Vinschen
On Nov  4 10:34, Corinna Vinschen wrote:
> On Nov  3 18:54, Christian Franke wrote:
> > Corinna Vinschen wrote:
> > > On Nov  3 17:27, Corinna Vinschen wrote:
> > > > On Nov  3 17:09, Christian Franke wrote:
> > > > > Unlike (S)ATA and NVMe, the serial number
> > > > > is not available for free in the device identify data block but 
> > > > > requires an
> > > > > extra command (SCSI INQUIRY of VPD page 0x80). This might not be 
> > > > > supported
> > > > > by the emulated controller or Windows does not use this command.
> > > > AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
> > > > which is equivalent to the data from VPD page 0x83.  As you can see,
> > > > it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data.  The data
> > > > returned for the VirtIo device is the identifier string "\x01\x00",
> > > > which is a bit underwhelming.
> > > > 
> > > > Would be great if we would learn how to access page 0x80...
> > > Uhm...
> > > 
> > > MSDN claims:
> > > 
> > >If the storage device is SCSI-compliant, the port driver attempts to
> > >extract the serial number from the optional Unit Serial Number page
> > >(page 0x80) of the VPD.
> > > 
> > > Now I'm puzzled.
> > 
> > A quick test with a Debian 12 VM in VirtualBox with many virtual
> > controllers+drives shows the same problem:
> > Entries in /dev/disk/by-id appear only for virtual disks behind emulated
> > SATA and NVMe controllers, but not for SCSI and SAS controllers.
> > A test with "smartctl -i ..." with SCSI/SAS devices doesn't print a serial
> > number. In debug mode it prints "Vital Product Data (VPD) INQUIRY failed..."
> > and other messages that suggest limited/buggy support of optional SCSI
> > commands.
> > 
> > If a Win11 PE (from install ISO) is run in same VM, the
> > STORAGE_DEVICE_DESCRIPTOR only provides the serial number for SATA (NVMe
> > drives not detected), but not for SCSI.
> > 
> > Conclusion: The behavior of the current patch is compatible with Linux :-)
> 
> Ok, but with the DUID we have a workaround which makes it  work even
> better than on Linux, so it would begreat if we used it, unless we find
> out where the UUID in "\GLOBAL??\Disk{} comes from...
> 
> Given the size of the STORAGE_DEVICE_UNIQUE_IDENTIFIER struct, we could
> even contemplate a 128 bit hash, just to be on the safe side.

Kind of like this

-  strcat (name, ioctl_buf + desc->SerialNumberOffset);
+  /* Use SerialNumber in the first place, if available */
+  if (desc->SerialNumberOffset && desc_buf[desc->SerialNumberOffset])
+strcat (name, desc_buf + desc->SerialNumberOffset);
+  else /* Utilize the DUID as defined by MSDN to generate a hash */
+{
+  union {
+   unsigned __int128 all;
+   struct {
+ unsigned long high;
+ unsigned long low;
+   };
+  } hash = { 0 };
+
+  for (ULONG i = 0; i < id->Size; ++i)
+   hash.all = ioctl_buf[i] + (hash.all << 6) + (hash.all << 16) - hash.all;
+  __small_sprintf (name + strlen (name), "%X%X", hash.high, hash.low);
+}


Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-04 Thread Corinna Vinschen
On Nov  3 18:54, Christian Franke wrote:
> Corinna Vinschen wrote:
> > On Nov  3 17:27, Corinna Vinschen wrote:
> > > On Nov  3 17:09, Christian Franke wrote:
> > > > Unlike (S)ATA and NVMe, the serial number
> > > > is not available for free in the device identify data block but 
> > > > requires an
> > > > extra command (SCSI INQUIRY of VPD page 0x80). This might not be 
> > > > supported
> > > > by the emulated controller or Windows does not use this command.
> > > AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
> > > which is equivalent to the data from VPD page 0x83.  As you can see,
> > > it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data.  The data
> > > returned for the VirtIo device is the identifier string "\x01\x00",
> > > which is a bit underwhelming.
> > > 
> > > Would be great if we would learn how to access page 0x80...
> > Uhm...
> > 
> > MSDN claims:
> > 
> >If the storage device is SCSI-compliant, the port driver attempts to
> >extract the serial number from the optional Unit Serial Number page
> >(page 0x80) of the VPD.
> > 
> > Now I'm puzzled.
> 
> A quick test with a Debian 12 VM in VirtualBox with many virtual
> controllers+drives shows the same problem:
> Entries in /dev/disk/by-id appear only for virtual disks behind emulated
> SATA and NVMe controllers, but not for SCSI and SAS controllers.
> A test with "smartctl -i ..." with SCSI/SAS devices doesn't print a serial
> number. In debug mode it prints "Vital Product Data (VPD) INQUIRY failed..."
> and other messages that suggest limited/buggy support of optional SCSI
> commands.
> 
> If a Win11 PE (from install ISO) is run in same VM, the
> STORAGE_DEVICE_DESCRIPTOR only provides the serial number for SATA (NVMe
> drives not detected), but not for SCSI.
> 
> Conclusion: The behavior of the current patch is compatible with Linux :-)

Ok, but with the DUID we have a workaround which makes it  work even
better than on Linux, so it would begreat if we used it, unless we find
out where the UUID in "\GLOBAL??\Disk{} comes from...

Given the size of the STORAGE_DEVICE_UNIQUE_IDENTIFIER struct, we could
even contemplate a 128 bit hash, just to be on the safe side.


Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Christian Franke

Corinna Vinschen wrote:

On Nov  3 17:27, Corinna Vinschen wrote:

On Nov  3 17:09, Christian Franke wrote:

Unlike (S)ATA and NVMe, the serial number
is not available for free in the device identify data block but requires an
extra command (SCSI INQUIRY of VPD page 0x80). This might not be supported
by the emulated controller or Windows does not use this command.

AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
which is equivalent to the data from VPD page 0x83.  As you can see,
it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data.  The data
returned for the VirtIo device is the identifier string "\x01\x00",
which is a bit underwhelming.

Would be great if we would learn how to access page 0x80...

Uhm...

MSDN claims:

   If the storage device is SCSI-compliant, the port driver attempts to
   extract the serial number from the optional Unit Serial Number page
   (page 0x80) of the VPD.

Now I'm puzzled.


A quick test with a Debian 12 VM in VirtualBox with many virtual 
controllers+drives shows the same problem:
Entries in /dev/disk/by-id appear only for virtual disks behind emulated 
SATA and NVMe controllers, but not for SCSI and SAS controllers.
A test with "smartctl -i ..." with SCSI/SAS devices doesn't print a 
serial number. In debug mode it prints "Vital Product Data (VPD) INQUIRY 
failed..." and other messages that suggest limited/buggy support of 
optional SCSI commands.


If a Win11 PE (from install ISO) is run in same VM, the 
STORAGE_DEVICE_DESCRIPTOR only provides the serial number for SATA (NVMe 
drives not detected), but not for SCSI.


Conclusion: The behavior of the current patch is compatible with Linux :-)

Christian



Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Corinna Vinschen
On Nov  3 17:27, Corinna Vinschen wrote:
> On Nov  3 17:09, Christian Franke wrote:
> > Unlike (S)ATA and NVMe, the serial number
> > is not available for free in the device identify data block but requires an
> > extra command (SCSI INQUIRY of VPD page 0x80). This might not be supported
> > by the emulated controller or Windows does not use this command.
> 
> AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
> which is equivalent to the data from VPD page 0x83.  As you can see,
> it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data.  The data
> returned for the VirtIo device is the identifier string "\x01\x00",
> which is a bit underwhelming.
> 
> Would be great if we would learn how to access page 0x80...

Uhm...

MSDN claims:

  If the storage device is SCSI-compliant, the port driver attempts to
  extract the serial number from the optional Unit Serial Number page
  (page 0x80) of the VPD.

Now I'm puzzled.


Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Corinna Vinschen
On Nov  3 17:09, Christian Franke wrote:
> Corinna Vinschen wrote:
> > I haven't found out where the UUID is coming from, yet, but based on the
> > description from
> > https://learn.microsoft.com/en-us/windows-hardware/drivers/storage/device-unique-identifiers--duids--for-storage-devices
> > I came up with this Q solution:
> > 
> > === SNIP 
> > diff --git a/winsup/cygwin/fhandler/dev_disk.cc 
> > b/winsup/cygwin/fhandler/dev_disk.cc
> > index caca57d63216..74abfb8a3288 100644
> > --- a/winsup/cygwin/fhandler/dev_disk.cc
> > +++ b/winsup/cygwin/fhandler/dev_disk.cc
> > @@ -36,29 +36,51 @@ sanitize_id_string (char *s)
> > return i;
> >   }
> > +typedef struct _STORAGE_DEVICE_UNIQUE_IDENTIFIER {
> > +  ULONG Version;
> > +  ULONG Size;
> > +  ULONG StorageDeviceIdOffset;
> > +  ULONG StorageDeviceOffset;
> > +  ULONG DriveLayoutSignatureOffset;
> > +} STORAGE_DEVICE_UNIQUE_IDENTIFIER, *PSTORAGE_DEVICE_UNIQUE_IDENTIFIER;
> > +
> > +typedef struct _STORAGE_DEVICE_LAYOUT_SIGNATURE {
> > +  ULONG   Version;
> > +  ULONG   Size;
> > +  BOOLEAN Mbr;
> > +  union {
> > +ULONG MbrSignature;
> > +GUID  GptDiskId;
> > +  } DeviceSpecific;
> > +} STORAGE_DEVICE_LAYOUT_SIGNATURE, *PSTORAGE_DEVICE_LAYOUT_SIGNATURE;
> > +
> 
> These are available in storduid.h

Yeah, and defining STORAGE_DEVICE_LAYOUT_SIGNATURE is useless,
but it was just for PoC.

> Thanks. Using this makes plenty of sense as a fallback if the serial number
> is unavailable. But if available, the serial number should be in the
> generated name as on Linux. This would provide a persistent name which
> reflects the actual device without a number invented by MS.

ACK

> The serial number is usually available with (S)ATA and NVMe (namespace uuid
> in the latter case). I'm not familiar with QEMU/KVM details. The fact that
> both 'vendor' and 'product' are returned on your system suggests that a
> SCSI/SAS controller is emulated.

Yes.

> Unlike (S)ATA and NVMe, the serial number
> is not available for free in the device identify data block but requires an
> extra command (SCSI INQUIRY of VPD page 0x80). This might not be supported
> by the emulated controller or Windows does not use this command.

AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
which is equivalent to the data from VPD page 0x83.  As you can see,
it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data.  The data
returned for the VirtIo device is the identifier string "\x01\x00",
which is a bit underwhelming.

Would be great if we would learn how to access page 0x80...

> IIRC the serial number is sometimes available via WMI even if missing in
> IOCTL_STORAGE_QUERY_PROPERTY:
> 
>   wmic diskdrive get manufacturer,model,serialnumber

Nope, it returns an empty string, just as the date from
STORAGE_DEVICE_DESCRIPTOR.


Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Christian Franke

Corinna Vinschen wrote:

On Nov  3 12:10, Corinna Vinschen wrote:

On Nov  3 11:09, Corinna Vinschen wrote:

On Nov  3 10:55, Corinna Vinschen wrote:

On Oct  3 14:39, Christian Franke wrote:

According to NtQueryObject(., ObjectBasicInformation, ...), using
NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
For some unknown reason, NVMe drives behind stornvme.sys additionally
require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
bug in the access check somewhere in the NVMe stack.

The disk scanning from the first patch has been reworked based on code
borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
make sense to provide one flexible scanning function for /dev/sdXN,
/proc/partitions and /proc/disk/by-id.

I applied your patch locally (patch looks pretty well, btw) but found
that /dev/disk/by-id is empty, even when running with admin rights.

I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM.  A
\Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
exists in both cases.  I straced it, and found the following debug
output:

   1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 
'Red_Hat' 'VirtIO' '' (ignored)

Is that really desired?

We could fake a serial number dependent on the path.  See
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/mount.cc;hb=main#l253

Alternatively, there's also a symlink in GLOBAL?? pointing
to the same target as \Device\Harddisk0\Partition0, i. e.:

   \Device\Harddisk0\Partition0 -> \Device\Harddisk0\DR0

and

   \GLOBAL??\Disk{4c622943-27e4-e81d-3fc7-c43fc9c7e126} -> \Device\Harddisk0\DR0

We could use that UUID from that path, but that's quite a hassle
to grab, because it requires to enumerate GLOBAL??.

But then again, where does Windows get the UUID from?  Something to
find out...

I haven't found out where the UUID is coming from, yet, but based on the
description from
https://learn.microsoft.com/en-us/windows-hardware/drivers/storage/device-unique-identifiers--duids--for-storage-devices
I came up with this Q solution:

=== SNIP 
diff --git a/winsup/cygwin/fhandler/dev_disk.cc 
b/winsup/cygwin/fhandler/dev_disk.cc
index caca57d63216..74abfb8a3288 100644
--- a/winsup/cygwin/fhandler/dev_disk.cc
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -36,29 +36,51 @@ sanitize_id_string (char *s)
return i;
  }
  
+typedef struct _STORAGE_DEVICE_UNIQUE_IDENTIFIER {

+  ULONG Version;
+  ULONG Size;
+  ULONG StorageDeviceIdOffset;
+  ULONG StorageDeviceOffset;
+  ULONG DriveLayoutSignatureOffset;
+} STORAGE_DEVICE_UNIQUE_IDENTIFIER, *PSTORAGE_DEVICE_UNIQUE_IDENTIFIER;
+
+typedef struct _STORAGE_DEVICE_LAYOUT_SIGNATURE {
+  ULONG   Version;
+  ULONG   Size;
+  BOOLEAN Mbr;
+  union {
+ULONG MbrSignature;
+GUID  GptDiskId;
+  } DeviceSpecific;
+} STORAGE_DEVICE_LAYOUT_SIGNATURE, *PSTORAGE_DEVICE_LAYOUT_SIGNATURE;
+


These are available in storduid.h



  /* Get ID string from STORAGE_DEVICE_DESCRIPTIOR. */
  static bool
  stordesc_to_id_name (const UNICODE_STRING *upath, char *ioctl_buf,
char (& name)[NAME_MAX + 1])
  {
+  const STORAGE_DEVICE_UNIQUE_IDENTIFIER *id =
+reinterpret_cast(ioctl_buf);
+  char *desc_buf = ioctl_buf + id->StorageDeviceOffset;
const STORAGE_DEVICE_DESCRIPTOR *desc =
[...]
strcat (name, "_");
-  strcat (name, ioctl_buf + desc->SerialNumberOffset);
+  if (1) /* Utilize the DUID as defined by MSDN */
+{
+  unsigned long hash = 0;
+
+  for (ULONG i = 0; i < id->Size; ++i)
+   hash = ioctl_buf[i] + (hash << 6) + (hash << 16) - hash;
+  __small_sprintf (name + strlen (name), "%X", hash);
+}
+  else
+strcat (name, desc_buf + desc->SerialNumberOffset);
return true;
  }
  
@@ -212,7 +243,7 @@ get_by_id_table (by_id_entry * )

  /* Fetch vendor, product and serial number. */
  DWORD bytes_read;
  STORAGE_PROPERTY_QUERY query =
-   { StorageDeviceProperty, PropertyStandardQuery, { 0 } };
+   { StorageDeviceUniqueIdProperty, PropertyStandardQuery, { 0 } };
  if (!DeviceIoControl (devhdl, IOCTL_STORAGE_QUERY_PROPERTY,
, sizeof (query),
ioctl_buf, NT_MAX_PATH,
=== SNAP 


Thanks. Using this makes plenty of sense as a fallback if the serial 
number is unavailable. But if available, the serial number should be in 
the generated name as on Linux. This would provide a persistent name 
which reflects the actual device without a number invented by MS.


The serial number is usually available with (S)ATA and NVMe (namespace 
uuid in the latter case). I'm not familiar with QEMU/KVM details. The 
fact that both 'vendor' and 'product' are returned on your system 
suggests that a SCSI/SAS controller is emulated. Unlike (S)ATA and 

Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Corinna Vinschen
On Nov  3 12:10, Corinna Vinschen wrote:
> On Nov  3 11:09, Corinna Vinschen wrote:
> > On Nov  3 10:55, Corinna Vinschen wrote:
> > > On Oct  3 14:39, Christian Franke wrote:
> > > > According to NtQueryObject(., ObjectBasicInformation, ...), using
> > > > NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets 
> > > > GrantedAccess
> > > > to 0x001200a0 
> > > > (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
> > > > For some unknown reason, NVMe drives behind stornvme.sys additionally
> > > > require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a 
> > > > harmless
> > > > bug in the access check somewhere in the NVMe stack.
> > > > 
> > > > The disk scanning from the first patch has been reworked based on code
> > > > borrowed from proc.cc:format_proc_partitions(). For the longer term, it 
> > > > may
> > > > make sense to provide one flexible scanning function for /dev/sdXN,
> > > > /proc/partitions and /proc/disk/by-id.
> > > 
> > > I applied your patch locally (patch looks pretty well, btw) but found
> > > that /dev/disk/by-id is empty, even when running with admin rights.
> > > 
> > > I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM.  A
> > > \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
> > > exists in both cases.  I straced it, and found the following debug
> > > output:
> > > 
> > >   1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 
> > > 'Red_Hat' 'VirtIO' '' (ignored)
> > > 
> > > Is that really desired?
> 
> We could fake a serial number dependent on the path.  See
> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/mount.cc;hb=main#l253
> 
> Alternatively, there's also a symlink in GLOBAL?? pointing
> to the same target as \Device\Harddisk0\Partition0, i. e.:
> 
>   \Device\Harddisk0\Partition0 -> \Device\Harddisk0\DR0
> 
> and
> 
>   \GLOBAL??\Disk{4c622943-27e4-e81d-3fc7-c43fc9c7e126} -> 
> \Device\Harddisk0\DR0
> 
> We could use that UUID from that path, but that's quite a hassle
> to grab, because it requires to enumerate GLOBAL??.
> 
> But then again, where does Windows get the UUID from?  Something to
> find out...

I haven't found out where the UUID is coming from, yet, but based on the
description from
https://learn.microsoft.com/en-us/windows-hardware/drivers/storage/device-unique-identifiers--duids--for-storage-devices
I came up with this Q solution:

=== SNIP 
diff --git a/winsup/cygwin/fhandler/dev_disk.cc 
b/winsup/cygwin/fhandler/dev_disk.cc
index caca57d63216..74abfb8a3288 100644
--- a/winsup/cygwin/fhandler/dev_disk.cc
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -36,29 +36,51 @@ sanitize_id_string (char *s)
   return i;
 }
 
+typedef struct _STORAGE_DEVICE_UNIQUE_IDENTIFIER {
+  ULONG Version;
+  ULONG Size;
+  ULONG StorageDeviceIdOffset;
+  ULONG StorageDeviceOffset;
+  ULONG DriveLayoutSignatureOffset;
+} STORAGE_DEVICE_UNIQUE_IDENTIFIER, *PSTORAGE_DEVICE_UNIQUE_IDENTIFIER;
+
+typedef struct _STORAGE_DEVICE_LAYOUT_SIGNATURE {
+  ULONG   Version;
+  ULONG   Size;
+  BOOLEAN Mbr;
+  union {
+ULONG MbrSignature;
+GUID  GptDiskId;
+  } DeviceSpecific;
+} STORAGE_DEVICE_LAYOUT_SIGNATURE, *PSTORAGE_DEVICE_LAYOUT_SIGNATURE;
+
 /* Get ID string from STORAGE_DEVICE_DESCRIPTIOR. */
 static bool
 stordesc_to_id_name (const UNICODE_STRING *upath, char *ioctl_buf,
char (& name)[NAME_MAX + 1])
 {
+  const STORAGE_DEVICE_UNIQUE_IDENTIFIER *id =
+reinterpret_cast(ioctl_buf);
+  char *desc_buf = ioctl_buf + id->StorageDeviceOffset;
   const STORAGE_DEVICE_DESCRIPTOR *desc =
-reinterpret_cast(ioctl_buf);
+reinterpret_cast(desc_buf);
+
   /* Ignore drive if information is missing, too short or too long. */
   int vendor_len = 0, product_len = 0, serial_len = 0;
   if (desc->VendorIdOffset)
-vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
+vendor_len = sanitize_id_string (desc_buf + desc->VendorIdOffset);
   if (desc->ProductIdOffset)
-product_len = sanitize_id_string (ioctl_buf + desc->ProductIdOffset);
+product_len = sanitize_id_string (desc_buf + desc->ProductIdOffset);
   if (desc->SerialNumberOffset)
-serial_len = sanitize_id_string (ioctl_buf + desc->SerialNumberOffset);
+serial_len = sanitize_id_string (desc_buf + desc->SerialNumberOffset);
 
-  bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
-   && (20 + vendor_len + 1 + product_len + 1 + serial_len + 10)
+  bool valid = (4 <= vendor_len + product_len
+   && (20 + vendor_len + 1 + product_len + 1 + 10)
   < (int) sizeof (name));
   debug_printf ("%S: '%s' '%s' '%s'%s", upath,
-   (vendor_len ? ioctl_buf + desc->VendorIdOffset : ""),
-   (product_len ? ioctl_buf + desc->ProductIdOffset : ""),
-   (serial_len ? ioctl_buf + desc->SerialNumberOffset : ""),
+   (vendor_len ? desc_buf + desc->VendorIdOffset : ""),
+ 

Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Corinna Vinschen
On Nov  3 12:06, Christian Franke wrote:
> Corinna Vinschen wrote:
> > > I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM.  A
> > > \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
> > > exists in both cases.  I straced it, and found the following debug
> > > output:
> > > 
> > >1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 
> > > 'Red_Hat' 'VirtIO' '' (ignored)
> > > 
> > > Is that really desired?
> 
> Yes - if IOCTL_STORAGE_QUERY_PROPERTY{... PropertyStandardQuery} does not
> return a serial number (''), the device is intentionally ignored.

See my other mail I just sent.

> > Thread 1 "ls" hit Breakpoint 2, stordesc_to_id_name (upath=0x7c500,
> >  ioctl_buf=0x10e0720 "(", name=...)
> >  at 
> > /home/corinna/src/cygwin/vanilla/winsup/cygwin/fhandler/dev_disk.cc:44
> > 44const STORAGE_DEVICE_DESCRIPTOR *desc =
> > (gdb) n
> > 47int vendor_len = 0, product_len = 0, serial_len = 0;
> > (gdb)
> > 48if (desc->VendorIdOffset)
> > (gdb)
> > 49  vendor_len = sanitize_id_string (ioctl_buf + 
> > desc->VendorIdOffset);
> > (gdb)
> > 50if (desc->ProductIdOffset)
> > (gdb)
> > 51  product_len = sanitize_id_string (ioctl_buf + 
> > desc->ProductIdOffset);
> > (gdb)
> > 52if (desc->SerialNumberOffset)
> > (gdb)
> > 53  serial_len = sanitize_id_string (ioctl_buf + 
> > desc->SerialNumberOffset);
> 
> If possibly, please check whether (desc->SerialNumberOffset) is 0 or
> (ioctl_buf + desc->SerialNumberOffset) points to '\0' or a string of spaces.
> If no, there is possibly something wrong in sanitize_id_string().

desc->SerialNumberOffset is > 0, serial_len is 0, it points to an empty
string.


Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Corinna Vinschen
On Nov  3 11:09, Corinna Vinschen wrote:
> On Nov  3 10:55, Corinna Vinschen wrote:
> > On Oct  3 14:39, Christian Franke wrote:
> > > According to NtQueryObject(., ObjectBasicInformation, ...), using
> > > NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets 
> > > GrantedAccess
> > > to 0x001200a0 
> > > (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
> > > For some unknown reason, NVMe drives behind stornvme.sys additionally
> > > require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a 
> > > harmless
> > > bug in the access check somewhere in the NVMe stack.
> > > 
> > > The disk scanning from the first patch has been reworked based on code
> > > borrowed from proc.cc:format_proc_partitions(). For the longer term, it 
> > > may
> > > make sense to provide one flexible scanning function for /dev/sdXN,
> > > /proc/partitions and /proc/disk/by-id.
> > 
> > I applied your patch locally (patch looks pretty well, btw) but found
> > that /dev/disk/by-id is empty, even when running with admin rights.
> > 
> > I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM.  A
> > \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
> > exists in both cases.  I straced it, and found the following debug
> > output:
> > 
> >   1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 
> > 'Red_Hat' 'VirtIO' '' (ignored)
> > 
> > Is that really desired?

We could fake a serial number dependent on the path.  See
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/mount.cc;hb=main#l253

Alternatively, there's also a symlink in GLOBAL?? pointing
to the same target as \Device\Harddisk0\Partition0, i. e.:

  \Device\Harddisk0\Partition0 -> \Device\Harddisk0\DR0

and

  \GLOBAL??\Disk{4c622943-27e4-e81d-3fc7-c43fc9c7e126} -> \Device\Harddisk0\DR0

We could use that UUID from that path, but that's quite a hassle
to grab, because it requires to enumerate GLOBAL??.

But then again, where does Windows get the UUID from?  Something to
find out...


Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Christian Franke

Corinna Vinschen wrote:

On Nov  3 10:55, Corinna Vinschen wrote:

Hi Christian,

On Oct  3 14:39, Christian Franke wrote:

Christian Franke wrote:

This is a first attempt to partly emulate the Linux directory
/dev/disk/by-id. Useful to make sure the correct device is accessed in
conjunction with dd, ddrescue, fdisk, 

Attached is the second attempt.



The additional '*-partN' links to partitions are not yet included.

These are now included.



This only works properly if Win32 path '\\.\PhysicalDriveN' is always
trivially mapped to NT path '\Device\HarddiskN\Partition0'.
IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(.,
READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with all
drivers. With stornvme.sys, it fails with permission denied. Perhaps
other permission bits are required for NtOpenFile(). Thanks for any info
regarding this.

According to NtQueryObject(., ObjectBasicInformation, ...), using
NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
For some unknown reason, NVMe drives behind stornvme.sys additionally
require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
bug in the access check somewhere in the NVMe stack.

The disk scanning from the first patch has been reworked based on code
borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
make sense to provide one flexible scanning function for /dev/sdXN,
/proc/partitions and /proc/disk/by-id.

I applied your patch locally (patch looks pretty well, btw) but found


Thanks!
Meantime I found 4 missing NtClose() in the main loop of 
get_by_id_table(). Will be fixed in the next version of the patch.




that /dev/disk/by-id is empty, even when running with admin rights.


Admin rights should not be necessary for IOCTL_STORAGE_QUERY_PROPERTY.




I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM.  A
\Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
exists in both cases.  I straced it, and found the following debug
output:

   1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 
'Red_Hat' 'VirtIO' '' (ignored)

Is that really desired?


Yes - if IOCTL_STORAGE_QUERY_PROPERTY{... PropertyStandardQuery} does 
not return a serial number (''), the device is intentionally ignored.




Thread 1 "ls" hit Breakpoint 2, stordesc_to_id_name (upath=0x7c500,
 ioctl_buf=0x10e0720 "(", name=...)
 at /home/corinna/src/cygwin/vanilla/winsup/cygwin/fhandler/dev_disk.cc:44
44const STORAGE_DEVICE_DESCRIPTOR *desc =
(gdb) n
47int vendor_len = 0, product_len = 0, serial_len = 0;
(gdb)
48if (desc->VendorIdOffset)
(gdb)
49  vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
(gdb)
50if (desc->ProductIdOffset)
(gdb)
51  product_len = sanitize_id_string (ioctl_buf + 
desc->ProductIdOffset);
(gdb)
52if (desc->SerialNumberOffset)
(gdb)
53  serial_len = sanitize_id_string (ioctl_buf + 
desc->SerialNumberOffset);


If possibly, please check whether (desc->SerialNumberOffset) is 0 or 
(ioctl_buf + desc->SerialNumberOffset) points to '\0' or a string of 
spaces. If no, there is possibly something wrong in sanitize_id_string().




(gdb)
55bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb) p vendor_len
$1 = 7
(gdb) p product_len
$2 = 6
(gdb) p serial_len
$3 = 0
(gdb) n
[New Thread 3944.0x1958]
56  && (20 + vendor_len + 1 + product_len + 1 + serial_len 
+ 10)
(gdb) n
55bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb)
55bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb)
58debug_printf ("%S: '%s' '%s' '%s'%s", upath,
(gdb) p valid
$4 = false


--
Regards,
Christian



Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Corinna Vinschen
On Nov  3 10:55, Corinna Vinschen wrote:
> Hi Christian,
> 
> On Oct  3 14:39, Christian Franke wrote:
> > Christian Franke wrote:
> > > This is a first attempt to partly emulate the Linux directory
> > > /dev/disk/by-id. Useful to make sure the correct device is accessed in
> > > conjunction with dd, ddrescue, fdisk, 
> > 
> > Attached is the second attempt.
> > 
> > 
> > > The additional '*-partN' links to partitions are not yet included.
> > 
> > These are now included.
> > 
> > 
> > > This only works properly if Win32 path '\\.\PhysicalDriveN' is always
> > > trivially mapped to NT path '\Device\HarddiskN\Partition0'.
> > > IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(.,
> > > READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with all
> > > drivers. With stornvme.sys, it fails with permission denied. Perhaps
> > > other permission bits are required for NtOpenFile(). Thanks for any info
> > > regarding this.
> > 
> > According to NtQueryObject(., ObjectBasicInformation, ...), using
> > NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
> > to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
> > For some unknown reason, NVMe drives behind stornvme.sys additionally
> > require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
> > bug in the access check somewhere in the NVMe stack.
> > 
> > The disk scanning from the first patch has been reworked based on code
> > borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
> > make sense to provide one flexible scanning function for /dev/sdXN,
> > /proc/partitions and /proc/disk/by-id.
> 
> I applied your patch locally (patch looks pretty well, btw) but found
> that /dev/disk/by-id is empty, even when running with admin rights.
> 
> I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM.  A
> \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
> exists in both cases.  I straced it, and found the following debug
> output:
> 
>   1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 
> 'Red_Hat' 'VirtIO' '' (ignored)
> 
> Is that really desired?

Thread 1 "ls" hit Breakpoint 2, stordesc_to_id_name (upath=0x7c500,
ioctl_buf=0x10e0720 "(", name=...)
at /home/corinna/src/cygwin/vanilla/winsup/cygwin/fhandler/dev_disk.cc:44
44const STORAGE_DEVICE_DESCRIPTOR *desc =
(gdb) n
47int vendor_len = 0, product_len = 0, serial_len = 0;
(gdb)
48if (desc->VendorIdOffset)
(gdb)
49  vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
(gdb)
50if (desc->ProductIdOffset)
(gdb)
51  product_len = sanitize_id_string (ioctl_buf + 
desc->ProductIdOffset);
(gdb)
52if (desc->SerialNumberOffset)
(gdb)
53  serial_len = sanitize_id_string (ioctl_buf + 
desc->SerialNumberOffset);
(gdb)
55bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb) p vendor_len
$1 = 7
(gdb) p product_len
$2 = 6
(gdb) p serial_len
$3 = 0
(gdb) n
[New Thread 3944.0x1958]
56  && (20 + vendor_len + 1 + product_len + 1 + serial_len 
+ 10)
(gdb) n
55bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb)
55bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb)
58debug_printf ("%S: '%s' '%s' '%s'%s", upath,
(gdb) p valid
$4 = false


HTH,
Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-11-03 Thread Corinna Vinschen
Hi Christian,

On Oct  3 14:39, Christian Franke wrote:
> Christian Franke wrote:
> > This is a first attempt to partly emulate the Linux directory
> > /dev/disk/by-id. Useful to make sure the correct device is accessed in
> > conjunction with dd, ddrescue, fdisk, 
> 
> Attached is the second attempt.
> 
> 
> > The additional '*-partN' links to partitions are not yet included.
> 
> These are now included.
> 
> 
> > This only works properly if Win32 path '\\.\PhysicalDriveN' is always
> > trivially mapped to NT path '\Device\HarddiskN\Partition0'.
> > IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(.,
> > READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with all
> > drivers. With stornvme.sys, it fails with permission denied. Perhaps
> > other permission bits are required for NtOpenFile(). Thanks for any info
> > regarding this.
> 
> According to NtQueryObject(., ObjectBasicInformation, ...), using
> NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
> to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
> For some unknown reason, NVMe drives behind stornvme.sys additionally
> require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
> bug in the access check somewhere in the NVMe stack.
> 
> The disk scanning from the first patch has been reworked based on code
> borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
> make sense to provide one flexible scanning function for /dev/sdXN,
> /proc/partitions and /proc/disk/by-id.

I applied your patch locally (patch looks pretty well, btw) but found
that /dev/disk/by-id is empty, even when running with admin rights.

I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM.  A
\Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
exists in both cases.  I straced it, and found the following debug
output:

  1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 
'Red_Hat' 'VirtIO' '' (ignored)

Is that really desired?


Thanks,
Corinna


Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-10-03 Thread Christian Franke

Christian Franke wrote:
This is a first attempt to partly emulate the Linux directory 
/dev/disk/by-id. Useful to make sure the correct device is accessed in 
conjunction with dd, ddrescue, fdisk, 


Attached is the second attempt.



The additional '*-partN' links to partitions are not yet included.


These are now included.


This only works properly if Win32 path '\\.\PhysicalDriveN' is always 
trivially mapped to NT path '\Device\HarddiskN\Partition0'. 
IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(., 
READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with 
all drivers. With stornvme.sys, it fails with permission denied. 
Perhaps other permission bits are required for NtOpenFile(). Thanks 
for any info regarding this.


According to NtQueryObject(., ObjectBasicInformation, ...), using 
NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets 
GrantedAccess to 0x001200a0 
(FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE). For some 
unknown reason, NVMe drives behind stornvme.sys additionally require 
SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless bug 
in the access check somewhere in the NVMe stack.


The disk scanning from the first patch has been reworked based on code 
borrowed from proc.cc:format_proc_partitions(). For the longer term, it 
may make sense to provide one flexible scanning function for /dev/sdXN, 
/proc/partitions and /proc/disk/by-id.



Note that the BusType information is often not accurate. For example 
drives behind Intel RST drivers may always be listed as 
"raid-PRODUCT_SERIAL" even if not part of a RAID volume.


The changes for the generated file devices.cc are not included in the 
patch.



--
Regards,
Christian

From ba1990a43b7585f88a9369f5db0bdc56d6e913bf Mon Sep 17 00:00:00 2001
From: Christian Franke 
Date: Tue, 3 Oct 2023 14:15:12 +0200
Subject: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

The new directory '/dev/disk/by-id' provides symlinks for each
disk and its partitions:
'BUSTYPE-[VENDOR_]PRODUCT_SERIAL[-partN]' -> '../../sdX[N]'.
IOCTL_STORAGE_QUERY_PROPERTY is used to fetch bus type, vendor,
product and serial number information.  Administrator privileges
are not required.

Signed-off-by: Christian Franke 
---
 winsup/cygwin/Makefile.am   |   1 +
 winsup/cygwin/devices.in|   4 +
 winsup/cygwin/dtable.cc |   3 +
 winsup/cygwin/fhandler/dev_disk.cc  | 589 
 winsup/cygwin/local_includes/devices.h  |   6 +-
 winsup/cygwin/local_includes/fhandler.h |  47 ++
 winsup/cygwin/mount.cc  |  10 +
 7 files changed, 659 insertions(+), 1 deletion(-)
 create mode 100644 winsup/cygwin/fhandler/dev_disk.cc

diff --git a/winsup/cygwin/Makefile.am b/winsup/cygwin/Makefile.am
index 64b252a22..376c79fc3 100644
--- a/winsup/cygwin/Makefile.am
+++ b/winsup/cygwin/Makefile.am
@@ -84,6 +84,7 @@ FHANDLER_FILES= \
fhandler/console.cc \
fhandler/cygdrive.cc \
fhandler/dev.cc \
+   fhandler/dev_disk.cc \
fhandler/dev_fd.cc \
fhandler/disk_file.cc \
fhandler/dsp.cc \
diff --git a/winsup/cygwin/devices.in b/winsup/cygwin/devices.in
index 2545dd85e..48d3843fe 100644
--- a/winsup/cygwin/devices.in
+++ b/winsup/cygwin/devices.in
@@ -115,6 +115,9 @@ const _device dev_cygdrive_storage =
 const _device dev_fs_storage =
   {"", {FH_FS}, "", exists};
 
+const _device dev_dev_disk_storage =
+  {"", {FH_DEV_DISK}, "", exists};
+
 const _device dev_proc_storage =
   {"", {FH_PROC}, "", exists};
 
@@ -173,6 +176,7 @@ const _device dev_error_storage =
the POSIX namespace.  */
 %%
 "/dev", BRACK(FH_DEV), "", exists, S_IFDIR
+"/dev/disk", BRACK(FH_DEV_DISK), "", exists, S_IFDIR
 "/dev/tty", BRACK(FH_TTY), "/dev/tty", exists, S_IFCHR
 "/dev/pty%(0-127)d", BRACK(FHDEV(DEV_PTYS_MAJOR, {$1})), "/dev/pty{$1}", 
exists_pty, S_IFCHR, =ptys_dev
 ":ptym%(0-127)d", BRACK(FHDEV(DEV_PTYM_MAJOR, {$1})), "/dev/ptym{$1}", 
exists_internal, S_IFCHR, =ptym_dev
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 21d525389..9508f3e0b 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -585,6 +585,9 @@ fh_alloc (path_conv& pc)
case FH_DEV:
  fh = cnew (fhandler_dev);
  break;
+   case FH_DEV_DISK:
+ fh = cnew (fhandler_dev_disk);
+ break;
case FH_DEV_FD:
  fh = cnew (fhandler_dev_fd);
  break;
diff --git a/winsup/cygwin/fhandler/dev_disk.cc 
b/winsup/cygwin/fhandler/dev_disk.cc
new file mode 100644
index 0..caca57d63
--- /dev/null
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -0,0 +1,589 @@
+/* fhandler/dev_disk.cc: fhandler for the /dev/disk/by-id/... symlinks.
+
+This file is part of Cygwin.
+
+This software is a copyrighted

[PATCH] Cygwin: Add /dev/disk/by-id symlinks

2023-09-25 Thread Christian Franke
This is a first attempt to partly emulate the Linux directory 
/dev/disk/by-id. Useful to make sure the correct device is accessed in 
conjunction with dd, ddrescue, fdisk, 


The additional '*-partN' links to partitions are not yet included.

This only works properly if Win32 path '\\.\PhysicalDriveN' is always 
trivially mapped to NT path '\Device\HarddiskN\Partition0'. 
IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(., 
READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with all 
drivers. With stornvme.sys, it fails with permission denied. Perhaps 
other permission bits are required for NtOpenFile(). Thanks for any info 
regarding this.


Note that the BusType information is often not accurate. For example 
drives behind Intel RST drivers may always be listed as 
"raid-PRODUCT_SERIAL" even if not part of a RAID volume.


The changes for the generated file devices.cc are not included in the patch.

--
Regards,
Christian

From 8a49ae067cfd318746e6fe332b8775011658d780 Mon Sep 17 00:00:00 2001
From: Christian Franke 
Date: Mon, 25 Sep 2023 13:24:40 +0200
Subject: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

The new directory '/dev/disk/by-id' provides symlinks for each
disk based on information from IOCTL_STORAGE_QUERY_PROPERTY:
'BUSTYPE_[VENDOR_]PRODUCT_SERIAL' -> '../../sdX'

Signed-off-by: Christian Franke 
---
 winsup/cygwin/Makefile.am   |   1 +
 winsup/cygwin/devices.in|   4 +
 winsup/cygwin/dtable.cc |   3 +
 winsup/cygwin/fhandler/dev_disk.cc  | 389 
 winsup/cygwin/local_includes/devices.h  |   6 +-
 winsup/cygwin/local_includes/fhandler.h |  45 +++
 winsup/cygwin/mount.cc  |  10 +
 7 files changed, 457 insertions(+), 1 deletion(-)
 create mode 100644 winsup/cygwin/fhandler/dev_disk.cc

diff --git a/winsup/cygwin/Makefile.am b/winsup/cygwin/Makefile.am
index 64b252a22..376c79fc3 100644
--- a/winsup/cygwin/Makefile.am
+++ b/winsup/cygwin/Makefile.am
@@ -84,6 +84,7 @@ FHANDLER_FILES= \
fhandler/console.cc \
fhandler/cygdrive.cc \
fhandler/dev.cc \
+   fhandler/dev_disk.cc \
fhandler/dev_fd.cc \
fhandler/disk_file.cc \
fhandler/dsp.cc \
diff --git a/winsup/cygwin/devices.in b/winsup/cygwin/devices.in
index 2545dd85e..48d3843fe 100644
--- a/winsup/cygwin/devices.in
+++ b/winsup/cygwin/devices.in
@@ -115,6 +115,9 @@ const _device dev_cygdrive_storage =
 const _device dev_fs_storage =
   {"", {FH_FS}, "", exists};
 
+const _device dev_dev_disk_storage =
+  {"", {FH_DEV_DISK}, "", exists};
+
 const _device dev_proc_storage =
   {"", {FH_PROC}, "", exists};
 
@@ -173,6 +176,7 @@ const _device dev_error_storage =
the POSIX namespace.  */
 %%
 "/dev", BRACK(FH_DEV), "", exists, S_IFDIR
+"/dev/disk", BRACK(FH_DEV_DISK), "", exists, S_IFDIR
 "/dev/tty", BRACK(FH_TTY), "/dev/tty", exists, S_IFCHR
 "/dev/pty%(0-127)d", BRACK(FHDEV(DEV_PTYS_MAJOR, {$1})), "/dev/pty{$1}", 
exists_pty, S_IFCHR, =ptys_dev
 ":ptym%(0-127)d", BRACK(FHDEV(DEV_PTYM_MAJOR, {$1})), "/dev/ptym{$1}", 
exists_internal, S_IFCHR, =ptym_dev
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 21d525389..9508f3e0b 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -585,6 +585,9 @@ fh_alloc (path_conv& pc)
case FH_DEV:
  fh = cnew (fhandler_dev);
  break;
+   case FH_DEV_DISK:
+ fh = cnew (fhandler_dev_disk);
+ break;
case FH_DEV_FD:
  fh = cnew (fhandler_dev_fd);
  break;
diff --git a/winsup/cygwin/fhandler/dev_disk.cc 
b/winsup/cygwin/fhandler/dev_disk.cc
new file mode 100644
index 0..14b0cdf36
--- /dev/null
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -0,0 +1,389 @@
+/* fhandler/dev_disk.cc: fhandler for the /dev/disk/by-id/... symlinks.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#include "winsup.h"
+#include "path.h"
+#include "fhandler.h"
+#include 
+
+/* Replace non-printing and unexpected characters, remove trailing spaces,
+   return remaining string length. */
+static int
+sanitize_id_string (char *s)
+{
+  int lastspace = -1, i;
+  for (i = 0; s[i]; i++)
+{
+  char c = s[i];
+  if (c != ' ')
+   lastspace = -1;
+  else if (lastspace < 0)
+   lastspace = i;
+  if (('0' <= c && c <= '9') || c == '.' || c == '-'
+ || ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'))
+   continue;
+  s[i] = '_';
+}
+  if (lastspace >= 0)
+s[(i = lastspace)] = '\0';
+  return i;
+}
+
+/* Get ID string for drive number. */