[PATCH] Cygwin: Fix access to block devices below /proc/sys.

[PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

There a very few but occasionally interesting use cases for read access 
to block devices below /proc/sys:

- Read raw images behind drive letters which are not linked to regular 
/dev/sdXN partitions. For example read decrypted images of VeraCrypt 
partitions or container files:
/proc/sys/DosDevices/X: -> /proc/sys/Device/VeraCryptVolumeX

- Read raw images of Volume Shadow Copies:
/proc/sys/Device/HarddiskVolumeShadowCopy*

Copying such an image actually works with 'dd', but 'ddrescue' reports a 
non seekable device. This is because fhandler_virtual::lseek() is used. 
It calls fhandler_procsys::fill_filebuf() which does not make any sense 
in this context. This lseek() always fails - without setting errno, BTW.

The attached experimental patch does not fix the lseek() (sorry), but 
handles such block devices with fhandler_dev_floppy instead. Tested with 
above use cases.

I'm not sure whether this could break access to other /proc/sys block 
devices. This would happen if fh->exists() returns virt_blk for devices 
which do not support IOCTL_DISK_GET_DRIVE_GEOMETRY* or 
IOCTL_DISK_GET_PARTITION_INFO*.

Regards,
Christian


[-- Attachment #2: 0001-Cygwin-Fix-access-to-block-devices-below-proc-sys.patch --]
[-- Type: text/plain, Size: 2138 bytes --]

From ed8f419524fc81a378280579ec3c23af527d4772 Mon Sep 17 00:00:00 2001
From: Christian Franke <franke@computer.org>
Date: Sat, 28 Nov 2020 22:09:23 +0100
Subject: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

Use fhandler_dev_floppy instead of fhandler_procsys for such devices.
The read()/write() functions from fhandler_procsys do not ensure
sector aligned transfers and lseek() fails always.

Signed-off-by: Christian Franke <franke@computer.org>
---
 winsup/cygwin/path.cc | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 4f5f03a76..7e6243d32 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -863,19 +863,28 @@ path_conv::check (const char *src, unsigned opt,
 			dev.parse (FH_FS);
 			goto is_fs_via_procsys;
 		      case virt_blk:
-			/* Block special device.  If the trailing slash has been
-			   requested, the target is the root directory of the
-			   filesystem on this block device.  So we convert this
-			   to a real file and attach the backslash. */
-			if (component == 0 && need_directory)
+			/* Block special device.  Convert to a /dev/sd* like
+			   block device unless the trailing slash has been
+			   requested.  In this case, the target is the root
+			   directory of the filesystem on this block device.
+			   So we convert this to a real file and attach the
+			   backslash. */
+			if (component == 0)
 			  {
-			    dev.parse (FH_FS);
-			    strcat (full_path, "\\");
-			    fileattr = FILE_ATTRIBUTE_DIRECTORY
-				       | FILE_ATTRIBUTE_DEVICE;
+			    fileattr = FILE_ATTRIBUTE_DEVICE;
+			    if (!need_directory)
+			      /* Use a /dev/sd* device number > /dev/sddx.
+				 FIXME: Define a new major DEV_ice number. */
+			      dev.parse (DEV_SD_HIGHPART_END, 9999);
+			    else
+			      {
+				dev.parse (FH_FS);
+				strcat (full_path, "\\");
+				fileattr |= FILE_ATTRIBUTE_DIRECTORY;
+			      }
 			    goto out;
 			  }
-			fallthrough;
+			break;
 		      case virt_chr:
 			if (component == 0)
 			  fileattr = FILE_ATTRIBUTE_DEVICE;
-- 
2.29.2

Re: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Corinna Vinschen]

On Nov 28 22:59, Christian Franke wrote:
> There a very few but occasionally interesting use cases for read access to
> block devices below /proc/sys:
> 
> - Read raw images behind drive letters which are not linked to regular
> /dev/sdXN partitions. For example read decrypted images of VeraCrypt
> partitions or container files:
> /proc/sys/DosDevices/X: -> /proc/sys/Device/VeraCryptVolumeX
> 
> - Read raw images of Volume Shadow Copies:
> /proc/sys/Device/HarddiskVolumeShadowCopy*
> 
> Copying such an image actually works with 'dd', but 'ddrescue' reports a non
> seekable device. This is because fhandler_virtual::lseek() is used. It calls
> fhandler_procsys::fill_filebuf() which does not make any sense in this
> context. This lseek() always fails - without setting errno, BTW.
> 
> The attached experimental patch does not fix the lseek() (sorry), but
> handles such block devices with fhandler_dev_floppy instead. Tested with
> above use cases.
> 
> I'm not sure whether this could break access to other /proc/sys block
> devices. This would happen if fh->exists() returns virt_blk for devices
> which do not support IOCTL_DISK_GET_DRIVE_GEOMETRY* or
> IOCTL_DISK_GET_PARTITION_INFO*.

Pushed, becasue it's a nice idea.  The above problem shouldn't happen,
in theory, but I'm not sure.  virt_blk is generated for devices types

  FILE_DEVICE_DISK
  FILE_DEVICE_CD_ROM
  FILE_DEVICE_VIRTUAL_DISK
  FILE_DEVICE_DFS
  FILE_DEVICE_NETWORK_FILE_SYSTEM

FILE_DEVICE_DFS or FILE_DEVICE_NETWORK_FILE_SYSTEM might be a problem,
but there should be a way to workaround that, if necessary, isn't it?
Maybe it's a bad idea to treat those as blk devices at all?


Thanks,
Corinna

Re: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Christian Franke]

Corinna Vinschen wrote:
> On Nov 28 22:59, Christian Franke wrote:
>> ...
>> The attached experimental patch does not fix the lseek() (sorry), but
>> handles such block devices with fhandler_dev_floppy instead. Tested with
>> above use cases.
>>
>> I'm not sure whether this could break access to other /proc/sys block
>> devices. This would happen if fh->exists() returns virt_blk for devices
>> which do not support IOCTL_DISK_GET_DRIVE_GEOMETRY* or
>> IOCTL_DISK_GET_PARTITION_INFO*.
> Pushed, becasue it's a nice idea.  The above problem shouldn't happen,
> in theory, but I'm not sure.  virt_blk is generated for devices types
>
>    FILE_DEVICE_DISK
>    FILE_DEVICE_CD_ROM
>    FILE_DEVICE_VIRTUAL_DISK
>    FILE_DEVICE_DFS
>    FILE_DEVICE_NETWORK_FILE_SYSTEM
>
> FILE_DEVICE_DFS or FILE_DEVICE_NETWORK_FILE_SYSTEM might be a problem,
> but there should be a way to workaround that, if necessary, isn't it?
> Maybe it's a bad idea to treat those as blk devices at all?

Could anything be read from such a node? If yes, treat as character device?

If no and /proc/sys/foo/bar/some/path allows access to /some/path behind 
DFS/NFS node /proc/sys/foo/bar, then treat as directory?

This is already the case for SMB shares:

$ ls -ld /proc/sys/DosDevices/X:
lr-------- 1 ... 0 Nov 30 13:10 /proc/sys/DosDevices/X: ->
   /proc/sys/Device/LanmanRedirector/;X:..../127.0.0.1/Share

$ ls -lLd /proc/sys/DosDevices/X:
drwxr-xr-x 1 ... 0 Nov 14 09:06 /proc/sys/DosDevices/X:

$ ls -L /proc/sys/DosDevices/X:
... files on this share

Thanks,
Christian

Re: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Corinna Vinschen]

