[PATCH 4/5] mountmgr.sys: Add the ability to query filesystem attributes on a file handle.

[Erich E. Hoover]

This patch adds capability to the new IOCTL_MOUNTMGR_QUERY_VOLUME
ioctl to be able to return volume information on a file handle.  This
code represents the "fallback" case when standard volume information
is unavailable (for more details see the code removed from
dlls/ntdll/unix/file.c in patch 5 in the case where
get_mountmgr_fs_info() fails).

Best,
Erich

[Zebediah Figura (she/her)]

On 3/18/21 11:54 AM, Erich E. Hoover wrote:
> @@ -2019,6 +2057,30 @@ NTSTATUS query_volume( void *buff, SIZE_T insize, SIZE_T outsize, IO_STATUS_BLOC
>      }
>      LeaveCriticalSection( &device_section );
>
> +    if (!volume && input->info_class == FileFsAttributeInformation)
> +    {
> +        enum fs_type fs_type = FS_UNKNOWN;
> +        HANDLE hProcess, handle;
> +        BOOL ret;
> +        int fd;
> +
> +        if (!(hProcess = OpenProcess( PROCESS_DUP_HANDLE, FALSE, input->process )))
> +            return status;

This is right, but it looks wrong. I'd recommend returning early in the 
if (volume) block, and explicitly returning STATUS_NO_SUCH_DEVICE 
instead of initializing it at the top.
> +        ret = DuplicateHandle( hProcess, input->handle, GetCurrentProcess(), &handle, 0, FALSE,
> +                               DUPLICATE_SAME_ACCESS );
> +        CloseHandle( hProcess );
> +        if (!ret) return status;
> +        status = wine_server_handle_to_fd( handle, 0, &fd, NULL );
> +        if (!status)
> +        {
> +            fs_type = get_fd_fs_type( fd );
> +            wine_server_release_fd( handle, fd );
> +        }
> +        CloseHandle( handle );
> +        status = fill_volume_info( info_class, fs_type, 0, NULL, buff, outsize,
> +                                   &iosb->Information );
> +    }
> +
>      return status;
>  }

[Alexandre Julliard]

"Zebediah Figura (she/her)" <zfigura(a)codeweavers.com> writes:
> On 3/18/21 11:54 AM, Erich E. Hoover wrote:
> > @@ -2019,6 +2057,30 @@ NTSTATUS query_volume( void *buff, SIZE_T insize, SIZE_T outsize, IO_STATUS_BLOC
> >      }
> >      LeaveCriticalSection( &device_section );
> >  +    if (!volume && input->info_class ==
> > FileFsAttributeInformation)
> > +    {
> > +        enum fs_type fs_type = FS_UNKNOWN;
> > +        HANDLE hProcess, handle;
> > +        BOOL ret;
> > +        int fd;
> > +
> > +        if (!(hProcess = OpenProcess( PROCESS_DUP_HANDLE, FALSE, input->process )))
> > +            return status;
>
> This is right, but it looks wrong. I'd recommend returning early in
> the if (volume) block, and explicitly returning STATUS_NO_SUCH_DEVICE 
> instead of initializing it at the top.

Actually I don't think mountmgr has any business resolving client
handles itself. There has to be a better way.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Erich E. Hoover]

On Wed, Mar 24, 2021 at 10:59 AM Alexandre Julliard <julliard(a)winehq.org> wrote:
> "Zebediah Figura (she/her)" <zfigura(a)codeweavers.com> writes:
> > On 3/18/21 11:54 AM, Erich E. Hoover wrote:
> > > @@ -2019,6 +2057,30 @@ NTSTATUS query_volume( void *buff, SIZE_T insize, SIZE_T outsize, IO_STATUS_BLOC
> > >      }
> > >      LeaveCriticalSection( &device_section );
> > >  +    if (!volume && input->info_class ==
> > > FileFsAttributeInformation)
> > > +    {
> > > +        enum fs_type fs_type = FS_UNKNOWN;
> > > +        HANDLE hProcess, handle;
> > > +        BOOL ret;
> > > +        int fd;
> > > +
> > > +        if (!(hProcess = OpenProcess( PROCESS_DUP_HANDLE, FALSE, input->process )))
> > > +            return status;
> >
> > This is right, but it looks wrong. I'd recommend returning early in
> > the if (volume) block, and explicitly returning STATUS_NO_SUCH_DEVICE
> > instead of initializing it at the top.
>
> Actually I don't think mountmgr has any business resolving client
> handles itself. There has to be a better way.

There are a couple other ways that I can see:
1) duplicate the handle on the calling end
2) call fstatfs() on the calling end and send the appropriate
"fallback" fs_type for the mountmgr to decode

Do either of these sound good to you?

Best,
Erich