cifs unable to access shares with read restricted at root level (bso#8950)

cifs unable to access shares with read restricted at root level (bso#8950)

[Aurélien Aptel]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

Hi!

I've recently started working on bso#8950 [1] but I'm both a
kernel/SMB protocol newbie so bear with me... :-)

Shirish Pargaonkar provided a fix but it doesn't work on my setup. I
would like to provide a fix for SUSE even if temporary/hacky. Shirish's
approach seems sound so I've tried debugging it. I've summarized the
situation and my comments here [2].

There are a few things I don't understand yet.

* Where are disconnected dentries stored?
* How do you allocate a new dentry?
* What do d_obtain_alias() and d_splice_dentry() actually do? I've read
  the doc strings several times but I still have a hard time wrapping my
  head around it.

The result I'm getting seems to indicate that when an intermediary path
element is inaccessible an alternate root dentry directly pointing to
the requested path is created. The problem is that the prefixpath of
that is not stored anywhere so when we ask for a pattern to list the
content of that share, we use the path of the dentry from the root
(which is, well /) instead of the share prefixpath.

1: https://bugzilla.samba.org/show_bug.cgi?id=8950
2: http://diobla.info/doc/suse-todo#bnc799133

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: cifs unable to access shares with read restricted at root level (bso#8950)

[Aurélien Aptel]

[-- Attachment #1: Type: text/plain, Size: 2251 bytes --]

I wanted to add a few things....

On Mon, 26 Oct 2015 13:18:17 +0100 Aurélien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
wrote:
> * How do you allocate a new dentry?  

That's too broad, forget this one. There are many functions to do that
depending on what you want.

> * What do d_obtain_alias() and d_splice_dentry() actually do? I've
> read the doc strings several times but I still have a hard time
> wrapping my head around it.  

To be more specific, where does d_obtain_alias() look for a dentry
associated with the provided inode? I would guess in all the connected
dentries for the inode superblock. If it's not there, what is the path
of the newly allocated dentry?

d_splice_alias() looks for an existing dentry associated with the
provided inode and if it founds one, replaces it with the one provided.
Correct?

Really, I think my biggest misunderstanding is, what does it mean for a
dentry to be disconnected?

> The result I'm getting seems to indicate that when an intermediary
> path element is inaccessible an alternate root dentry directly
> pointing to the requested path is created. The problem is that the
> prefixpath of that is not stored anywhere so when we ask for a
> pattern to list the content of that share, we use the path of the
> dentry from the root (which is, well /) instead of the share
> prefixpath.  

To illustrate this:

* User bill can only access HOST/share/sub/path and none of the
  intermediary path.

* When we mount this path to /mnt we realize we cannot access
  intermediary path.

* We create an alternative root which directly sores
  HOST/share/sub/path as / (but /sub/path/ is not stored anywhere!).

* When we ls /mnt, we use this alternate root path. The function that
  builds the share path from a dentry (build_path_from_dentry) simply
  walk the d_parent dentry until it reach /. In that case it immediatly
  stops because the disconnected root is.. a root. It should
  use /sub/path instead.

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: cifs unable to access shares with read restricted at root level (bso#8950)

[Shirish Pargaonkar]

On Mon, Oct 26, 2015 at 7:18 AM, Aurélien Aptel <aaptel-IBi9RG/b67k@public.gmane.org> wrote:
> Hi!
>
> I've recently started working on bso#8950 [1] but I'm both a
> kernel/SMB protocol newbie so bear with me... :-)
>
> Shirish Pargaonkar provided a fix but it doesn't work on my setup. I
> would like to provide a fix for SUSE even if temporary/hacky. Shirish's
> approach seems sound so I've tried debugging it. I've summarized the
> situation and my comments here [2].
>
> There are a few things I don't understand yet.
>
> * Where are disconnected dentries stored?

They are stored in a linked list of struct cifs_rdelem off of
superblock where each element of this linked list points to a
disconnected dentry.

> * How do you allocate a new dentry?
> * What do d_obtain_alias() and d_splice_dentry() actually do? I've read
>   the doc strings several times but I still have a hard time wrapping my
>   head around it.

I think the simplistic answer is d_obtain_alias creates a dentry without
a parent and d_splice_dentry connects a disconnected dentry to a parent.
So let us say you have a a filesystem like /a/b/c/d/e/f where c is
not accessible, we would create a disconnected for d (and for all the
entries starting d such as e and f)
Then when c is accessible and we are trying to reach f, while
traversing, for each dentry we would look whether it is disconnected
or not and if found (by any of three search routines), splice dentry with
c.

What kind of setup do you have where the patchset is not working?

>
> The result I'm getting seems to indicate that when an intermediary path
> element is inaccessible an alternate root dentry directly pointing to
> the requested path is created. The problem is that the prefixpath of
> that is not stored anywhere so when we ask for a pattern to list the
> content of that share, we use the path of the dentry from the root
> (which is, well /) instead of the share prefixpath.
>
> 1: https://bugzilla.samba.org/show_bug.cgi?id=8950
> 2: http://diobla.info/doc/suse-todo#bnc799133
>
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
> Nürnberg)

Re: cifs unable to access shares with read restricted at root level (bso#8950)

[Aurélien Aptel]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

Hi Shirish,

On Mon, 26 Oct 2015 22:04:28 -0500 Shirish Pargaonkar
<shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> They are stored in a linked list of struct cifs_rdelem off of
> superblock where each element of this linked list points to a
> disconnected dentry.

This list is only used use when we mount with noserverino. It doesn't
work in both case anyway.
 
> > * How do you allocate a new dentry?
> > * What do d_obtain_alias() and d_splice_dentry() actually do? I've
> > read the doc strings several times but I still have a hard time
> > wrapping my head around it.
> 
> I think the simplistic answer is d_obtain_alias creates a dentry
> without a parent and d_splice_dentry connects a disconnected dentry
> to a parent. So let us say you have a a filesystem like /a/b/c/d/e/f
> where c is not accessible, we would create a disconnected for d (and
> for all the entries starting d such as e and f)
> Then when c is accessible and we are trying to reach f, while
> traversing, for each dentry we would look whether it is disconnected
> or not and if found (by any of three search routines), splice dentry
> with c.
> 
> What kind of setup do you have where the patchset is not working?

Everything is explained on the link I provided. It's the same setup you
used I think.

  http://diobla.info/doc/suse-todo#bnc799133

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: cifs unable to access shares with read restricted at root level (bso#8950)

[Shirish Pargaonkar]

On Tue, Oct 27, 2015 at 4:59 AM, Aurélien Aptel <aaptel-IBi9RG/b67k@public.gmane.org> wrote:
> Hi Shirish,
>
> On Mon, 26 Oct 2015 22:04:28 -0500 Shirish Pargaonkar
> <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> They are stored in a linked list of struct cifs_rdelem off of
>> superblock where each element of this linked list points to a
>> disconnected dentry.
>
> This list is only used use when we mount with noserverino. It doesn't
> work in both case anyway.

No, that is not the case. I have tested this against a Windows 2012
server and did not mount the shares with noserverino option.
Since we can query info off of disconnected entires, we fill the inodes
which get filled with correct inode info as dentires get spliced.

>
>> > * How do you allocate a new dentry?
>> > * What do d_obtain_alias() and d_splice_dentry() actually do? I've
>> > read the doc strings several times but I still have a hard time
>> > wrapping my head around it.
>>
>> I think the simplistic answer is d_obtain_alias creates a dentry
>> without a parent and d_splice_dentry connects a disconnected dentry
>> to a parent. So let us say you have a a filesystem like /a/b/c/d/e/f
>> where c is not accessible, we would create a disconnected for d (and
>> for all the entries starting d such as e and f)
>> Then when c is accessible and we are trying to reach f, while
>> traversing, for each dentry we would look whether it is disconnected
>> or not and if found (by any of three search routines), splice dentry
>> with c.
>>
>> What kind of setup do you have where the patchset is not working?
>
> Everything is explained on the link I provided. It's the same setup you
> used I think.
>
>   http://diobla.info/doc/suse-todo#bnc799133

Sorry, missed looking at the link before. yes, the same setup.
I could never get right seutp for Samba server.

>
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
> Nürnberg)

Re: cifs unable to access shares with read restricted at root level (bso#8950)

[Aurélien Aptel]

[-- Attachment #1: Type: text/plain, Size: 2888 bytes --]

Oops, I forgot to CC my last email to the mailing list; re-sending now.

On Tue, 27 Oct 2015 19:43:46 -0500 Shirish Pargaonkar
<shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Oct 27, 2015 at 4:59 AM, Aurélien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
> wrote:  
> > Hi Shirish,
> >
> > On Mon, 26 Oct 2015 22:04:28 -0500 Shirish Pargaonkar
> > <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:  
> >> They are stored in a linked list of struct cifs_rdelem off of
> >> superblock where each element of this linked list points to a
> >> disconnected dentry.  
> >
> > This list is only used use when we mount with noserverino. It
> > doesn't work in both case anyway.  
> 
> No, that is not the case. I have tested this against a Windows 2012
> server and did not mount the shares with noserverino option.
> Since we can query info off of disconnected entires, we fill the
> inodes which get filled with correct inode info as dentires get
> spliced.  

The only function that adds elements to the rdlist is this one, and it
only does so with noserverino. I believe d_obtain_alias() doesn't store
the full path on the share associated with the disconnected entry it creates.
The full path is therefore lost and build_path_from_dentry() fails.

static struct dentry *
create_root_dis_dentry(struct super_block *sb, struct inode *rinode,
			char *fpath)
{
	int rc;
	unsigned int xid;
	struct dentry *dentry = NULL;
	struct cifs_rdelem *rdelem = NULL;
	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);

	printk("create_root_dis_dentry %s, rinode %p\n", fpath, rinode);
	
	xid = get_xid();
	if (tcon->unix_ext)
		rc = cifs_get_inode_info_unix(&rinode, fpath, sb, xid);
	else
		rc = cifs_get_inode_info(&rinode, fpath, NULL, sb, xid, NULL);
	free_xid(xid);
	
	if ((rc == 0) && (rinode != NULL)) {
		dentry = d_obtain_alias(rinode);
		if (IS_ERR(dentry)) {
			iput(rinode);
			goto rdelem_ret;
		}

		if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {

			rdelem =
				cifs_alloc_rdelem(fpath, dentry, rinode);
			if (IS_ERR(rdelem))
				goto rdelem_ret;
			spin_lock(&cifs_sb->rtdislock);
			list_add(&rdelem->rdlist, &cifs_sb->rtdislist);
			spin_unlock(&cifs_sb->rtdislock);
		}
		return dentry;
	}

rdelem_ret:
	return ERR_PTR(-EACCES);
}

 
> Sorry, missed looking at the link before. yes, the same setup.
> I could never get right seutp for Samba server.  

I must be missing something because I cannot make your patch work.
We're using the exact same setup.

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]