Bug 6318 - registry shares with empty path lead to share with root directory access
Summary: registry shares with empty path lead to share with root directory access
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.3
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.3.4
Hardware: Other All
: P3 normal
Target Milestone: ---
Assignee: Michael Adam
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-04 08:33 UTC by Michael Adam
Modified: 2009-05-06 02:31 UTC (History)
1 user (show)

See Also:


Attachments
patch for the bug (8.54 KB, patch)
2009-05-05 07:14 UTC, Michael Adam
no flags Details
minimal version of the patch (1.51 KB, patch)
2009-05-05 10:55 UTC, Michael Adam
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Adam 2009-05-04 08:33:00 UTC
A share defined in registry configuration without a path option
is not marked unavailable as a text-defined share, but it is
available with "path = /" instead.

This bug applies to all Samba versions since 3.2.

It has been fixed in 3.4 and master.

Michael
Comment 1 Michael Adam 2009-05-05 07:14:25 UTC
Created attachment 4117 [details]
patch for the bug

Attacht find a git patch mbox (comprised of 4 individual commits) to be
applied to the v3-3-test branch.

I'd like these to get into the next 3.3 bugfix release.

Michael
Comment 2 Volker Lendecke 2009-05-05 08:23:08 UTC
Isn't f7562e7bf7a7779b2f the only one that is really required to fix the bug?

Volker
Comment 3 Michael Adam 2009-05-05 08:58:33 UTC
(In reply to comment #2)
> Isn't f7562e7bf7a7779b2f the only one that is really required to fix the bug?
> 
> Volker

No, this just make testparm pretend the share is unavailable.
The important other path is to convert smbd/service.c to use the
higher level routines in loadparm that use libsmbconf and do the
path checks.

Michael
Comment 4 Volker Lendecke 2009-05-05 08:59:49 UTC
Ok, sorry. Going back to review the  rest.

Volker
Comment 5 Volker Lendecke 2009-05-05 09:22:05 UTC
Ok, we have to make a policy decision here. For the 3.3 release, do we want to have a "minimal patch" policy, or do we want to port back larger changes from master for fixes?

If we want the former, then this patch is too large, if we want the latter, then this patch is certainly fine.

This is something we need a community decision on, I can't decide this on my own.

Volker
Comment 6 Michael Adam 2009-05-05 09:41:55 UTC
(In reply to comment #5)
> Ok, we have to make a policy decision here. For the 3.3 release, do we want to
> have a "minimal patch" policy, or do we want to port back larger changes from
> master for fixes?
> 
> If we want the former, then this patch is too large, if we want the latter,
> then this patch is certainly fine.
> 
> This is something we need a community decision on, I can't decide this on my
> own.

While I think that the patch I have attached is conceptually the right thing to do, it is admittedly more than a pure bugfix for 3.3, so I am going to make a more minimal patch that repeats the check done in loadparm in the server, and I am going to attach that patch for comparison.

Michael
Comment 7 Michael Adam 2009-05-05 09:49:01 UTC
Uh, Crap - I just noticed an error in my patch (in the "obvious" part...)
I need to figure out why it seems to work anyways.
Please forget the attached patch until then...

Michael
Comment 8 Michael Adam 2009-05-05 10:55:18 UTC
Created attachment 4120 [details]
minimal version of the patch

here is a minimal version of the patch.
Also the loadparm portion is fixed
(the original version did not use a return value of service_ok(), which was not bad in our case, but this one is correct)
Comment 9 Volker Lendecke 2009-05-05 13:43:42 UTC
That's kindof what I had expected, so +1 :-)

Volker
Comment 10 Karolin Seeger 2009-05-06 02:31:29 UTC
Patch (min version) is upstream and will be included in 3.3.5.
Closing out bug report.