On Nov 30 13:49, Christian Franke wrote:
> Corinna Vinschen wrote:
> > On Nov 28 22:59, Christian Franke wrote:
> > > ...
> > > The attached experimental patch does not fix the lseek() (sorry), but
> > > handles such block devices with fhandler_dev_floppy instead. Tested with
> > > above use cases.
> > > 
> > > I'm not sure whether this could break access to other /proc/sys block
> > > devices. This would happen if fh->exists() returns virt_blk for devices
> > > which do not support IOCTL_DISK_GET_DRIVE_GEOMETRY* or
> > > IOCTL_DISK_GET_PARTITION_INFO*.
> > Pushed, becasue it's a nice idea.  The above problem shouldn't happen,
> > in theory, but I'm not sure.  virt_blk is generated for devices types
> > 
> >    FILE_DEVICE_DISK
> >    FILE_DEVICE_CD_ROM
> >    FILE_DEVICE_VIRTUAL_DISK
> >    FILE_DEVICE_DFS
> >    FILE_DEVICE_NETWORK_FILE_SYSTEM
> > 
> > FILE_DEVICE_DFS or FILE_DEVICE_NETWORK_FILE_SYSTEM might be a problem,
> > but there should be a way to workaround that, if necessary, isn't it?
> > Maybe it's a bad idea to treat those as blk devices at all?
> 
> Could anything be read from such a node? If yes, treat as character device?
> 
> If no and /proc/sys/foo/bar/some/path allows access to /some/path behind
> DFS/NFS node /proc/sys/foo/bar, then treat as directory?
> 
> This is already the case for SMB shares:
> 
> $ ls -ld /proc/sys/DosDevices/X:
> lr-------- 1 ... 0 Nov 30 13:10 /proc/sys/DosDevices/X: ->
>   /proc/sys/Device/LanmanRedirector/;X:..../127.0.0.1/Share
> 
> $ ls -lLd /proc/sys/DosDevices/X:
> drwxr-xr-x 1 ... 0 Nov 14 09:06 /proc/sys/DosDevices/X:
> 
> $ ls -L /proc/sys/DosDevices/X:
> ... files on this share

