lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <Y/7ZVZ5Wv0NnzT4g@sol.localdomain>
Date:   Tue, 28 Feb 2023 20:49:25 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Paulo Antonio Alvarez <pauloaalvarez@...il.com>
Cc:     linux-ext4@...r.kernel.org
Subject: What is the purpose of windows_io_manager in e2fsprogs?

Hi Paulo,

I'm very confused by all the new Windows-specific code that you added to
e2fsprogs in the following commit:

	commit d04f34a050e3334456e9ee80f95869dc47b0fd9f
	Author: Paulo Antonio Alvarez <pauloaalvarez@...il.com>
	Date:   Tue Dec 22 15:15:50 2020 -0300

	    libext2fs: add a Windows implementation of the IO manager

So, Windows now has a windows_io_manager, which is used instead of
unix_io_manager.  That *sounds* like something that makes sense.

But actually, AFAICT, Windows used unix_io_manager before, and it more or less
works fine.  The basic UNIX I/O functions like open(), read(), write(), etc.
are available in MSVCRT on Windows.  (Often with quirks, but they are
available.)  MinGW provides some compatibility definitions too, and Windows
builds of e2fsprogs are only supported through MinGW anyway.

So I'd say that unix_io_manager is misleadingly named and really is more like
"regular_io_manager"...

I'm having a hard time seeing what the windows_io_manager gives us for the price
of duplicating lots of code, such as the I/O cache, from unix_io.c.

The only *potential* benefits of windows_io_manager I can see are the following:

  * Its ext2fs_file_open() always uses O_BINARY.  This seems to be a bug fix.

    However, unix_io.c could trivially do this.

  * windows_io_manager considers a path with one leading backslash to be an NT
    namespace path, for example \Device\HarddiskVolume4.  It uses
    DefineDosDevice() to create an alias for it and access it.  Perhaps the
    intent here was to add support for Windows volumes.

    However, this doesn't actually make sense, and IMO it is a bug, not a
    feature.  Paths need to be accepted in a consistent format.  It should just
    use the normal path format that Windows programs use (Win32 namespace).
    Windows volumes can still be specified using paths like \\?\D: or
    \\?\GLOBALROOT\Device\HarddiskVolume4.  (BTW, mke2fs doesn't actually work
    on volumes regardless of any of this, since check_plausibility() fails.)

  * windows_io_manager uses 'FILE_FLAG_NO_BUFFERING | FILE_FLAG_WRITE_THROUGH',
    which is the equivalent of O_DIRECT.  However, it is broken since it does
    this regardless of IO_FLAG_DIRECT_IO, and it also doesn't honor the
    alignment restrictions.  Note: e2fsprogs doesn't use direct I/O by default
    anyway, so direct I/O support doesn't seem like an important feature here.

Would you or anyone else have any objection if I just removed windows_io.c and
made Windows use unix_io.c again, with the O_BINARY fix as that seems to have
been the only real issue with it?

- Eric

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