Bug 11216 - Samba calls SMB_VFS_STAT with an absolute path during TreeConnect processing but a relative path everywhere else
Samba calls SMB_VFS_STAT with an absolute path during TreeConnect processing ...
Status: ASSIGNED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services
unspecified
All All
: P5 normal
: ---
Assigned To: Jeremy Allison
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-14 17:24 UTC by Richard Sharpe
Modified: 2015-04-15 06:43 UTC (History)
3 users (show)

See Also:


Attachments
git-am fix for master. (3.58 KB, patch)
2015-04-14 19:14 UTC, Jeremy Allison
no flags Details
git-am fix for master. (5.92 KB, patch)
2015-04-14 19:46 UTC, Jeremy Allison
jra: review?
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Sharpe 2015-04-14 17:24:42 UTC
One of my team noticed that Samba calls SMB_VFS_STAT with absolute paths sometimes.

I tracked it down to the code in source3/smbd/service.c around line 824. There is a comment there:

        /* 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 */

It should always use the relative path or give us a flag to let us know which is being used.
Comment 1 Richard Sharpe 2015-04-14 19:01:09 UTC
Note that there are two actual calls to SMB_VFS_STAT with an absolute path. One of them is coming from source3/modules/vfs_default.c:vfswrap_fs_capabilities.

Perhaps we should restructure that call to pass in the statbuf we got from the earlier call ... or something ...
Comment 2 Jeremy Allison 2015-04-14 19:14:00 UTC
Created attachment 10954 [details]
git-am fix for master.

Richard, can you test this and see if it fixes the issue ? I'm still doing testing here.

Thanks !

Jeremy.
Comment 3 Richard Sharpe 2015-04-14 19:22:52 UTC
OK, giving it a spin now ...
Comment 4 Jeremy Allison 2015-04-14 19:24:35 UTC
Ah - won't fix the capabilities use. Maybe a follow-up patch for that ?
Comment 5 Jeremy Allison 2015-04-14 19:46:22 UTC
Created attachment 10955 [details]
git-am fix for master.

This should fix both places...
Comment 6 Richard Sharpe 2015-04-14 22:22:32 UTC
OK, I have tested the first patch and it works to fix the first usage.

However, I noticed some interesting things.

Here is a log of all the stat calls made when I use smbclient and do an ls and then exit:

[root@rjsnvm1 ~]# grep ' called with' /var/log/samba/ntnxclus1.log
  hvnas_stat called with .                    # The patch fixes this one
  hvnas_stat called with /home/shares/share1  # This one is expected
  hvnas_stat called with .
  hvnas_stat called with .
  hvnas_stat called with .
  hvnas_stat called with .
  hvnas_stat called with .
  hvnas_stat called with .
  hvnas_stat called with ..                   # What is this?
  hvnas_stat called with ./testdir2
  hvnas_stat called with ./testdir6
  hvnas_stat called with ./testdir13
  hvnas_stat called with ./testdir8
  hvnas_stat called with ./testdir4
  hvnas_stat called with ./testdir11
  hvnas_stat called with ./somefunnydir
  hvnas_stat called with .
  hvnas_stat called with .
  hvnas_stat called with .
  hvnas_stat called with .
  hvnas_stat called with /                    # Whoa, and this?

Excuse my little joke, BTW.
Comment 7 Richard Sharpe 2015-04-14 22:41:40 UTC
Hmmm, with the second patch I get this:

[2015/04/14 15:41:04.750357, 10, pid=4212, effective(0, 0), real(0, 0)] ../source3/smbd/service.c:164(set_conn_connectpath)
  set_conn_connectpath: service share1, connectpath = /home/shares/share1
[2015/04/14 15:41:04.750380, 10, pid=4212, effective(0, 0), real(0, 0)] ../source3/modules/vfs_vnas.c:97(vnas_stat)
  my_test_vfs called with .
[2015/04/14 15:41:04.750396, 10, pid=4212, effective(0, 0), real(0, 0)] ../source3/modules/vfs_vnas.c:97(vnas_stat)
  my_test_vfs called with .
[2015/04/14 15:41:04.750417,  4, pid=4212, effective(0, 0), real(0, 0), class=vfs] ../source3/smbd/vfs.c:844(vfs_ChDir)
  vfs_ChDir to /home/shares/share1
[2015/04/14 15:41:04.750432, 10, pid=4212, effective(0, 0), real(0, 0)] ../source3/modules/vfs_vnas.c:97(vnas_stat)
  my_test_vfs called with .
[2015/04/14 15:41:04.750446, 10, pid=4212, effective(0, 0), real(0, 0)] ../source3/modules/vfs_vnas.c:97(vnas_stat)
  my_test_vfs called with /home/shares/share1
