Re: [ros-dev] [ros-diffs] [cfinck] 75630: [FS_REC] - Add a detection routine for CDFS (ISO-9660) volumes that verifies the Primary Volume Descriptor. - Use this to also detect CDFS on disks and load the CDFS driver if it ha...

[Pierre Schweitzer]

Hi Colin,

Thanks for your work. This commit is likely breaking UDFS recognition
when media has a protective CDFS partition with UDFS afterwards, while
both FSD are unloaded.
You'll first match the CDFS recognizer and not the UDFS recognizer.

I would suggest you modify FsRecRegisterFs() so that you can register
low priority FS.

Cheers,
Pierre

Le 20/08/2017 à 17:30, [e-mail] a écrit :
> Author: cfinck
> Date: Sun Aug 20 15:30:59 2017
> New Revision: 75630
>
> URL: http://svn.reactos.org/svn/reactos?rev=75630&view=rev
> Log:
> [FS_REC]
> Add a detection routine for CDFS (ISO-9660) volumes that verifies the Primary Volume Descriptor.
> Use this to also detect CDFS on disks and load the CDFS driver if it has not been loaded yet (e.g. when a bootcd/livecd flashed USB drive is inserted at boot of an installed ReactOS).
> Fix a comment in udfs.c.
>
> Added:
>     trunk/reactos/drivers/filesystems/fs_rec/cdfs.h   (with props)
> Modified:
>     trunk/reactos/drivers/filesystems/fs_rec/cdfs.c
>     trunk/reactos/drivers/filesystems/fs_rec/fs_rec.c
>     trunk/reactos/drivers/filesystems/fs_rec/udfs.c
>
> Modified: trunk/reactos/drivers/filesystems/fs_rec/cdfs.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fs_rec/...
> ==============================================================================
> --- trunk/reactos/drivers/filesystems/fs_rec/cdfs.c	[iso-8859-1] (original)
> +++ trunk/reactos/drivers/filesystems/fs_rec/cdfs.c	[iso-8859-1] Sun Aug 20 15:30:59 2017
> @@ -1,10 +1,10 @@
>  /*
> COPYRIGHT:        See COPYING in the top level directory
> PROJECT:          ReactOS File System Recognizer
> FILE:             drivers/filesystems/fs_rec/cdfs.c
> PURPOSE:          CDFS Recognizer
> PROGRAMMER:       Alex Ionescu ([e-mail])
>               Eric Kohl
> PROJECT:     ReactOS File System Recognizer
> LICENSE:     GPL-2.0 (https://spdx.org/licenses/GPL-2.0)
> PURPOSE:     CDFS Recognizer
> COPYRIGHT:   Copyright 2002 Eric Kohl
>          Copyright 2007 Alex Ionescu <[e-mail]>
>          Copyright 2017 Colin Finck <[e-mail]>
> */
>
> /* INCLUDES *****************************************************************/
> @@ -14,13 +14,60 @@
>  #define NDEBUG
>  #include <debug.h>
>
> +#include "cdfs.h"
>
> /* FUNCTIONS ****************************************************************/
>
> +BOOLEAN
> +NTAPI
> +FsRecIsCdfsVolume(IN PDEVICE_OBJECT DeviceObject,
>              IN ULONG SectorSize)
>
> +{
> BOOLEAN bReturnValue = FALSE;
> LARGE_INTEGER Offset;
> PVD_HEADER pVdHeader = NULL;
> PAGED_CODE();
> // Read the Primary Volume Descriptor.
> Offset.QuadPart = VD_HEADER_OFFSET;
> if (!FsRecReadBlock(DeviceObject, &Offset, sizeof(VD_HEADER), SectorSize, (PVOID*)&pVdHeader, NULL))
> {
>    // We cannot read this block, so no reason to let the CDFS driver try it.
>    goto Cleanup;
> }
> // Verify the fields.
> if (pVdHeader->Type != VD_TYPE_PRIMARY)
>    goto Cleanup;
> DPRINT("pVdHeader->Type verified!\n");
> if (RtlCompareMemory(pVdHeader->Identifier, VD_IDENTIFIER, VD_IDENTIFIER_LENGTH) != VD_IDENTIFIER_LENGTH)
>    goto Cleanup;
> DPRINT("pVdHeader->Identifier verified!\n");
> if (pVdHeader->Version != VD_VERSION)
>    goto Cleanup;
> DPRINT("pVdHeader->Version verified! This is a CDFS volume!\n");
> bReturnValue = TRUE;
>
> +Cleanup:
> if (pVdHeader)
>    ExFreePool(pVdHeader);
> return bReturnValue;
>
> +}
>
> NTSTATUS
>  NTAPI
>  FsRecCdfsFsControl(IN PDEVICE_OBJECT DeviceObject,
>                     IN PIRP Irp)
>  {
> PDEVICE_OBJECT MountDevice;
> ULONG SectorSize;
> PIO_STACK_LOCATION Stack;
> NTSTATUS Status;
> PAGED_CODE();
>
> @@ -31,8 +78,21 @@
>      {
>          case IRP_MN_MOUNT_VOLUME:
>        /* We don't validate */
>        Status = STATUS_FS_DRIVER_REQUIRED;
>        /* Assume failure */
>        Status = STATUS_UNRECOGNIZED_VOLUME;
>        /* Get the device object and request the sector size */
>        MountDevice = Stack->Parameters.MountVolume.DeviceObject;
>        if (FsRecGetDeviceSectorSize(MountDevice, &SectorSize))
>        {
>            /* Check if it's an actual CDFS (ISO-9660) volume */
>            if (FsRecIsCdfsVolume(MountDevice, SectorSize))
>            {
>                /* It is! */
>                Status = STATUS_FS_DRIVER_REQUIRED;
>            }
>        }
>        break;
>
>    case IRP_MN_LOAD_FILE_SYSTEM:
>
> Added: trunk/reactos/drivers/filesystems/fs_rec/cdfs.h
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fs_rec/...
> ==============================================================================
> --- trunk/reactos/drivers/filesystems/fs_rec/cdfs.h	(added)
> +++ trunk/reactos/drivers/filesystems/fs_rec/cdfs.h	[iso-8859-1] Sun Aug 20 15:30:59 2017
> @@ -0,0 +1,24 @@
> +/*
> PROJECT:     ReactOS File System Recognizer
> LICENSE:     GPL-2.0 (https://spdx.org/licenses/GPL-2.0+)
> PURPOSE:     CDFS Recognizer
> COPYRIGHT:   Copyright 2017 Colin Finck [e-mail]
> */
>
> +// Information from http://wiki.osdev.org/ISO_9660#Volume_Descriptors
>
> +// Structures
> +typedef struct _VD_HEADER
> +{
> char Type;
> char Identifier[5];
> char Version;
>
> +}
> +VD_HEADER, *PVD_HEADER;
>
> +// Constants
> +#define VD_HEADER_OFFSET        32768       // Offset of the VD Header
> +#define VD_IDENTIFIER           "CD001"     // Identifier that must be in the Volume Descriptor
> +#define VD_IDENTIFIER_LENGTH    5           // Character count of VD_IDENTIFIER
> +#define VD_TYPE_PRIMARY         1           // Type code for Primary Volume Descriptor
> +#define VD_VERSION              1           // Volume Descriptor Version
> Propchange: trunk/reactos/drivers/filesystems/fs_rec/cdfs.h
> svn:eol-style = native
>
> Modified: trunk/reactos/drivers/filesystems/fs_rec/fs_rec.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fs_rec/...
> ==============================================================================
> --- trunk/reactos/drivers/filesystems/fs_rec/fs_rec.c	[iso-8859-1] (original)
> +++ trunk/reactos/drivers/filesystems/fs_rec/fs_rec.c	[iso-8859-1] Sun Aug 20 15:30:59 2017
> @@ -316,6 +316,7 @@
>  {
>      ULONG DeviceCount = 0;
>      NTSTATUS Status;
> PDEVICE_OBJECT CdfsObject;
> PDEVICE_OBJECT UdfsObject;
> PAGED_CODE();
>
> @@ -340,14 +341,24 @@
>      DriverObject->MajorFunction[IRP_MJ_FILE_SYSTEM_CONTROL] = FsRecFsControl;
>      DriverObject->DriverUnload = FsRecUnload;
> /* Register CDFS */
> Status = FsRecRegisterFs(DriverObject,
>                         NULL,
>                         NULL,
>
> /* Register CDFS for CDs */
>
> Status = FsRecRegisterFs(DriverObject,
>                         NULL,
>                         &CdfsObject,
>                         L"\\Cdfs",
>                         L"\\FileSystem\\CdfsRecognizer",
>                         FS_TYPE_CDFS,
>                         FILE_DEVICE_CD_ROM_FILE_SYSTEM);
>
> if (NT_SUCCESS(Status)) DeviceCount++;
>
> /* Register CDFS for HDDs */
>
> Status = FsRecRegisterFs(DriverObject,
>                         CdfsObject,
>                         NULL,
>                         L"\\CdfsHdd",
>                         L"\\FileSystem\\CdfsHddRecognizer",
>                         FS_TYPE_CDFS,
>                         FILE_DEVICE_DISK_FILE_SYSTEM);
>
> if (NT_SUCCESS(Status)) DeviceCount++;
>
> /* Register UDFS for CDs */
>
> Modified: trunk/reactos/drivers/filesystems/fs_rec/udfs.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fs_rec/...
> ==============================================================================
> --- trunk/reactos/drivers/filesystems/fs_rec/udfs.c	[iso-8859-1] (original)
> +++ trunk/reactos/drivers/filesystems/fs_rec/udfs.c	[iso-8859-1] Sun Aug 20 15:30:59 2017
> @@ -125,7 +125,7 @@
>              MountDevice = Stack->Parameters.MountVolume.DeviceObject;
>              if (FsRecGetDeviceSectorSize(MountDevice, &SectorSize))
>              {
>            /* Check if it's an actual FAT volume */
>            /* Check if it's an actual UDF volume */
>            if (FsRecIsUdfsVolume(MountDevice, SectorSize))
>            {
>                /* It is! */
-- 
Pierre Schweitzer <pierre at reactos.org>
System & Network Administrator
Senior Kernel Developer
ReactOS Deutschland e.V.

[Colin Finck]

Hey Pierre,

Am 21.08.2017 um 21:32 schrieb Pierre Schweitzer:
> This commit is likely breaking UDFS recognition
> when media has a protective CDFS partition with UDFS afterwards, while
> both FSD are unloaded.
> You'll first match the CDFS recognizer and not the UDFS recognizer.

Not sure how my commit is breaking exactly that as I first register CDFS 
and then UDF, just like before. With these two subsequent InsertHeadList 
calls, UDF always comes before CDFS in the queue when both drivers are 
unloaded.

Anyway, thanks for your tip about registering as a low-priority 
filesystem! I've done the necessary changes in r75638. This now ensures 
that CDFS is always inserted last and UDF takes precedence, no matter 
which filesystem drivers have already been loaded.
And we're consistent with how CDFS registers itself.

Cheers,

Colin