...and it's already the case for NFS shares, too:

$ ls -ld /proc/sys/DosDevices/Y:
lr-------- 1 corinna vinschen 0 Nov 30 14:59 /proc/sys/DosDevices/Y: -> /proc/sys/Device/MRxNfs/;Y:00000000001cb27f/...

$ ls -lLd /proc/sys/DosDevices/Y:
drwxr-xr-x 5 corinna vinschen 41 May 19  2016 /proc/sys/DosDevices/Y:

That means we don't have to handle FILE_DEVICE_NETWORK_FILE_SYSTEM in
the code creating the virt_blk device type at all.  I have high hopes
this is the same for DFS, albeit I can't test it...


Thanks,
Corinna

Re: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Corinna Vinschen]

On Nov 30 15:04, Corinna Vinschen wrote:
> On Nov 30 13:49, Christian Franke wrote:
> > Corinna Vinschen wrote:
> > > On Nov 28 22:59, Christian Franke wrote:
> > > > ...
> > > > The attached experimental patch does not fix the lseek() (sorry), but
> > > > handles such block devices with fhandler_dev_floppy instead. Tested with
> > > > above use cases.
> > > > 
> > > > I'm not sure whether this could break access to other /proc/sys block
> > > > devices. This would happen if fh->exists() returns virt_blk for devices
> > > > which do not support IOCTL_DISK_GET_DRIVE_GEOMETRY* or
> > > > IOCTL_DISK_GET_PARTITION_INFO*.
> > > Pushed, becasue it's a nice idea.  The above problem shouldn't happen,
> > > in theory, but I'm not sure.  virt_blk is generated for devices types
> > > 
> > >    FILE_DEVICE_DISK
> > >    FILE_DEVICE_CD_ROM
> > >    FILE_DEVICE_VIRTUAL_DISK
> > >    FILE_DEVICE_DFS
> > >    FILE_DEVICE_NETWORK_FILE_SYSTEM
> > > 
> > > FILE_DEVICE_DFS or FILE_DEVICE_NETWORK_FILE_SYSTEM might be a problem,
> > > but there should be a way to workaround that, if necessary, isn't it?
> > > Maybe it's a bad idea to treat those as blk devices at all?
> > 
> > Could anything be read from such a node? If yes, treat as character device?
> > 
> > If no and /proc/sys/foo/bar/some/path allows access to /some/path behind
> > DFS/NFS node /proc/sys/foo/bar, then treat as directory?
> > 
> > This is already the case for SMB shares:
> > 
> > $ ls -ld /proc/sys/DosDevices/X:
> > lr-------- 1 ... 0 Nov 30 13:10 /proc/sys/DosDevices/X: ->
> >   /proc/sys/Device/LanmanRedirector/;X:..../127.0.0.1/Share
> > 
> > $ ls -lLd /proc/sys/DosDevices/X:
> > drwxr-xr-x 1 ... 0 Nov 14 09:06 /proc/sys/DosDevices/X:
> > 
> > $ ls -L /proc/sys/DosDevices/X:
> > ... files on this share
> 
> ...and it's already the case for NFS shares, too:
> 
> $ ls -ld /proc/sys/DosDevices/Y:
> lr-------- 1 corinna vinschen 0 Nov 30 14:59 /proc/sys/DosDevices/Y: -> /proc/sys/Device/MRxNfs/;Y:00000000001cb27f/...
> 
> $ ls -lLd /proc/sys/DosDevices/Y:
> drwxr-xr-x 5 corinna vinschen 41 May 19  2016 /proc/sys/DosDevices/Y:
> 
> That means we don't have to handle FILE_DEVICE_NETWORK_FILE_SYSTEM in
> the code creating the virt_blk device type at all.  I have high hopes
> this is the same for DFS, albeit I can't test it...

