On 04/23/2010 03:12 PM, Chuck Lever wrote: > Hi Steve- > > On 04/23/2010 02:22 PM, Steve Dickson wrote: >> On 04/23/2010 01:28 PM, Chuck Lever wrote: >>> On 04/23/2010 12:29 PM, Steve Dickson wrote: >>>> On 04/20/2010 04:46 AM, Mike Frysinger wrote: >>>>> The new code using libcap is quite minor, so rather than always >>>>> reqiure >>>>> libcap support, make it a normal --enable type flag. Current default >>>>> behavior is retained -- if libcap is found, it is enabled, else it is >>>>> disabled like every nfs-utils version in the past. >>>>> >>>>> Signed-off-by: Mike Frysinger<vapier@xxxxxxxxxx> >>>>> >>>> Committed... >>> >>> I somehow missed this one. Why are we disabling libcap? And why are we >>> adding another --enable flag when everyone has agreed that we should >>> avoid that if at all possible? >> The justification I was used was it made nfs-utils more portable on >> systems/distros that may not have the libcap support. > > As an aside, the patch description is where we should be documenting the > thinking behind these decisions in an audit-able and transparent manner. > The description for this patch doesn't have a strong justification > IMHO. It would be hard for any of us to come back to this patch a year > from now and figure out exactly why this change was made. (I say this > having spent the last year doing just that for a long history of patches > to statd and mount). True, the patch description could have been a bit more verbose, but I feel I understood the reason for the patch and that reason the made sense to me... I feel backwards compatibility is important.. > > Back on topic: I get it that, in general, we want to allow older distros > to build the latest nfs-utils. However I don't think we can blithely > rip this libcap support out, even just for old configurations. > > If we really do need to drop libcap for some configurations, then such a > change should be thoroughly tested in those environments. Some features > won't always work without libcap, and appropriate warnings should be > added to man pages and/or should be displayed by statd. Well dropping libcap is not the default and I don't see us (i.e. upstream) ever making it the default... If people want set that config flag, its up to them to document the ramifications, IMHO... > >>> It is especially on older systems that nfs-utils will break without >>> libcap support. Without CAP_NET_BIND, pmap_unregister() will fail when >>> statd is shut down, leaving NSM registered with the portmapper, but with >>> no active listeners. When statd is started up again, it won't be able >>> to register the new NSM listener ports. >> Hmm... I agree the unregister() would fail on exit, but that's the reason >> an unregister() (and then an register) is done on start up before the >> privileges are drop... Actually this how it worked for a very long time, >> well before the capabilities support added... > > When I was working on it, subsequent attempts to register would always > fail if an NSM service was already registered. In other words, this was > broken when I found it. > > Commits e2446fda and 7dd13420 explain why CAP_NET_BIND was introduced, > and what bugs are addressed. Without CAP_NET_BIND, we can't guarantee > that the NSM service can be unregistered, and neither can we guarantee > that a privileged port, when requested, can be used for listening. > > The problem is that statd drops its root privileges, so any subsequent > attempts to acquire a privileged port (such as to do a > pmap_unregister()) will fail, and leave the NSM service registered. > > Since rpcbind registration is done via AF_UNIX, it can work without > CAP_NET_BIND. But it requires that the registering UID be the same as > the UID used to unregister it. Thus both registration and > unregistration must be root, or both must be done as "rpcuser." Since > statd drops its privileges just after start-up, I chose the latter. > > However, using lower privileges means a pmap_unregister() will always > fail in common cases. So CAP_NET_BIND is retained for this purpose. > > We also have to worry about mount.nfs these days, as it pings the statd > service when mounting with "lock". If NSM is registered, but no statd > is listening (as would be the case if statd couldn't unregister itself > on its way down), then most subsequent NFSv2/v3 mounts would hang. This > is why even "unregister/register" at start-up isn't always adequate. > I can't disagree with any of the above... but the above assumes that the --disable libcap will some how become the default... that is not the case... All that config flag allows is backwards compatibility, which I know we both think is a good thing... steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