[2015/04/14 15:41:04.781368,  0, pid=4212, effective(0, 0), real(0, 0)] ../source3/lib/util.c:901(log_stack_trace)
  BACKTRACE: 30 stack frames:
   #0 /usr/lib/libsmbconf.so.0(log_stack_trace+0x1f) [0x7faf22ded3a4]
   #1 /usr/lib64/samba/vfs/vnas.so(+0xd12) [0x7faf0df35d12]
   #2 /usr/lib/samba/libsmbd-base-samba4.so(smb_vfs_call_stat+0x50) [0x7faf248ec482]
   #3 /usr/lib/samba/libsmbd-base-samba4.so(vfs_GetWd+0x2ad) [0x7faf248e9bbf]
   #4 /usr/lib/samba/libsmbd-base-samba4.so(vfs_ChDir+0x190) [0x7faf248e98a8]
   #5 /usr/lib/samba/libsmbd-base-samba4.so(+0x2828b4) [0x7faf24a148b4]
   #6 /usr/lib/samba/libsmbd-base-samba4.so(smb_vfs_call_fs_capabilities+0x4a) [0x7faf248eb3f8]
   #7 /usr/lib/samba/libsmbd-base-samba4.so(+0x17a2b6) [0x7faf2490c2b6]
   #8 /usr/lib/samba/libsmbd-base-samba4.so(+0x17a6d1) [0x7faf2490c6d1]
   #9 /usr/lib/samba/libsmbd-base-samba4.so(make_connection+0x7d1) [0x7faf2490d143]
   #10 /usr/lib/samba/libsmbd-base-samba4.so(reply_tcon_and_X+0x8bf) [0x7faf2488ffe9]

I added the stack trace to see where we were getting that absolute path from.
Comment 8 Richard Sharpe 2015-04-14 22:47:13 UTC
I'm also seeing this:

  vfs_ChDir to /
[2015/04/14 15:41:08.660703, 10, pid=4212, effective(0, 0), real(0, 0)] ../source3/modules/vfs_vnas.c:97(vnas_stat)
  my_test_vfs called with .
[2015/04/14 15:41:08.660722, 10, pid=4212, effective(0, 0), real(0, 0)] ../source3/modules/vfs_vnas.c:97(vnas_stat)
  my_test_vfs called with /
[2015/04/14 15:41:08.661219,  0, pid=4212, effective(0, 0), real(0, 0)] ../source3/lib/util.c:901(log_stack_trace)
  BACKTRACE: 27 stack frames:
   #0 /usr/lib/libsmbconf.so.0(log_stack_trace+0x1f) [0x7faf22ded3a4]
   #1 /usr/lib64/samba/vfs/vnas.so(+0xd12) [0x7faf0df35d12]
   #2 /usr/lib/samba/libsmbd-base-samba4.so(smb_vfs_call_stat+0x50) [0x7faf248ec482]
   #3 /usr/lib/samba/libsmbd-base-samba4.so(vfs_GetWd+0x2ad) [0x7faf248e9bbf]
   #4 /usr/lib/samba/libsmbd-base-samba4.so(vfs_ChDir+0x190) [0x7faf248e98a8]
   #5 /usr/lib/samba/libsmbd-base-samba4.so(close_cnum+0x153) [0x7faf2490d2b6]
   #6 /usr/lib/samba/libsmbd-base-samba4.so(smbXsrv_tcon_disconnect+0x5d7) [0x7faf24952b8c]
   #7 /usr/lib/samba/libsmbd-base-samba4.so(reply_tdis+0xc3) [0x7faf2489e2c1]
   #8 /usr/lib/samba/libsmbd-base-samba4.so(+0x170c9f) [0x7faf24902c9f]
   #9 /usr/lib/samba/libsmbd-base-samba4.so(+0x170e6f) [0x7faf24902e6f]

and this bizarre one:

(128975951, 0), class=vfs] ../source3/smbd/vfs.c:1203(check_reduced_name)
  check_reduced_name realpath [.] -> [/]
[2015/04/14 15:41:06.841583,  2, pid=4212, effective(128975951, 128975361), real(128975951, 0), class=vfs] ../source3/smbd/vfs.c:1233(check_reduced_name)
  check_reduced_name: Bad access attempt: . is a symlink outside the share path
  conn_rootdir =/home/shares/share1
  resolved_name=/
[2015/04/14 15:41:06.841620,  5, pid=4212, effective(128975951, 128975361), real(128975951, 0)] ../source3/smbd/filename.c:1050(check_name)
  check_name: name . failed with NT_STATUS_ACCESS_DENIED
[2015/04/14 15:41:06.841645,  3, pid=4212, effective(128975951, 128975361), real(128975951, 0)] ../source3/smbd/filename.c:1404(filename_convert_internal)
  filename_convert_internal: check_name failed for name . with NT_STATUS_ACCESS_DENIED
