Hi,
Upgrading the nfs-utils package from 2.3.4 to 2.4.1 on my nas device
running Arch Linux ARM (alarm) broke the nfs server functionality.
On all of my clients, mount fails with a "Stale file handle" error. On the
server, I get this (side note: the reported version number is wrong):
Jul 08 10:28:50 nas rpc.mountd[19752]: Version 2.3.4 starting
Jul 08 10:30:08 nas rpc.mountd[19752]: auth_unix_ip: inbuf 'nfsd <redacted valid ipv6 address>'
Jul 08 10:30:08 nas rpc.mountd[19752]: auth_unix_ip: client 0x492c50 'zero.local'
Jul 08 10:30:08 nas rpc.mountd[19752]: nfsd_fh: inbuf 'zero.local 1 \x00000000'
Jul 08 10:30:08 nas rpc.mountd[19752]: nfsd_fh: found 0x49b698 path /srv/nfs
Jul 08 10:30:08 nas rpc.mountd[19752]: nfsd_export: inbuf 'zero.local /srv/nfs/tmp2'
Jul 08 10:30:09 nas rpc.mountd[19752]: nfsd_export: found 0x499ed0 path /srv/nfs/tmp2
Jul 08 10:30:09 nas rpc.mountd[19752]: nfsd_fh: inbuf 'zero.local 7 \x0100140000000000ae18d6965c0a40f78701c770897a4fc>
Jul 08 10:30:09 nas rpc.mountd[19752]: nfsd_fh: found (nil) path (null)
Analysis:
Consider this code snippet from utils/mountd/cache.c:
627 static bool match_fsid(struct parsed_fsid *parsed, nfs_export *exp, char *path)
628 {
629 struct stat stb;
630 int type;
631 char u[16];
632
633 if (nfsd_path_stat(path, &stb) != 0)
634 return false;
Variable stb gets defined, then gets filled by nfs_path_stat(), which is
implemented in support/misc/nfsd_path.c. At least on alarm, definition of
struct stat in stat.h depends on __USE_FILE_OFFSET64, which comes from
config.h if defined. This requires config.h to be included before stat.h.
The include order is right in cache.c, however, it's reversed in
nfsd_path.c. This causes the data returned by nfs_path_stat() to be in a
different structure than expected, and that's what eventually causes the
error.
Proposed solution:
The following patch fixes those occurrences where the include order
between config.h and stat.h is wrong, by moving config.h to the top.
Regards,
Zoltan Karcagi
At least on Arch linux ARM, the definition of struct stat in stat.h depends
on __USE_FILE_OFFSET64. This symbol comes from config.h when defined,
therefore config.h must always be included before stat.h. Fix all
occurrences where the order is wrong by moving config.h to the top.
This fixes the client side error "Stale file handle" when mounting from
a server running Arch Linux ARM.
Signed-off-by: Zoltan Karcagi <[email protected]>
---
support/misc/nfsd_path.c | 5 ++++-
support/misc/xstat.c | 5 ++++-
utils/blkmapd/device-discovery.c | 8 ++++----
utils/idmapd/idmapd.c | 8 ++++----
4 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c
index 84e48028..f078a668 100644
--- a/support/misc/nfsd_path.c
+++ b/support/misc/nfsd_path.c
@@ -1,3 +1,7 @@
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
@@ -5,7 +9,6 @@
#include <stdlib.h>
#include <unistd.h>
-#include "config.h"
#include "conffile.h"
#include "xmalloc.h"
#include "xlog.h"
diff --git a/support/misc/xstat.c b/support/misc/xstat.c
index fa047880..4c997eea 100644
--- a/support/misc/xstat.c
+++ b/support/misc/xstat.c
@@ -1,3 +1,7 @@
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
#include <errno.h>
#include <sys/types.h>
#include <fcntl.h>
@@ -5,7 +9,6 @@
#include <sys/sysmacros.h>
#include <unistd.h>
-#include "config.h"
#include "xstat.h"
#ifdef HAVE_FSTATAT
diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
index e811703d..f5f9b10b 100644
--- a/utils/blkmapd/device-discovery.c
+++ b/utils/blkmapd/device-discovery.c
@@ -26,6 +26,10 @@
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif /* HAVE_CONFIG_H */
+
#include <sys/sysmacros.h>
#include <sys/types.h>
#include <sys/stat.h>
@@ -51,10 +55,6 @@
#include <errno.h>
#include <libdevmapper.h>
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif /* HAVE_CONFIG_H */
-
#include "device-discovery.h"
#include "xcommon.h"
#include "nfslib.h"
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 62e37b8a..267acea5 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -34,6 +34,10 @@
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif /* HAVE_CONFIG_H */
+
#include <sys/types.h>
#include <sys/time.h>
#include <sys/inotify.h>
@@ -62,10 +66,6 @@
#include <libgen.h>
#include <nfsidmap.h>
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif /* HAVE_CONFIG_H */
-
#include "xlog.h"
#include "conffile.h"
#include "queue.h"
--
2.22.0
Ping...
On 7/26/19 4:44 PM, Zoltan Karcagi wrote:
> At least on Arch linux ARM, the definition of struct stat in stat.h depends
> on __USE_FILE_OFFSET64. This symbol comes from config.h when defined,
> therefore config.h must always be included before stat.h. Fix all
> occurrences where the order is wrong by moving config.h to the top.
>
> This fixes the client side error "Stale file handle" when mounting from
> a server running Arch Linux ARM.
>
> Signed-off-by: Zoltan Karcagi <[email protected]>
> ---
> support/misc/nfsd_path.c | 5 ++++-
> support/misc/xstat.c | 5 ++++-
> utils/blkmapd/device-discovery.c | 8 ++++----
> utils/idmapd/idmapd.c | 8 ++++----
> 4 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c
> index 84e48028..f078a668 100644
> --- a/support/misc/nfsd_path.c
> +++ b/support/misc/nfsd_path.c
> @@ -1,3 +1,7 @@
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> #include <errno.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> @@ -5,7 +9,6 @@
> #include <stdlib.h>
> #include <unistd.h>
>
> -#include "config.h"
> #include "conffile.h"
> #include "xmalloc.h"
> #include "xlog.h"
> diff --git a/support/misc/xstat.c b/support/misc/xstat.c
> index fa047880..4c997eea 100644
> --- a/support/misc/xstat.c
> +++ b/support/misc/xstat.c
> @@ -1,3 +1,7 @@
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> #include <errno.h>
> #include <sys/types.h>
> #include <fcntl.h>
> @@ -5,7 +9,6 @@
> #include <sys/sysmacros.h>
> #include <unistd.h>
>
> -#include "config.h"
> #include "xstat.h"
>
> #ifdef HAVE_FSTATAT
> diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
> index e811703d..f5f9b10b 100644
> --- a/utils/blkmapd/device-discovery.c
> +++ b/utils/blkmapd/device-discovery.c
> @@ -26,6 +26,10 @@
> * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif /* HAVE_CONFIG_H */
> +
> #include <sys/sysmacros.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> @@ -51,10 +55,6 @@
> #include <errno.h>
> #include <libdevmapper.h>
>
> -#ifdef HAVE_CONFIG_H
> -#include "config.h"
> -#endif /* HAVE_CONFIG_H */
> -
> #include "device-discovery.h"
> #include "xcommon.h"
> #include "nfslib.h"
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index 62e37b8a..267acea5 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -34,6 +34,10 @@
> * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif /* HAVE_CONFIG_H */
> +
> #include <sys/types.h>
> #include <sys/time.h>
> #include <sys/inotify.h>
> @@ -62,10 +66,6 @@
> #include <libgen.h>
> #include <nfsidmap.h>
>
> -#ifdef HAVE_CONFIG_H
> -#include "config.h"
> -#endif /* HAVE_CONFIG_H */
> -
> #include "xlog.h"
> #include "conffile.h"
> #include "queue.h"
>
On 7/26/19 10:44 AM, Zoltan Karcagi wrote:
> At least on Arch linux ARM, the definition of struct stat in stat.h depends
> on __USE_FILE_OFFSET64. This symbol comes from config.h when defined,
> therefore config.h must always be included before stat.h. Fix all
> occurrences where the order is wrong by moving config.h to the top.
>
> This fixes the client side error "Stale file handle" when mounting from
> a server running Arch Linux ARM.
>
> Signed-off-by: Zoltan Karcagi <[email protected]>
Committed!
steved.
> ---
> support/misc/nfsd_path.c | 5 ++++-
> support/misc/xstat.c | 5 ++++-
> utils/blkmapd/device-discovery.c | 8 ++++----
> utils/idmapd/idmapd.c | 8 ++++----
> 4 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c
> index 84e48028..f078a668 100644
> --- a/support/misc/nfsd_path.c
> +++ b/support/misc/nfsd_path.c
> @@ -1,3 +1,7 @@
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> #include <errno.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> @@ -5,7 +9,6 @@
> #include <stdlib.h>
> #include <unistd.h>
>
> -#include "config.h"
> #include "conffile.h"
> #include "xmalloc.h"
> #include "xlog.h"
> diff --git a/support/misc/xstat.c b/support/misc/xstat.c
> index fa047880..4c997eea 100644
> --- a/support/misc/xstat.c
> +++ b/support/misc/xstat.c
> @@ -1,3 +1,7 @@
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> #include <errno.h>
> #include <sys/types.h>
> #include <fcntl.h>
> @@ -5,7 +9,6 @@
> #include <sys/sysmacros.h>
> #include <unistd.h>
>
> -#include "config.h"
> #include "xstat.h"
>
> #ifdef HAVE_FSTATAT
> diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
> index e811703d..f5f9b10b 100644
> --- a/utils/blkmapd/device-discovery.c
> +++ b/utils/blkmapd/device-discovery.c
> @@ -26,6 +26,10 @@
> * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif /* HAVE_CONFIG_H */
> +
> #include <sys/sysmacros.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> @@ -51,10 +55,6 @@
> #include <errno.h>
> #include <libdevmapper.h>
>
> -#ifdef HAVE_CONFIG_H
> -#include "config.h"
> -#endif /* HAVE_CONFIG_H */
> -
> #include "device-discovery.h"
> #include "xcommon.h"
> #include "nfslib.h"
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index 62e37b8a..267acea5 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -34,6 +34,10 @@
> * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif /* HAVE_CONFIG_H */
> +
> #include <sys/types.h>
> #include <sys/time.h>
> #include <sys/inotify.h>
> @@ -62,10 +66,6 @@
> #include <libgen.h>
> #include <nfsidmap.h>
>
> -#ifdef HAVE_CONFIG_H
> -#include "config.h"
> -#endif /* HAVE_CONFIG_H */
> -
> #include "xlog.h"
> #include "conffile.h"
> #include "queue.h"
>