samba - Dec 2020 - Strange logs: check_usershare_stat: file /var/lib/samba/usershares/ owned by uid 0 is not a regular file

On 01/12/2020 21:45, Jeremy Allison via samba wrote:
> On Tue, Dec 01, 2020 at 01:29:55PM -0800, Jeremy Allison via samba wrote:
>> On Tue, Dec 01, 2020 at 08:10:52PM -0000, Roy Eastwood via samba wrote:
>>> Thanks Andrew for your quick reply.
>>>
>>> Here are the results of those commands:
>>>
>>> root at lxd-m1:~# ls -l /var/lib/samba/usershares
>>> total 0
>>> root at lxd-m1:~# ls -l /var/lib/samba/
>>> total 136
>>> -rw------- 1 root root????????? 421888 Jun 16 19:18
account_policy.tdb
>>> drwxr-xr-x 1 root root????????????? 36 Jul 19? 2019 DriverStore
>>> -rw------- 1 root root????????? 425984 Jul 19? 2019
group_mapping.tdb
>>> drwxr-xr-x 1 root root????????????? 98 Jul 19? 2019 printers
>>> drwxr-xr-x 1 root root???????????? 124 Jul 19? 2019 private
>>> -rw------- 1 root root????????? 528384 Jul 19? 2019 registry.tdb
>>> -rw------- 1 root root????????? 421888 Jul 21? 2019 share_info.tdb
>>> drwxrwx--T 1 root sambashare???????? 0 Jul 19? 2019 usershares
>>> -rw------- 1 root root?????????? 32768 Dec? 1 19:57
winbindd_cache.tdb
>>> -rw-r--r-- 1 root root????????? 421888 Jul 20? 2019
winbindd_idmap.tdb
>>> drwxr-x--- 1 root winbindd_priv????? 8 Nov? 4 17:08
winbindd_privileged
>>> root at lxd-m1:~# stat? /var/lib/samba/usershares
>>> File: /var/lib/samba/usershares
>>> Size: 0?????????????? Blocks: 0????????? IO Block: 4096 directory
>>> Device: 41h/65d Inode: 24656?????? Links: 1
>>> Access: (1770/drwxrwx--T)? Uid: (??? 0/??? root)?? Gid: ( 
>>> 111/sambashare)
>>> Access: 2020-12-01 19:00:10.120922818 +0000
>>> Modify: 2019-07-19 12:14:08.718571118 +0100
>>> Change: 2020-01-17 09:43:21.506782092 +0000
>>> Birth: -
>>> root at lxd-m1:~#
>>
>> This is triggered by this code:
>>
>> ?????? if (!S_ISREG(psbuf->st_ex_mode)) {
>> ?????????????? DEBUG(0,("check_usershare_stat: file %s owned by
uid
>> %u is "
>> ?????????????????????? "not a regular file\n",
>> ?????????????????????? fname, (unsigned int)psbuf->st_ex_uid ));
>> ?????????????? return false;
>> ?????? }
>>
>> but it should be looking at the files inside /var/lib/samba/usershares,
>> not the /var/lib/samba/usershares directory itself.
>>
>> This is called from load_usershare_service(), which is passed
>> the name of the share to find as a usershare. It looks like
>> this is being passed an empty "" string.
>
> Here is the code that is processing the usersharepath:
>
> In your case - usersharepath = "/var/lib/samba/usershares".
>
> As you can see below (error return processing removed for
> clarity), it's passing the return from readdir(dp)->d_name
> to the process_usershare_file() code that calls check_usershare_stat().
>
> ??????? dp = opendir(usersharepath);
> ??????? for (num_dir_entries = 0, num_bad_dir_entries = 0, 
> num_tmp_dir_entries = 0;
> ??????????????????????? (de = readdir(dp));
> ??????????????????????? num_dir_entries++ ) {
> ??????????????? int r;
> ??????????????? const char *n = de->d_name;
>
> ??????????????? /* Ignore . and .. */
> ??????????????? if (*n == '.') {
> ??????????????????????? if ((n[1] == '\0') || (n[1] == '.'
&& n[2] ==
> '\0')) {
> ??????????????????????????????? continue;
> ??????????????????????? }
> ??????????????? }
>
> ??????????????? if (n[0] == ':') {
> ??????????????????????? /* Temporary file used when creating a share. */
> ??????????????????????? num_tmp_dir_entries++;
> ??????????????? }
>
> ??????????????? r = process_usershare_file(usersharepath, n, 
> snum_template);
>
> If n=="" here, then we'd get (error processing removed):
>
> static int process_usershare_file(const char *dir_name, const char 
> *file_name, int snum_template)
> {
> ...
> ?????? /* Ensure share name doesn't contain invalid characters. */
> ??????? if (!validate_net_name(file_name, INVALID_SHARENAME_CHARS, 
> strlen(file_name))) {
> ??????? ...
> ??????????????? goto out;
> ??????? }
>
> ??????? canon_name = canonicalize_servicename(ctx, file_name);
> ??????? fname = talloc_asprintf(ctx, "%s/%s", dir_name,
file_name);
>
> ??????? /* Minimize the race condition by doing an lstat before we
> ?????????? open and fstat. Ensure this isn't a symlink link. */
>
> ??????? if (sys_lstat(fname, &lsbuf, false) != 0) {
> ??????????????? goto out;
> ??????? }
>
> ??????? /* This must be a regular file, not a symlink, directory or
> ?????????? other strange filetype. */
> ??????? if (!check_usershare_stat(fname, &lsbuf)) {
> ...
>
> n="", fname = "/var/lib/samba/usershares/" and
you'd get the results
> you see.
>
> Can you add some debugging inside the directory enumeration
> code to see what you're getting back from the readdir() ?
>
could this have anything to do with it:

4.12.0
Default: usershare max shares = 0

4.13.2
Default: usershare max shares = 100

Rowland

On Tue, Dec 01, 2020 at 10:08:34PM +0000, Rowland penny via samba wrote:
>could this have anything to do with it:
>
>4.12.0
>Default: usershare max shares = 0
>
>4.13.2
>Default: usershare max shares = 100
Good catch. Yes, that would cause
the usershare load path to be executed
now whereas it wasn't before.

Relevent code:

int load_usershare_service(const char *servicename)
{
         SMB_STRUCT_STAT sbuf;
         const char *usersharepath = Globals.usershare_path;
         int max_user_shares = Globals.usershare_max_shares;
         int snum_template = -1;

         if (*usersharepath == 0 ||  max_user_shares == 0) {
                                     ^^^^^^^^^^^^^^^^^^^^^
                                       4.12 causes the return -1,
                                       4.13 continues into the code.
                 return -1;
         }