[2015/04/14 15:41:06.841676,  3, pid=4212, effective(128975951, 128975361), real(128975951, 0)] ../source3/smbd/error.c:82(error_packet_set)
  NT error packet at ../source3/smbd/trans2.c(5536) cmd=50 (SMBtrans2) NT_STATUS_ACCESS_DENIED

Digging further.
Comment 9 Richard Sharpe 2015-04-14 22:54:05 UTC
OK, I know what is going on with the claim that . is a symlink.

It looks like we did a ChDir back to / and now . means something different.
Comment 10 Jeremy Allison 2015-04-14 22:59:34 UTC
(In reply to Richard Sharpe from comment #9)

Hmmm. Is that after the make_connection() call ? That should be being done as root, and I didn't think we should be doing any more file access in the reply from the tcon call.
Comment 11 Richard Sharpe 2015-04-14 23:32:08 UTC
I see what the problem is.

The share is an msdfs root:

[2015/04/14 16:32:06.446677,  5, pid=4449, effective(0, 0), real(0, 0)] ../source3/auth/token_util.c:639(debug_unix_user_token)
  UNIX token of user 0
  Primary group is 0 and contains 0 supplementary groups
[2015/04/14 16:32:06.446696,  5, pid=4449, effective(0, 0), real(0, 0)] ../source3/smbd/uid.c:425(smbd_change_to_root_user)
  change_to_root_user: now uid=(0,0) gid=(0,0)
[2015/04/14 16:32:06.446708,  2, pid=4449, effective(0, 0), real(0, 0)] ../source3/smbd/service.c:1152(close_cnum)
  ntnxclus1 (ipv4:127.0.0.1:37352) closed connection to service share1
[2015/04/14 16:32:06.446732,  4, pid=4449, effective(0, 0), real(0, 0), class=vfs] ../source3/smbd/vfs.c:844(vfs_ChDir)
  vfs_ChDir to /

We are doing an explicit chdir to / after we disconnect for doing the MSDFS stuff.
Comment 12 Richard Sharpe 2015-04-14 23:48:49 UTC
(In reply to Richard Sharpe from comment #11)

OK, ignore that comment. We were doing a ChDir to / because the client disconnected from the share.
Comment 13 Richard Sharpe 2015-04-14 23:57:13 UTC
This appears to be the problem:

[2015/04/14 16:32:03.976431,  4, pid=4449, effective(0, 0), real(0, 0), class=vfs] ../source3/smbd/vfs.c:855(vfs_ChDir)
  vfs_ChDir got /home/shares/share1
[2015/04/14 16:32:03.976454, 10, pid=4449, effective(0, 0), real(0, 0)] ../source3/modules/vfs_vnas.c:97(vnas_stat)
  my_test_vfs called with .
[2015/04/14 16:32:03.976472, 10, pid=4449, effective(0, 0), real(0, 0), class=vfs] ../source3/modules/vfs_default.c:176(vfswrap_fs_capabilities)
  vfswrap_fs_capabilities: timestamp resolution of sec available on share share1, directory /home/shares/share1
[2015/04/14 16:32:03.976488,  2, pid=4449, effective(0, 0), real(0, 0)] ../source3/smbd/service.c:865(make_connection_snum)
  ntnxclus1 (ipv4:127.0.0.1:37352) connect to service share1 initially as user VNASDOM1\test1 (uid=128975951, gid=128975361) (pid 4449)
[2015/04/14 16:32:03.976508, 10, pid=4449, effective(0, 0), real(0, 0)] ../source3/smbd/service.c:879(make_connection_snum)
  Changing to /

vfs_ChDir was called to set us to the share path, and behind it's back we did a chdir to /.
Comment 14 Richard Sharpe 2015-04-15 00:10:24 UTC
This appears to be what we need instead of chdir in those two places:

+       /* We're exiting as root - ensure we're in / */
+       DEBUG(10, ("Changing to /"));
+       if (vfs_ChDir(conn, "/") != 0) {
+               smb_panic("Failed to ChDir(/).");
+       }
+
        return status;

   err_root_exit:
@@ -881,6 +894,11 @@ static NTSTATUS make_connection_snum(struct smbXsrv_connect
                /* Call VFS disconnect hook */
                SMB_VFS_DISCONNECT(conn);
        }
+       /* We're exiting as root - ensure we're in / */
+       DEBUG(10, ("Changing to /"));
+       if (vfs_ChDir(conn, "/") != 0) {
+               smb_panic("Failed to ChDir(/).");
+       }
        return status;
Comment 15 Jeremy Allison 2015-04-15 00:13:31 UTC
Oh I get it, I forgot to use vfs_Chdir() to keep the cache consistent once we've already used it. Doh ! Thanks...
Comment 16 Richard Sharpe 2015-04-15 00:19:49 UTC
One more issue, we call vfs_GetWd and that calls SMB_VFS_STAT on smb_fname_full, so we still get absolute path names in vfs module stat functions.