On Sat, Jul 15, 2023 at 04:53:02PM -0400, Steve Dickson wrote: > > This is not the case of rpc-statd: if sm and sm.bak (under > > $statdpath, which also defaults to /var/lib/nfs) are missing the > > daemon will refuse to start and will exit with an error. > Why are they would be missing? They are created on the nfs-utils > installation. Hello, yes, in a traditional Linux system that is indeed the case. The idea behind this is to add support to factory reset and stateless scenarios like the ones described here: https://0pointer.net/blog/projects/stateless.html The goal is that a system can boot with an empty /var and all necessary files and directories are created without user intervention. In the case of nfs-utils this is already happening except for rpc-statd. For projects that use systemd this is generally easy to do without touching the code because systemd provides directives that can be used to ensure that /var/lib/foo, /var/log/foo, etc. exist before a service is started. In the rpc-statd case this would normally be as simple as adding something like "StateDirectory=nfs/sm nfs/sm.bak" to the .service file. However it seems that this one is a bit special because it goes like this if I'm not mistaken: 1. The configure script determines $statduser (the value of --with-statduser, else rpcuser if available, else nobody). 2. 'make install' creates sm / sm.bak followed by chown $statduser 3. rpc.statd starts as root, then does lstat("/var/lib/nfs/sm", &st) and finally setgid(st.st_gid) / setuid(st.st_uid). At this point uid/gid is not necessarily what was set during configure/make install ($statduser/root) because downstreams can create a different user/group and change the ownership of those directories. StateDirectory and similar directives from systemd can only create directories owned by the user that starts the service, but since here the service needs to run as root this would not work. systemd-tmpfiles can be used for cases like this one, and that's why I chose it for this patch. > Just curious... how did you test this patch? When I apply it > I get this error > > Failed to insert: creating /var/lib/nfs/statd/sm/<client>: Permission denied > STAT_FAIL to <server> for SM_MON of <server_ip> > > Maybe this is packing issue but I'm thinking it is more > of systemd issue... the permissions on the sm directory > are > 283 drwx------. 2 nobody rpcuser 6 Apr 18 20:00 /var/lib/nfs/statd/sm > instead of > 283 drwx------. 2 rpcuser rpcuser 6 Apr 18 20:00 /var/lib/nfs/statd/sm Are you creating a package with the patched sources? If it's something like the Fedora one then I think that the problem is that since the configure script does not use --with-statduser then there's a mismatch between the user that appears in nfs-utils.conf (added by this patch) and these lines from the .spec file: %dir %attr(700,rpcuser,rpcuser) %{_sharedstatedir}/nfs/statd %dir %attr(700,rpcuser,rpcuser) %{_sharedstatedir}/nfs/statd/sm %dir %attr(700,rpcuser,rpcuser) %{_sharedstatedir}/nfs/statd/sm.bak So probably /var/lib/nfs/statd/sm is drwx------ nobody but /var/lib/nfs/statd is drwx------ rpcuser ? Passing --with-statduser=rpcuser to configure should fix this problem. After having a look at a couple of downstream packages it seems that they simply don't use --with-statduser at all and change the ownership to whatever user/group they want in their post-installation scripts. So they would need to start doing it if this patch is included in nfs-utils. I realize that although this should be trivial to handle by downstream packagers it does require manual intervention so I'm not expecting it to be completely uncontroversial. But if you like the overall idea I'm happy to discuss / iterate this patch further. This can of course be applied only by the downstreams who are interested in this feature, but since nfs-utils already uses systemd and the change is rather small I thought it made more sense to have it directly upstream. Regards, Berto
