Fix bug found by adding regression test for bug 14892.

I realized I could exploit a Samba difference from Windows to test the rmdir_internal() race condition of bug 14892 so I wrote a test just to ensure we don't regress. The test does:

1). Create "DEL_ON_CLOSE_DIR" - returns handle fnum
2). Set DELETE-ON-CLOSE bit on handle fnum
3). Create file "DEL_ON_CLOSE_DIR\\testfile" (this would fail on Windows with DELETE_PENDING, but works against stock smbd).
4). Close fnum - we should get NT_STATUS_DIRECTORY_NOT_EMPTY.

With bbdcd66c reverted (dirfsp is being used uninitialized inside rmdir_internals()) this crashes as you'd expect (runs into the uninitialized dirfsp). But with bbdcd66c applied the test still failed by returning NT_STATUS_OK in step (4) above.

Turns out the STAT/LSTAT calls inside the "scan directory for non-veto files"/"delete veto file loops" are overwriting the 'int ret' variable used to return error in rmdir_internal().

This patch (including the test, naturally) ensures we don't overwrite 'ret' in the directory scanning loops (use 'int retval' for the STAT/LSTAT calls, which is already being used in one existing place but this subtlety was missed when the STAT/LSTAT calls were cleaned up to use 'ret = STAT()/LSTAT()' as a helper variable (older versions of this code just did 'if (LSTAT(..)!=0' so avoided this problem).

I'm intending to clean up rmdir_internal() to use NT_STATUS internally instead of relying on ret=-1,errno for errors, but that's a later patch I'll need to look at more carefully.

Passes ci.

Edited by Jeremy Allison

Merge request reports

Loading