[PATCH 0/3] Add getrandom() fallback, cleanup headers

[PATCH 0/3] Add getrandom() fallback, cleanup headers

[Petr Vorel]

Hi,

motivation to add this is to allow to compile reexport on systems with
older libc. (getrandom() wrapper is supported on glibc 2.25+ and  musl
1.1.20+, uclibc-ng does
not yet support it).

getrandom() syscall is supported Linux 3.17+ (old enough to bother with
a check).

I also wonder why getrandom() syscall does not called with GRND_NONBLOCK
flag. Is it ok/needed to block?

Kind regards,
Petr

Petr Vorel (3):
  reexport/fsidd.c: Remove unused headers
  support/reexport.c: Remove unused headers
  support/backend_sqlite.c: Add getrandom() fallback

 Makefile.am                       |  1 +
 aclocal/getrandom.m4              | 16 ++++++++++++++++
 configure.ac                      |  3 +++
 support/reexport/backend_sqlite.c | 18 +++++++++++++++++-
 support/reexport/fsidd.c          | 10 ----------
 support/reexport/reexport.c       |  7 -------
 6 files changed, 37 insertions(+), 18 deletions(-)
 create mode 100644 aclocal/getrandom.m4

-- 
2.42.0

[PATCH 1/3] reexport/fsidd.c: Remove unused headers

[Petr Vorel]

Some of them are needed but included elsewhere, e.g. <string.h>
included in xcommon.h, but at least <sys/random.h> is removed due
further code simplification.

Fixes: 6fd2732d ("export: Add fsidd")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 support/reexport/fsidd.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/support/reexport/fsidd.c b/support/reexport/fsidd.c
index d4b245e8..3e62b3fc 100644
--- a/support/reexport/fsidd.c
+++ b/support/reexport/fsidd.c
@@ -7,16 +7,6 @@
 #include <dlfcn.h>
 #endif
 #include <event2/event.h>
-#include <limits.h>
-#include <stdint.h>
-#include <stdio.h>
-#include <sys/random.h>
-#include <sys/socket.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <sys/un.h>
-#include <sys/vfs.h>
-#include <unistd.h>
 
 #include "conffile.h"
 #include "reexport_backend.h"
-- 
2.42.0

[PATCH 2/3] support/reexport.c: Remove unused headers

[Petr Vorel]

Some of them are needed but included elsewhere, e.g. <sys/socket.h>
included in nfslib.h or <string.h> included in xcommon.h, but at least
<sys/random.h> is removed due further code simplification.

Fixes: 878674b3 ("Add reexport helper library")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 support/reexport/reexport.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/support/reexport/reexport.c b/support/reexport/reexport.c
index d9a700af..78516586 100644
--- a/support/reexport/reexport.c
+++ b/support/reexport/reexport.c
@@ -5,16 +5,9 @@
 #ifdef HAVE_DLFCN_H
 #include <dlfcn.h>
 #endif
-#include <stdint.h>
-#include <stdio.h>
-#include <sys/random.h>
-#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
-#include <unistd.h>
 #include <errno.h>
-#include <sys/socket.h>
-#include <sys/un.h>
 
 #include "nfsd_path.h"
 #include "conffile.h"
-- 
2.42.0

Re: [PATCH 2/3] support/reexport.c: Remove unused headers

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Petr Vorel" <pvorel@suse.cz>
> An: "linux-nfs" <linux-nfs@vger.kernel.org>
> CC: "Petr Vorel" <pvorel@suse.cz>, "richard" <richard@nod.at>, "Steve Dickson" <steved@redhat.com>
> Gesendet: Mittwoch, 25. Oktober 2023 21:47:00
> Betreff: [PATCH 2/3] support/reexport.c: Remove unused headers

> Some of them are needed but included elsewhere, e.g. <sys/socket.h>
> included in nfslib.h or <string.h> included in xcommon.h, but at least
> <sys/random.h> is removed due further code simplification.
> 
> Fixes: 878674b3 ("Add reexport helper library")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

[PATCH 3/3] support/backend_sqlite.c: Add getrandom() fallback

[Petr Vorel]

Allow to compile reexport on systems with older libc. (getrandom()
wrapper is supported on glibc 2.25+ and  musl 1.1.20+, uclibc-ng does
not yet support it).

