MDEV-39533 my_realpath and MY_NOSYMLINKS on Windows by vaintroub · Pull Request #5682 · MariaDB/server

vaintroub · 2026-09-15T16:56:55Z

Summary

On Windows, my_realpath() only called GetFullPathName(), which
canonicalizes ., .. and drive letters but does not resolve NTFS
symlinks, junctions or mount points (unlike POSIX realpath()). At
the same time, my_open() and my_delete() ignored MY_NOSYMLINKS
entirely, so the symlink-attack protection used for MyISAM/Aria's
DATA DIRECTORY/INDEX DIRECTORY (mi_open()/ma_open(),
my_handler_delete_with_symlink()) was silently absent on Windows.

Fix my_realpath() to actually resolve reparse points, using
CreateFile() + GetFinalPathNameByHandle(). A missing path now
correctly returns 1/ENOENT on Windows too (previously always
returned 0), matching POSIX realpath().
Make my_open()/my_delete() honor MY_NOSYMLINKS on Windows by
comparing the caller-supplied (expected already-resolved) name
against the actually-opened handle's resolved path, mirroring the
realpath()-equality branch of the POSIX NOSYMLINK_FUNCTION_BODY
macro (same ENOTDIR errno for a mismatch).
GetFinalPathNameByHandle(FILE_NAME_NORMALIZED) can fail on some
SMB shares; both my_realpath() and MY_NOSYMLINKS verification
fall back to the weaker FILE_NAME_OPENED query in that case, and
log a one-time warning naming the affected path.
MyISAM/Aria's data-file open only requested MY_NOSYMLINKS when
O_NOFOLLOW was set, and O_NOFOLLOW was plain 0 on Windows, so
the data file was never actually protected (only the index file
was). Give O_NOFOLLOW a real, reserved bit on Windows so that
gating works as intended.
Rewrite my_win_unlink() (moved from my_delete.c to
my_winfile.cc): verify no symlinks before any delete disposition
is set (closes a window where a rejected delete could still commit
via delete-on-close), rename through the handle rather than by
path, and report real posix-delete errors (e.g. a read-only file)
instead of masking them with a fallback attempt.

Test plan

unittest/mysys/my_symlink-t (17/17): junction resolution and
MY_NOSYMLINKS enforcement for my_open()/my_delete().
unittest/mysys/my_delete-t (31/31): my_win_unlink()'s own
fallback paths (posix-semantics failure, rename failure,
read-only files, orphan-rename-back).
Full main mtr suite (1195/1195) and the full buildbot-matching
suite set (vcol,gcol,period,perfschema,parts,main,innodb, versioning,plugins,mariabackup,roles,auth_gssapi,mysql_sha2, rocksdb,sysschema, 2641/2655 -- the 14 failures are unrelated
innochecksum-tool and audit/error-log-plugin tests, verified
against an unmodified baseline build).

🤖 Generated with Claude Code

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot review overview

🟡 Changes recommended

Critical MY_NOSYMLINKS bypasses and unsafe post-open validation remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Reject final symlinks before DeleteFile for MY_NOSYMLINKS · New
Enforce MY_NOSYMLINKS flags at runtime · New

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved critical deletion-safety issues and Windows path-resolution contract findings remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Cancel pending deletion when rejecting symlink or junction · New
Prevent path-based fallback rename from bypassing handle verification · New

Resolved since last review (2)

Enforce MY_NOSYMLINKS flags at runtime
Reject final symlinks before DeleteFile for MY_NOSYMLINKS

