[PATCH] Fix failure to update dirpath

Jeremy Allison jra at samba.org
Tue Aug 21 16:54:05 UTC 2018


On Tue, Aug 21, 2018 at 11:42:40AM +0530, Anoop C S wrote:
> On Mon, 2018-08-20 at 18:35 -0700, Jeremy Allison wrote:
> > On Mon, Aug 20, 2018 at 06:02:31PM -0700, Jeremy Allison via samba-technical wrote:
> > > 
> > > Hang on a minute. I've been looking really closely
> > > now at the *callers* of check_parent_exists() and
> > > many of them depend on *dirpath == '\0' in the
> > > case where smb_fname->base_name doesn't contain
> > > a component containing '/' characters.
> > > 
> > > I think my fix, although it might work, will affect
> > > other paths. Let me look at this more closely
> > > tomorrow. I'm withdrawing this for now.
> > 
> > OK, my fix was way too invasive for the fix that
> > you need to make Gluster work again for that bug.
> > 
> > The problem is that the code that constructs
> > the complete pathname depends on *dirpath == '\0'
> > to avoid adding a "./" prefix to every constructed
> > pathname. My patch would cause all of those
> > code paths to change to the full/component/path
> > cases, which is not what we want. That's going
> > to affect what goes into the stat-cache as well
> > as many other things I'll have to go through
> > carefully to find.
> 
> Ok.
> 
> > This is a set of code-paths that we might clean
> > up in the future, but not for this specific bug
> > I think.

FYI, the attached diff is the full clean-up
that is required to make everything
work correctly (otherwise it fails in the
samba3.blackbox.smbclient_s3.SMB3.plain
test).

I'm not planning to propose this as it's a
more invasive change than we need, but I wanted
to make sure I fully understood what the
issue was :-).

> > Can you test the following change instead ?
> 
> The attached diff also works.

Thanks for testing !

> > Much simpler and I think it will fix the immediate
> > Gluster problem (it's the same change that already
> > exists inside get_real_filename_full_scan() so
> > I think this is something we already ran across
> > there).
> 
> Considering the fact that we are not 100% sure about the side effects of previous fix which was more
> aggressive, the current solution is more clean and elegant. In addition to that the presence of same
> statements inside get_real_filename_full_scan(), which is a fallback, gives us more confidence to
> fix it here.
> 
> I hope you you will be attaching the diff as a patch to other thread for review.

Yep, I'll post the "simple" patch as the
proposed fix for bug:

https://bugzilla.samba.org/show_bug.cgi?id=13585

in another thread.

Thanks for all your help !

Cheers,

	Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-smbd-Ensure-check_parent_exists-returns-valid-pat.patch
Type: text/x-diff
Size: 4809 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180821/753bc167/0001-s3-smbd-Ensure-check_parent_exists-returns-valid-pat.diff>


More information about the samba-technical mailing list