Oh, right, I just realized that \Device\MRxNfs, as well as
\Device\LanmanRedirector are symlinks pointing below \Device\Mup:

$ ls -l /proc/sys/Device/LanmanRedirector
lr--r--r-- 1 Administrators SYSTEM 0 Nov 30 15:20 /proc/sys/Device/LanmanRedirector -> /proc/sys/Device/Mup/;LanmanRedirector

$ ls -l /proc/sys/Device/MRxNfs
lr--r--r-- 1 Administrators SYSTEM 0 Nov 30 15:20 /proc/sys/Device/MRxNfs -> /proc/sys/Device/Mup/;MRxNfs

\Device\Mup is a character device and thus the devices below are not
accessible for directory enumeration.  I assume it's the same for DFS.


Corinna

Re: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Christian Franke]

Corinna Vinschen wrote:
> On Nov 30 15:04, Corinna Vinschen wrote:
>> On Nov 30 13:49, Christian Franke wrote:
>>> Corinna Vinschen wrote:
>>>> On Nov 28 22:59, Christian Franke wrote:
>>>>> ...
>>>>> The attached experimental patch does not fix the lseek() (sorry), but
>>>>> handles such block devices with fhandler_dev_floppy instead. Tested with
>>>>> above use cases.
>>>>>
>>>>> I'm not sure whether this could break access to other /proc/sys block
>>>>> devices. This would happen if fh->exists() returns virt_blk for devices
>>>>> which do not support IOCTL_DISK_GET_DRIVE_GEOMETRY* or
>>>>> IOCTL_DISK_GET_PARTITION_INFO*.
>>>> Pushed, becasue it's a nice idea.  The above problem shouldn't happen,
>>>> in theory, but I'm not sure.  virt_blk is generated for devices types
>>>>
>>>>     FILE_DEVICE_DISK
>>>>     FILE_DEVICE_CD_ROM
>>>>     FILE_DEVICE_VIRTUAL_DISK
>>>>     FILE_DEVICE_DFS
>>>>     FILE_DEVICE_NETWORK_FILE_SYSTEM
>>>>
>>>> FILE_DEVICE_DFS or FILE_DEVICE_NETWORK_FILE_SYSTEM might be a problem,
>>>> but there should be a way to workaround that, if necessary, isn't it?
>>>> Maybe it's a bad idea to treat those as blk devices at all?
>>> Could anything be read from such a node? If yes, treat as character device?
>>>
>>> If no and /proc/sys/foo/bar/some/path allows access to /some/path behind
>>> DFS/NFS node /proc/sys/foo/bar, then treat as directory?
>>>
>>> This is already the case for SMB shares:
>>>
>>> $ ls -ld /proc/sys/DosDevices/X:
>>> lr-------- 1 ... 0 Nov 30 13:10 /proc/sys/DosDevices/X: ->
>>>    /proc/sys/Device/LanmanRedirector/;X:..../127.0.0.1/Share
>>>
>>> $ ls -lLd /proc/sys/DosDevices/X:
>>> drwxr-xr-x 1 ... 0 Nov 14 09:06 /proc/sys/DosDevices/X:
>>>
>>> $ ls -L /proc/sys/DosDevices/X:
>>> ... files on this share
>> ...and it's already the case for NFS shares, too:
>>
>> $ ls -ld /proc/sys/DosDevices/Y:
>> lr-------- 1 corinna vinschen 0 Nov 30 14:59 /proc/sys/DosDevices/Y: -> /proc/sys/Device/MRxNfs/;Y:00000000001cb27f/...
>>
>> $ ls -lLd /proc/sys/DosDevices/Y:
>> drwxr-xr-x 5 corinna vinschen 41 May 19  2016 /proc/sys/DosDevices/Y:
>>
>> That means we don't have to handle FILE_DEVICE_NETWORK_FILE_SYSTEM in
>> the code creating the virt_blk device type at all.  I have high hopes
>> this is the same for DFS, albeit I can't test it...
> Oh, right, I just realized that \Device\MRxNfs, as well as
> \Device\LanmanRedirector are symlinks pointing below \Device\Mup:
>
> $ ls -l /proc/sys/Device/LanmanRedirector
> lr--r--r-- 1 Administrators SYSTEM 0 Nov 30 15:20 /proc/sys/Device/LanmanRedirector -> /proc/sys/Device/Mup/;LanmanRedirector
>
> $ ls -l /proc/sys/Device/MRxNfs
> lr--r--r-- 1 Administrators SYSTEM 0 Nov 30 15:20 /proc/sys/Device/MRxNfs -> /proc/sys/Device/Mup/;MRxNfs
>
> \Device\Mup is a character device and thus the devices below are not
> accessible for directory enumeration.  I assume it's the same for DFS.