+      if (win_unlink_check_nosymlinks(handle, name, MyFlags))
+      {
+        CloseHandle(handle);
+        DBUG_RETURN(-1); /* errno set by my_win_verify_nosymlinks() */


+    if (win_unlink_check_nosymlinks(handle, name, MyFlags))
+    {
+      CloseHandle(handle);
+      DBUG_RETURN(-1); /* errno set by my_win_verify_nosymlinks() */
+    }


Copilot

Copilot review overview

🟡 Changes recommended

The fallback can undermine symlink protection, and debug Windows builds have a compilation error.

Review effort: Lite
Findings: 3 · 1

Open (4)

Fallback path can bypass nosymlink checks using unresolved opened names · New
Prevent path-based fallback rename from bypassing handle verification
Cancel pending deletion when rejecting symlink or junction
Debug build fails due to goto crossing disp_ex initialization · New

+    /* NORMALIZED fails outright on some SMB servers; OPENED still names
+       the same resolved file (just a possibly different spelling), so
+       it's a safe fallback even for MY_NOSYMLINKS verification. */
+    len= GetFinalPathNameByHandle(handle, buf, (DWORD) sizeof(buf),
+                                   FILE_NAME_OPENED | VOLUME_NAME_DOS);


+  DBUG_EXECUTE_IF("force_posix_delete_fail", goto nonposix_delete;);
+  FILE_DISPOSITION_INFO_EX disp_ex;


Copilot

Copilot review overview

🟡 Changes recommended

Critical and moderate issues remain in path verification, deletion behavior, build compatibility, and test cleanup.

Review effort: Lite
Findings: 4 · 1 · 1

Open (6)

Fail closed when realpath resolution falls back unresolved · New
Fallback path can bypass nosymlink checks using unresolved opened names
Prevent path-based fallback rename from bypassing handle verification
Cancel pending deletion when rejecting symlink or junction
Debug build fails due to goto crossing disp_ex initialization
Wrap documentation lines exceeding 80 characters · New

+    /* NORMALIZED fails outright on some SMB servers; fall back to OPENED
+       so such shares remain usable, though it may not fully resolve
+       reparse points, weakening MY_NOSYMLINKS verification. */
+    warn_final_path_fallback_once(name);
+    len= GetFinalPathNameByHandle(handle, buf, (DWORD) sizeof(buf),


+  falls back to renaming to a unique name, then marking for classic delete-on-close.
+  The fallback is needed for FAT32/ExFAT, which do not have "POSIX-semantics" delete.
+  Renaming to unique name is needed, otherwise recreating file with the same name might
+  fail if another handle on file is still open.


Copilot

Copilot review overview

🟡 Changes recommended

The fallback path can fail open and bypass MY_NOSYMLINKS protection.

Review effort: Balanced
Findings: 4 · 1 · 2

Open (7)

Fail closed when realpath resolution falls back unresolved
Fallback path can bypass nosymlink checks using unresolved opened names
Prevent path-based fallback rename from bypassing handle verification
Cancel pending deletion when rejecting symlink or junction
Debug build fails due to goto crossing disp_ex initialization
Line exceeds the 80-character limit · New
Wrap documentation lines exceeding 80 characters

+  */
+  DBUG_EXECUTE_IF("force_posix_delete_fail", goto nonposix_delete;);
+  FILE_DISPOSITION_INFO_EX disp_ex;
+  disp_ex.Flags= FILE_DISPOSITION_FLAG_DELETE|FILE_DISPOSITION_FLAG_POSIX_SEMANTICS;


On Windows, my_realpath() only called GetFullPathName(), which doesn't resolve symlinks, junctions or mount points, unlike POSIX realpath(). At the same time, my_open() and my_delete() ignored MY_NOSYMLINKS entirely, so Windows silently lacked the symlink-attack protection MyISAM/Aria rely on for DATA DIRECTORY/INDEX DIRECTORY. Fix my_realpath() to resolve reparse points via CreateFile() + GetFinalPathNameByHandle(). A missing path now correctly returns 1/ENOENT on Windows too, matching POSIX. Make my_open()/my_delete() honor MY_NOSYMLINKS by comparing the caller-supplied (already-resolved) name against the opened handle's resolved path, mirroring the realpath()-equality branch of POSIX's NOSYMLINK_FUNCTION_BODY macro, rejecting a mismatch with ENOTDIR. Known limitation: GetFinalPathNameByHandle(FILE_NAME_NORMALIZED) can fail on some SMB shares; both my_realpath() and MY_NOSYMLINKS then fall back to the weaker FILE_NAME_OPENED query, logging a one-time warning naming the affected path. O_NOFOLLOW was plain 0 on Windows, so MyISAM/Aria's data-file open never actually got MY_NOSYMLINKS (only the index file did); it's now a real, reserved bit so that gating works as intended. my_win_unlink() (moved from my_delete.c to my_winfile.cc) now verifies no symlinks before any delete disposition is set, renames through the handle rather than by path, and reports real posix-delete errors (e.g. a read-only file) instead of masking them with a fallback attempt. Add unittest/mysys/my_symlink-t.c and extend my_delete-t.c with my_win_unlink()'s own fallback paths. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

vaintroub force-pushed the 11.4-MDEV-39533 branch from 27036f7 to ef5f555 Compare September 15, 2026 17:18

vaintroub requested a lite review from Copilot September 15, 2026 17:36

Copilot AI reviewed Sep 15, 2026

vaintroub force-pushed the 11.4-MDEV-39533 branch from ef5f555 to c8e9423 Compare September 15, 2026 18:28

gkodinov added the MariaDB Corporation label Sep 16, 2026

vaintroub requested a lite review from Copilot September 25, 2026 08:55

Copilot started reviewing on behalf of vaintroub September 25, 2026 08:55 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread mysys/my_delete.c Outdated

Comment thread mysys/my_winfile.cc Outdated

vaintroub force-pushed the 11.4-MDEV-39533 branch from c8e9423 to 66f2da1 Compare September 25, 2026 10:46

vaintroub requested a lite review from Copilot September 25, 2026 11:03

Copilot started reviewing on behalf of vaintroub September 25, 2026 11:03 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

vaintroub force-pushed the 11.4-MDEV-39533 branch 2 times, most recently from 5a514a3 to 49f9c91 Compare October 2, 2026 23:17

vaintroub requested a lite review from Copilot October 2, 2026 23:39

Copilot started reviewing on behalf of vaintroub October 2, 2026 23:40 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

vaintroub force-pushed the 11.4-MDEV-39533 branch from 49f9c91 to 5d39563 Compare October 3, 2026 15:58

vaintroub requested a lite review from Copilot October 3, 2026 15:58

Copilot started reviewing on behalf of vaintroub October 3, 2026 15:59 View session

Copilot AI reviewed Oct 3, 2026

View reviewed changes

vaintroub force-pushed the 11.4-MDEV-39533 branch from 5d39563 to b7b23c1 Compare October 4, 2026 13:13

vaintroub requested a balanced review from Copilot October 4, 2026 13:14

Copilot started reviewing on behalf of vaintroub October 4, 2026 13:15 View session

Copilot AI reviewed Oct 4, 2026

View reviewed changes

Comment thread mysys/my_winfile.cc

*/

DBUG_EXECUTE_IF("force_posix_delete_fail", goto nonposix_delete;);

FILE_DISPOSITION_INFO_EX disp_ex;

disp_ex.Flags= FILE_DISPOSITION_FLAG_DELETE|FILE_DISPOSITION_FLAG_POSIX_SEMANTICS;

vaintroub force-pushed the 11.4-MDEV-39533 branch 2 times, most recently from 675880d to 1aceefa Compare October 9, 2026 23:16

vaintroub force-pushed the 11.4-MDEV-39533 branch from 1aceefa to 7cc18d0 Compare October 12, 2026 11:16

vaintroub requested a review from vuvova October 12, 2026 15:56

Navigation Menu

Repository navigation

MDEV-39533 my_realpath and MY_NOSYMLINKS on Windows - #5682

MDEV-39533 my_realpath and MY_NOSYMLINKS on Windows#5682
vaintroub wants to merge 1 commit into
11.4from
11.4-MDEV-39533

vaintroub commented Sep 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

		DBUG_EXECUTE_IF("force_posix_delete_fail", goto nonposix_delete;);
		FILE_DISPOSITION_INFO_EX disp_ex;

Conversation

vaintroub commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

vaintroub commented Sep 15, 2026 •

edited

Loading