[PATCH] SMB2 AAPL create context

[Ralph Böhme]

Hi Stefan

On Thu, Sep 11, 2014 at 04:20:23PM +0200, Stefan (metze) Metzmacher wrote:
> I think I found a little bug in your code...

guess you found gaping holes, not little bugs. :)

> Am 11.09.2014 um 15:22 schrieb Ralph Böhme:
> > +
> > +		if (aapl) {
> > +			/* We know we have a SMB2_CRTCTX_AAPL_SERVER_QUERY query */
> > +			bool ok;
> > +			uint8_t p[8];
> 
> This should be p[16];

*ouch* fixed.

> 
> > +			DATA_BLOB blob = data_blob_talloc(smb2req, NULL, 0);
> > +			uint64_t req_bitmap, client_caps;
> > +			uint64_t server_caps = SMB2_CRTCTX_AAPL_UNIX_BASED;
> > +
> > +			req_bitmap = BVAL(aapl->data.data, 8);
> > +			client_caps = BVAL(aapl->data.data, 16);
> > +
> > +			SIVAL(&p, 0, SMB2_CRTCTX_AAPL_SERVER_QUERY);
> > +			SIVAL(&p, 4, 0);
> > +			SBVAL(&p, 8, req_bitmap);
> 
> This should be
> 
> 			SIVAL(p, 0, SMB2_CRTCTX_AAPL_SERVER_QUERY);
> 			SIVAL(p, 4, 0);
> 			SBVAL(p, 8, req_bitmap);
> 
> 'p' vs. &p also in other places...

Iirc it doesn't matter, the pointer *type* will be different, but not
the pointed to object:

$ cat pointer-magic.c
#include <stdio.h>

static void func(void *p1, void *p2)
{
    if (p1 != p2) {
        printf("yikes!\n");
    }
}

int main(int argc, char **argv)
{
    char foo[1];
    func(foo, &foo);
    func(&foo, &foo[0]);
    func(foo, &foo[0]);
    return 0;
}
$ gcc -Wall -o pointer-magic pointer-magic.c
$ ./pointer-magic 
$ 

But then, I really don't know what made me choose &p because I don't
dislike this style, so I've fixed it for the next patchset update.

> 
> > 			if (req_bitmap & SMB2_CRTCTX_AAPL_SERVER_CAPS) {
> > +				if ((client_caps & SMB2_CRTCTX_AAPL_SUPPORTS_READ_DIR_ATTR) &&
> > +				    (smb2req->tcon->compat->fs_capabilities & FILE_NAMED_STREAMS)) {
> > +					server_caps |= SMB2_CRTCTX_AAPL_SUPPORTS_READ_DIR_ATTR;
> > +					smb2req->tcon->compat->smb2_crtctx_aapl_readdir_attr = true;
> > +				}
> 
> Is it really correct to store smb2_crtctx_aapl_readdir_attr on the tree
> connect?

Oh, good catch, no it isn't, in fact...

> I'd expect that this is relative to the currently opened handle.

...it's relative to the whole SMB2 session. So you can't have one
share with AAPL and one without.

Client and server negotiate this capability once at the first tree
connect, but it's then fixed for the whole session.

> Is it really possible to ask for this on every file and directory?

Probably. In practice the client does it in an open on the share base
directory.

> What is with printer shares or IPC$?
> 
> Can you write some torture tests which prove the correct behavior?

Yep, will do.

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de