[PATCHES][BUG 13189] Fix coredump on failing chdir during logoff

Christof Schmitt cs at samba.org
Fri Dec 15 22:14:17 UTC 2017


On Fri, Dec 15, 2017 at 12:38:43PM -0800, Jeremy Allison wrote:
> On Sat, Dec 16, 2017 at 09:14:39AM +1300, Andrew Bartlett via samba-technical wrote:
> > On Fri, 2017-12-15 at 11:24 -0800, Jeremy Allison wrote:
> > > On Thu, Dec 14, 2017 at 01:06:13PM -0700, Christof Schmitt via samba-technical wrote:
> > > > On Thu, Dec 14, 2017 at 12:02:18PM +1300, Andrew Bartlett wrote:
> > > > > On Wed, 2017-12-13 at 15:50 -0700, Christof Schmitt wrote:
> > > > > > On Thu, Dec 14, 2017 at 11:07:00AM +1300, Andrew Bartlett wrote:
> > > > > > > On Wed, 2017-12-13 at 13:24 -0700, Christof Schmitt via samba-technical 
> > > > > > > wrote:
> > > > > > > 
> > > > > > > This looks great!  I love that you went to such effort to create an
> > > > > > > automated test for this!
> > > > > > 
> > > > > > Thanks. I wanted an easy way to trigger this problem, and it might be
> > > > > > useful in general to trigger errors for testing. Wrapping that in test
> > > > > > script was then a quick addition.
> > > > > > 
> > > > > > > > +       "SERVERCONFFILE",
> > > > > > > 
> > > > > > > This is the only part I don't think you need, you can use SMB_CONF_FILE
> > > > > > > for this as you are running with :local (that is essentially what
> > > > > > > :local does). 
> > > > > > 
> > > > > > Correct, i missed that part. Updated patches are attached.
> > > > > 
> > > > > I think we should we check that a panic would be detected.  I realise
> > > > > that is a real pain, but otherwise I fear a change to the logging would
> > > > > render the patch neutered. 
> > > > > 
> > > > > Just make the fault injection module also call panic directly. 
> > > > > 
> > > > > Then, finally, put the test first, with a knownfail, and then remove
> > > > > the knownfail with the fix.
> > > > > 
> > > > > To be clear, this is great, but if you can do this one last change I
> > > > > would appreciate it.
> > > > 
> > > > See attached patches for these changes.
> > > 
> > > Nice work Christof, thanks. RB+ and pushed !
> > 
> > I likewise reviewed and pushed it.  However autobuild fails, it needs
> > the attached fixup.  (I had meant to push it again with this folded in,
> > but clearly did not). 
> 
> Thanks Andrew, will squash and re-push.

Thank you both. It might make sense to use static_decl_vfs in all vfs
modules for consistency and to provide the correct example for anybody
looking into existing code to start a new module. I can look into
writing a patch for this.

Christof



More information about the samba-technical mailing list