[PATCH] Master fix for bug #9518 - conn->share_access appears not be be reset between users

[Andrew Bartlett]

On Tue, 2013-01-08 at 13:29 -0800, Jeremy Allison wrote:
> On Wed, Jan 09, 2013 at 08:19:22AM +1100, Andrew Bartlett wrote:
> > On Tue, 2013-01-08 at 12:14 -0800, Jeremy Allison wrote:
> > > On Tue, Jan 08, 2013 at 10:38:40PM +1100, Andrew Bartlett wrote:
> > > > On Tue, 2013-01-08 at 18:33 +1100, Andrew Bartlett wrote:
> > > > > On Tue, 2013-01-08 at 08:09 +0100, Stefan (metze) Metzmacher wrote:
> > > > > > Am 06.01.2013 15:13, schrieb Volker Lendecke:
> > > > > > > On Fri, Jan 04, 2013 at 04:12:55PM -0800, Jeremy Allison wrote:
> > > > > > >> Please review and apply to master if ok. It includes
> > > > > > >> Andrew's regression test patch also.
> > > > > > > 
> > > > > > > Looks very well crafted to me. I haven't tested it, but it
> > > > > > > looks completely sane. Attached find some cosmetic patches
> > > > > > > on top.
> > > > > > > 
> > > > > > > With best regards,
> > > > > > 
> > > > > > Reviewed-by: Stefan Metzmacher <metze at samba.org>
> > > > > 
> > > > > Thanks.  All these (Volker's cosmetic patches included) are now on their
> > > > > way to autobuild.  
> > > > 
> > > > Jermey,
> > > > 
> > > > I've got bad news sorry.  
> > > > 
> > > > source3/printing/nt_printing.c:get_correct_cversion() calls
> > > > become_user_by_session() which uses UID_FIELD_INVALID.
> > > > 
> > > > This means that on sn-devel (but not on my Fedora system!) we segfault:
> > > 
> > > Ok, can you try this revised version. It's the same as the previous
> > > patchset, with Volker's cleanup fixes included, plus this additional
> > > change which should cope with the special become_user_by_session()
> > > case without having to restore all the UID_FIELD_INVALID code.
> > > 
> > > I'll upload this to the bug report, and also revise the 4.0.x
> > > patchset on the bug report.
> > > 
> > > Thanks a *lot* for catching that use-case. As you can see,
> > > this code is really tricky to get right (holdovers from having
> > > to support "force user" and "force group" correctly).
> > > 
> > > Here is the additional change as text so you can see what I did
> > > here:
> > 
> > While this is helpful (avoids creating a 'conn->session_info == NULL',
> > it isn't actually the fix.  The issue is lack of initialisation in the
> > other code paths that can create a conn.  We may need to fix up vfstest
> > as well (preferably to also call create_conn_struct()).
> > 
> > The attached fix tries to call a common routine to fix this and avoid
> > the issue in the future.
> 
> Ah, not being able to reproduce the crash here meant I didn't
> see the backtrace.

It's very odd that a NULL pointer de-reference would be
platform-specific, but that's why I included the backtrace.  That said,
I totally misread it last night. :-)

> So the issue is with conn->vuid_cache == NULL because conn_new()
> isn't being called inside create_conn_struct() ?

Correct.

That is my analysis, and what I fixed in the patch I attached.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org