[PATCH] Fix nasty vfs_fruit bug 13646

[Ralph Böhme]

On Mon, Oct 29, 2018 at 05:02:57PM -0700, Jeremy Allison wrote:
>On Mon, Oct 29, 2018 at 04:24:06PM -0700, Jeremy Allison via samba-technical wrote:
>> On Fri, Oct 26, 2018 at 04:59:24PM +0200, Ralph Böhme via samba-technical wrote:
>> >
>> > thanks!
>> >
>> > Turned out the order was not really correct as this patchset wasn't rebased
>> > onto the other ones.
>> >
>> > Updated, rebased patchset attached.
>>
>> Quick comment:
>>
>>
>> > From a04bcc4e0514b8f54af8dc936710f4a2096a0492 Mon Sep 17 00:00:00 2001
>> > From: Ralph Boehme <slow at samba.org>
>> > Date: Wed, 17 Oct 2018 10:54:10 +0200
>> > Subject: [PATCH 02/34] vfs_streams_xattr: consistently log dev/ino as hex
>> >
>> > open_file_ntcreate() logs dev/ino in hex format if it hits a "dev/ino
>> > mismatch". Logging it consistently as hex helps debugging such mismatch
>> > issues.
>>
>> The above doesn't seem true. According to master
>> I have:
>>
>>         if (file_existed && !check_same_dev_ino(&saved_stat, &smb_fname->st)) {
>>                 /*
>>                  * The file did exist, but some other (local or NFS)
>>                  * process either renamed/unlinked and re-created the
>>                  * file with different dev/ino after we walked the path,
>>                  * but before we did the open. We could retry the
>>                  * open but it's a rare enough case it's easier to
>>                  * just fail the open to prevent creating any problems
>>                  * in the open file db having the wrong dev/ino key.
>>                  */
>>                 fd_close(fsp);
>>                 DEBUG(1,("open_file_ntcreate: file %s - dev/ino mismatch. "
>>                         "Old (dev=0x%llu, ino =0x%llu). "
>>                         "New (dev=0x%llu, ino=0x%llu). Failing open "
>>                         " with NT_STATUS_ACCESS_DENIED.\n",
>>                          smb_fname_str_dbg(smb_fname),
>>                          (unsigned long long)saved_stat.st_ex_dev,
>>                          (unsigned long long)saved_stat.st_ex_ino,
>>                          (unsigned long long)smb_fname->st.st_ex_dev,
>>                          (unsigned long long)smb_fname->st.st_ex_ino));
>>                 return NT_STATUS_ACCESS_DENIED;
>>         }
>>
>>  - which is just wrong I think (sorry, this is my bad code I'm
>>  sure :-). Should be:
>>
>>   %llx, not 0x%llu (which is 0x followed by unsigned int!).
>>
>> So I'm going to NAK [PATCH 02/34] and continue reviewing
>> the rest, which (hopefully) doesn't depend on this.
>
>Oh - same issue with [PATCH 23/34] vfs_fruit: update and add some debugging of dev/ino
>
>Can we add in the dev/ino debugging fixes as a separate
>patchset once the other reviews are done ?

hm, I remember *something* was confusing, guess the wrong 0x prefix from open.c 
triggered this. :)

I've no just updated my changes to the VFS modules to use decimal format and 
added an initial fix for open.c to remove the misleading 0x prefix. That patch 
is without a bugnumber, it could go to master only.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46