getrandom() syscall is supported Linux 3.17+ (old enough to bother with
a check).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 Makefile.am                       |  1 +
 aclocal/getrandom.m4              | 16 ++++++++++++++++
 configure.ac                      |  3 +++
 support/reexport/backend_sqlite.c | 18 +++++++++++++++++-
 4 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 aclocal/getrandom.m4

diff --git a/Makefile.am b/Makefile.am
index 00220842..72ad4ba7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -10,6 +10,7 @@ EXTRA_DIST = \
 	autogen.sh \
 	\
 	aclocal/bsdsignals.m4 \
+	aclocal/getrandom.m4 \
 	aclocal/nfs-utils.m4 \
 	aclocal/kerberos5.m4 \
 	aclocal/tcp-wrappers.m4 \
diff --git a/aclocal/getrandom.m4 b/aclocal/getrandom.m4
new file mode 100644
index 00000000..bc0fe16a
--- /dev/null
+++ b/aclocal/getrandom.m4
@@ -0,0 +1,16 @@
+dnl Checks for getrandom support (glibc 2.25+, musl 1.1.20+)
+dnl
+AC_DEFUN([AC_GETRANDOM], [
+    AC_MSG_CHECKING(for getrandom())
+    AC_LINK_IFELSE(
+		[AC_LANG_PROGRAM([[
+		   #include <stdlib.h>  /* for NULL */
+		   #include <sys/random.h>
+		]],
+		[[ return getrandom(NULL, 0U, 0U); ]] )],
+		[AC_DEFINE([HAVE_GETRANDOM], [1], [Define to 1 if you have the `getrandom' function.])
+		AC_MSG_RESULT([yes])],
+		[AC_MSG_RESULT([no])])
+
+	AC_SUBST(HAVE_GETRANDOM)
+])
diff --git a/configure.ac b/configure.ac
index 6fbcb974..4bff679d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -277,6 +277,9 @@ AC_TCP_WRAPPERS
 # Arrange for large-file support
 AC_SYS_LARGEFILE
 
+dnl Check for getrandom() libc support
+AC_GETRANDOM
+
 AC_CONFIG_SRCDIR([support/include/config.h.in])
 AC_CONFIG_HEADERS([support/include/config.h])
 
diff --git a/support/reexport/backend_sqlite.c b/support/reexport/backend_sqlite.c
index 132f30c4..0eb5ea37 100644
--- a/support/reexport/backend_sqlite.c
+++ b/support/reexport/backend_sqlite.c
@@ -7,9 +7,16 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/random.h>
 #include <unistd.h>
 
+#ifdef HAVE_GETRANDOM
+# include <sys/random.h>
+# if !defined(SYS_getrandom) && defined(__NR_getrandom)
+   /* usable kernel-headers, but old glibc-headers */
+#  define SYS_getrandom __NR_getrandom
+# endif
+#endif
+
 #include "conffile.h"
 #include "reexport_backend.h"
 #include "xlog.h"
@@ -20,6 +27,15 @@
 static sqlite3 *db;
 static int init_done;
 
+#if !defined(HAVE_GETRANDOM) && defined(SYS_getrandom)
+/* libc without function, but we have syscall */
+static int getrandom(void *buf, size_t buflen, unsigned int flags)
+{
+	return (syscall(SYS_getrandom, buf, buflen, flags));
+}
+# define HAVE_GETRANDOM
+#endif
+
 static int prng_init(void)
 {
 	int seed;
-- 
2.42.0

Re: [PATCH 0/3] Add getrandom() fallback, cleanup headers

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Petr Vorel" <pvorel@suse.cz>
> I also wonder why getrandom() syscall does not called with GRND_NONBLOCK
> flag. Is it ok/needed to block?

With GRND_NONBLOCK it would return EAGAIN if not enough
randomness is ready. How to handle this then? Aborting the start of the daemon?

Before we other think the whole thing, the sole purpose of the getrandom()
call is seeding libc's PRNG with srand() to give every waiter a different
amount of sleep time upon concurrent database access.
See wait_for_dbaccess() and handling of SQLITE_LOCKED.

I'm pretty sure instead of seeding from getrandom() we can also use the current
time or read a few bytes from /dev/urandom.
Just make sure that every user of sqlite_plug_init() has a different seed.

Thanks,
//richard

Re: [PATCH 0/3] Add getrandom() fallback, cleanup headers

[Petr Vorel]

Hi Richard,

> ----- Ursprüngliche Mail -----
> > Von: "Petr Vorel" <pvorel@suse.cz>
> > I also wonder why getrandom() syscall does not called with GRND_NONBLOCK
> > flag. Is it ok/needed to block?

> With GRND_NONBLOCK it would return EAGAIN if not enough
> randomness is ready. How to handle this then? Aborting the start of the daemon?

Well, current code uses /dev/urandom and blocks until pool is ready (man
random(7)), which is probably OK (on VM people may need to use haveged to avoid
blocking, but that's known). But even with blocking mode blocking requests of
any size can be interrupted by a signal handler with errno EINTR. That's
probably the reason why people write more robust code. I'm not sure if it's
really needed to be handled in our case.

Nice example is ul_random_get_bytes() in util-linux [1]:

#ifdef HAVE_GETRANDOM
	while (n > 0) {
		int x;

		errno = 0;
		x = getrandom(cp, n, GRND_NONBLOCK);
		if (x > 0) {			/* success */
		       n -= x;
		       cp += x;
		       lose_counter = 0;
		       errno = 0;
		} else if (errno == ENOSYS) {	/* kernel without getrandom() */
			break;

		} else if (errno == EAGAIN && lose_counter < UL_RAND_READ_ATTEMPTS) {
			xusleep(UL_RAND_READ_DELAY);	/* no entropy, wait and try again */
			lose_counter++;
		} else
			break;
	}

	if (errno == ENOSYS)
#endif

1) sleep on EAGAIN and try again (needed to be handled due GRND_NONBLOCK).

2) It also handles ENOSYS (run on kernel without getrandom() although it was built
with libc support), which would be very rare (IMHO getrandom() is on all
architectures, but looking into drivers/char/random.c, it would be on kernels
without CONFIG_SYSCTL).  Then the code also adds fallback to read
/dev/{u,}random in this case. It could be added to nfs-utils, if anybody really
needs it.

> Before we other think the whole thing, the sole purpose of the getrandom()
> call is seeding libc's PRNG with srand() to give every waiter a different
> amount of sleep time upon concurrent database access.
> See wait_for_dbaccess() and handling of SQLITE_LOCKED.

> I'm pretty sure instead of seeding from getrandom() we can also use the current
> time or read a few bytes from /dev/urandom.

Sure. Current time would work everywhere, but I guess getrandom() with syscall
is good enough. Systems which have /dev/urandom also have getrandom() syscall
(thus will work with my current proposal).

> Just make sure that every user of sqlite_plug_init() has a different seed.

Thanks for info.

Kind regards,
Petr

> Thanks,
> //richard

[1] https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/lib/randutils.c

Re: [PATCH 0/3] Add getrandom() fallback, cleanup headers

[Petr Vorel]

Hi all,

> 1) sleep on EAGAIN and try again (needed to be handled due GRND_NONBLOCK).

(man getrandom(2))

Kind regards,
Petr

Re: [PATCH 0/3] Add getrandom() fallback, cleanup headers

[Steve Dickson]

On 10/25/23 3:46 PM, Petr Vorel wrote:
> Hi,
> 
> motivation to add this is to allow to compile reexport on systems with
> older libc. (getrandom() wrapper is supported on glibc 2.25+ and  musl
> 1.1.20+, uclibc-ng does
> not yet support it).
> 
> getrandom() syscall is supported Linux 3.17+ (old enough to bother with
> a check).
> 
> I also wonder why getrandom() syscall does not called with GRND_NONBLOCK
> flag. Is it ok/needed to block?
> 
> Kind regards,
> Petr
> 
> Petr Vorel (3):
>    reexport/fsidd.c: Remove unused headers
>    support/reexport.c: Remove unused headers
>    support/backend_sqlite.c: Add getrandom() fallback
> 
>   Makefile.am                       |  1 +
>   aclocal/getrandom.m4              | 16 ++++++++++++++++
>   configure.ac                      |  3 +++
>   support/reexport/backend_sqlite.c | 18 +++++++++++++++++-
>   support/reexport/fsidd.c          | 10 ----------
>   support/reexport/reexport.c       |  7 -------
>   6 files changed, 37 insertions(+), 18 deletions(-)
>   create mode 100644 aclocal/getrandom.m4
> 
Committed... (tag: nfs-utils-2-6-4-rc6)

steved.