Here I see \Device\Mup as a block device on two systems (cygwin1.dll 3.1.7):

$ ls -l /proc/sys/Device/Mup
brwxrwx--x 1 Administrators SYSTEM 0, 250 Dec  1 16:50 /proc/sys/Device/Mup

Device could be opened for reading, but actual read fails with NTSTATUS 
STATUS_INVALID_DEVICE_REQUEST.

Any path which do not exist produce misleading results:

$ ls -l /proc/sys/Device/Mup/no.such.file
crw-rw---- 1 Administrators SYSTEM 0, 250 Dec  1 16:52 
/proc/sys/Device/Mup/no.such.file


Thanks,
Christian

Re: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Corinna Vinschen]

On Dec  1 16:59, Christian Franke wrote:
> Corinna Vinschen wrote:
> > \Device\Mup is a character device and thus the devices below are not
> > accessible for directory enumeration.  I assume it's the same for DFS.
> 
> Here I see \Device\Mup as a block device on two systems (cygwin1.dll 3.1.7):
> 
> $ ls -l /proc/sys/Device/Mup
> brwxrwx--x 1 Administrators SYSTEM 0, 250 Dec  1 16:50 /proc/sys/Device/Mup

Huh?

$ ls -l /proc/sys/Device/Mup
crwxrwx--x 1 Administrators SYSTEM 0, 250 Dec  1 17:04 /proc/sys/Device/Mup

This is what I'd expect.  Can you debug why this is a block device
on your systems?


