Inconsistency in calls to SMB_VFS_STAT ...

[Richard Sharpe]

On Tue, Apr 14, 2015 at 9:58 AM, Jeremy Allison <jra at samba.org> wrote:
> On Tue, Apr 14, 2015 at 09:52:20AM -0700, Richard Sharpe wrote:
>> Hi folks,
>>
>> When helping someone who is doing a VFS module recently, I noticed an
>> inconsistency in the what that SMB_VFS_STAT is called:
>>
>> 1. When a share is first connected to, Samba calls SMB_VFS_STAT on the
>> absolute path that the share is declared to be at.
>>
>> 2. On all other cases (I believe) it is called with a pathname that
>> relative to the root of the share.
>>
>> I can document this in the VFS documentation, but it would be nice,
>> perhaps to have a flag passed to stat that tells the module writer if
>> the current call is for Samba's internal purposes (absolute path name)
>> or for a user request (relative path name ).
>>
>> Does anyone have any strong feelings about this either way?
>
> That's a bug. We should chdir() to the share root, then
> call SMB_VFS_STAT on the relative path.
>
> We should *never* be calling SMB_VFS_STAT on an absolute
> path.

OK, the offending code appears to be in source3/smbd/service.c around
line 824, as shown below. Notice the comment about Win2000.

        smb_fname_cpath = synthetic_smb_fname(talloc_tos(), conn->connectpath,
                                              NULL, NULL);
        if (smb_fname_cpath == NULL) {
                status = NT_STATUS_NO_MEMORY;
                goto err_root_exit;
        }

        /* win2000 does not check the permissions on the directory
           during the tree connect, instead relying on permission
           check during individual operations. To match this behaviour
           I have disabled this chdir check (tridge) */
        /* the alternative is just to check the directory exists */

        if ((ret = SMB_VFS_STAT(conn, smb_fname_cpath)) != 0 ||
            !S_ISDIR(smb_fname_cpath->st.st_ex_mode)) {
                if (ret == 0 && !S_ISDIR(smb_fname_cpath->st.st_ex_mode)) {
                        DEBUG(0,("'%s' is not a directory, when connecting to "
                                 "[%s]\n", conn->connectpath,
                                 lp_servicename(talloc_tos(), snum)));
                } else {
                        DEBUG(0,("'%s' does not exist or permission denied "
                                 "when connecting to [%s] Error was %s\n",
                                 conn->connectpath,
                                 lp_servicename(talloc_tos(), snum),
                                 strerror(errno) ));
                }
                status = NT_STATUS_BAD_NETWORK_NAME;
                goto err_root_exit;
        }
        conn->base_share_dev = smb_fname_cpath->st.st_ex_dev;

Should I file a bug?

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)