Thanks,
Corinna

Re: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Christian Franke]

Corinna Vinschen wrote:
> On Dec  1 16:59, Christian Franke wrote:
>> Corinna Vinschen wrote:
>>> \Device\Mup is a character device and thus the devices below are not
>>> accessible for directory enumeration.  I assume it's the same for DFS.
>> Here I see \Device\Mup as a block device on two systems (cygwin1.dll 3.1.7):
>>
>> $ ls -l /proc/sys/Device/Mup
>> brwxrwx--x 1 Administrators SYSTEM 0, 250 Dec  1 16:50 /proc/sys/Device/Mup
> Huh?
>
> $ ls -l /proc/sys/Device/Mup
> crwxrwx--x 1 Administrators SYSTEM 0, 250 Dec  1 17:04 /proc/sys/Device/Mup
>
> This is what I'd expect.  Can you debug why this is a block device
> on your systems?
>

NtQueryVolumeInformationFile() returns {DeviceType = 0x14, 
Characteristics = 0x20010}

fhandler_procsys::exists(...):
...
   status = NtOpenFile (&h, READ_CONTROL | FILE_READ_ATTRIBUTES, &attr, &io,
                   FILE_SHARE_VALID_FLAGS, FILE_OPEN_FOR_BACKUP_INTENT);
...
   if (NT_SUCCESS (status))
     {
       FILE_FS_DEVICE_INFORMATION ffdi;
...
       /* Check for the device type. */
       status = NtQueryVolumeInformationFile (h, &io, &ffdi, sizeof ffdi,
                          FileFsDeviceInformation);
...
       if (NT_SUCCESS (status))
       {
         if (ffdi.DeviceType == FILE_DEVICE_NETWORK_FILE_SYSTEM)
           file_type = virt_blk;  <<===============
        ...


Thanks,
Christian

Re: [PATCH] Cygwin: Fix access to block devices below /proc/sys.

[Corinna Vinschen]

On Dec  1 19:48, Christian Franke wrote:
> Corinna Vinschen wrote:
> > On Dec  1 16:59, Christian Franke wrote:
> > > Corinna Vinschen wrote:
> > > > \Device\Mup is a character device and thus the devices below are not
> > > > accessible for directory enumeration.  I assume it's the same for DFS.
> > > Here I see \Device\Mup as a block device on two systems (cygwin1.dll 3.1.7):
> > > 
> > > $ ls -l /proc/sys/Device/Mup
> > > brwxrwx--x 1 Administrators SYSTEM 0, 250 Dec  1 16:50 /proc/sys/Device/Mup
> > Huh?
> > 
> > $ ls -l /proc/sys/Device/Mup
> > crwxrwx--x 1 Administrators SYSTEM 0, 250 Dec  1 17:04 /proc/sys/Device/Mup
> > 
> > This is what I'd expect.  Can you debug why this is a block device
> > on your systems?
> > 
> 
> NtQueryVolumeInformationFile() returns {DeviceType = 0x14, Characteristics =
> 0x20010}
> 
> fhandler_procsys::exists(...):
> ...
>   status = NtOpenFile (&h, READ_CONTROL | FILE_READ_ATTRIBUTES, &attr, &io,
>                   FILE_SHARE_VALID_FLAGS, FILE_OPEN_FOR_BACKUP_INTENT);
> ...
>   if (NT_SUCCESS (status))
>     {
>       FILE_FS_DEVICE_INFORMATION ffdi;
> ...
>       /* Check for the device type. */
>       status = NtQueryVolumeInformationFile (h, &io, &ffdi, sizeof ffdi,
>                          FileFsDeviceInformation);
> ...
>       if (NT_SUCCESS (status))
>       {
>         if (ffdi.DeviceType == FILE_DEVICE_NETWORK_FILE_SYSTEM)
>           file_type = virt_blk;  <<===============
>        ...


Uh... ok, that's what had changed with commit 80e35a211f69 as of
this morning :}


Corinna