[Buildroot] [PATCH 0/3] package/pppd: fix pppd startup with upstream patches

[Buildroot] [PATCH 0/3] package/pppd: fix pppd startup with upstream patches

[Alexis Lothoré via buildroot]

Hello,
pppd fails to start on a systems with buildroot 2024.02.x because of
missing pppd directory in /var/run. There are some logs hinting at this
issue:
Warning: couldn't open ppp database /var/run/pppd/pppd2.tdb
Can't create lock file /var/run/pppd/lock/LCK..ppp-tty-fifo1: No such file or directory
Can't create lock file /var/run/pppd/lock/LCK..ppp-tty-fifo1: No such file or directory
Can't create lock file /var/run/pppd/lock/LCK..ppp-tty-fifo1: No such file or directory
Can't create lock file /var/run/pppd/lock/LCK..ppp-tty-fifo1: No such file or directory
Can't create lock file /var/run/pppd/lock/LCK..ppp-tty-fifo1: No such file or directory
Can't create lock file /var/run/pppd/lock/LCK..ppp-tty-fifo1: No such file or directory
Can't create lock file /var/run/pppd/lock/LCK..ppp-tty-fifo1: No such file or directory

The issue has already been detected and fixed upstream (see [1]) and is
expected to be released on a v2.5.1, but this release seems to be stalled
for now (see [2]). A mere bump of the package is then not currently
possible, so I am proposing a pick of the 3 relevant patches to fix this
startup issue, both for master and 2024.02.x

Tested on ARM Cortex A9

[1] https://github.com/ppp-project/ppp/issues/419
[2] https://github.com/ppp-project/ppp/issues/460

Alexis Lothoré (3):
  package/pppd: revert lock path
  package/pppd: add mkdir recursive
  package/pppd: create rundir when it is missing

 ...005-Revert-lock-path-to-var-lock-435.patch |  64 ++++
 .../pppd/0006-utils-add-mkdir_recursive.patch | 292 ++++++++++++++++++
 ...0007-try-to-create-rundir-if-missing.patch |  50 +++
 3 files changed, 406 insertions(+)
 create mode 100644 package/pppd/0005-Revert-lock-path-to-var-lock-435.patch
 create mode 100644 package/pppd/0006-utils-add-mkdir_recursive.patch
 create mode 100644 package/pppd/0007-try-to-create-rundir-if-missing.patch

-- 
2.43.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/3] package/pppd: revert lock path

[Alexis Lothoré via buildroot]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

pppd fails to start on buildroot 2024.02, because of non-existing directory
pppd in /var/run

Upstream: https://github.com/ppp-project/ppp/commit/99cbf5e269994482edaf64624be8b1c806f9587c
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
Backport to: 2024.02.x
---
 ...005-Revert-lock-path-to-var-lock-435.patch | 64 +++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 package/pppd/0005-Revert-lock-path-to-var-lock-435.patch

diff --git a/package/pppd/0005-Revert-lock-path-to-var-lock-435.patch b/package/pppd/0005-Revert-lock-path-to-var-lock-435.patch
new file mode 100644
index 000000000000..bbff884f2113
--- /dev/null
+++ b/package/pppd/0005-Revert-lock-path-to-var-lock-435.patch
@@ -0,0 +1,64 @@
+From 99cbf5e269994482edaf64624be8b1c806f9587c Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Alexis=20Lothor=C3=A9?= <alexis.lothore@bootlin.com>
+Date: Tue, 10 Oct 2023 10:05:50 +0900
+Subject: [PATCH 1/3] Revert lock path to /var/lock (#435)
+
+From: Dominique Martinet <asmadeus@codewreck.org>
+
+lock dir changed on linux from /var/lock to /run/pppd/lock with
+pppd-2.5.0, which makes pppd fail to start if the distribution does not
+pre-create the directory.
+
+This reverts it back to /var/lock.
+
+The paths for other OS should be identical as LOCALSTATEDIR should be
+/var, but also revert them back as well just in case.
+Since the variable is no longer used remove it from makefiles.
+
+Fixes: 66a8c74c3f73 ("Let ./configure control the paths for pppd")
+Fixes: #419
+
+Upstream: https://github.com/ppp-project/ppp/commit/99cbf5e269994482edaf64624be8b1c806f9587c
+Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
+Co-authored-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
+---
+ pppd/Makefile.am | 2 +-
+ pppd/pathnames.h | 6 +++---
+ 2 files changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/pppd/Makefile.am b/pppd/Makefile.am
+index e5bedf26af96..7cb30053322e 100644
+--- a/pppd/Makefile.am
++++ b/pppd/Makefile.am
+@@ -83,7 +83,7 @@ pppd_SOURCES = \
+     upap.c \
+     utils.c
+ 
+-pppd_CPPFLAGS = -DSYSCONFDIR=\"${sysconfdir}\" -DLOCALSTATEDIR=\"${localstatedir}\" -DPPPD_RUNTIME_DIR='"@PPPD_RUNTIME_DIR@"' -DPPPD_LOGFILE_DIR='"@PPPD_LOGFILE_DIR@"'
++pppd_CPPFLAGS = -DSYSCONFDIR=\"${sysconfdir}\" -DPPPD_RUNTIME_DIR='"@PPPD_RUNTIME_DIR@"' -DPPPD_LOGFILE_DIR='"@PPPD_LOGFILE_DIR@"'
+ pppd_LDFLAGS =
+ pppd_LIBS =
+ 
+diff --git a/pppd/pathnames.h b/pppd/pathnames.h
+index de2fb6889005..12609a969429 100644
+--- a/pppd/pathnames.h
++++ b/pppd/pathnames.h
+@@ -120,12 +120,12 @@
+ #define PPP_PATH_PPPDB          PPP_PATH_VARRUN  "/pppd2.tdb"
+ 
+ #ifdef __linux__
+-#define PPP_PATH_LOCKDIR        PPP_PATH_VARRUN  "/lock"
++#define PPP_PATH_LOCKDIR        "/var/lock"
+ #else
+ #ifdef SVR4
+-#define PPP_PATH_LOCKDIR        LOCALSTATEDIR "/spool/locks"
++#define PPP_PATH_LOCKDIR        "/var/spool/locks"
+ #else
+-#define PPP_PATH_LOCKDIR        LOCALSTATEDIR "/spool/lock"
++#define PPP_PATH_LOCKDIR        "/var/spool/lock"
+ #endif
+ #endif
+ 
+-- 
+2.43.1
+
-- 
2.43.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/3] package/pppd: add mkdir recursive

[Alexis Lothoré via buildroot]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

pppd fails to start on buildroot 2024.02, because of non-existing directory
pppd in /var/run. This intermediate patch is needed to bring the second
part of the upstream fix

Upstream: https://github.com/ppp-project/ppp/commit/b0e7307b3569a5dad0f2606d2736cc8317851598
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
Backport to: 2024.02.x
---
 .../pppd/0006-utils-add-mkdir_recursive.patch | 292 ++++++++++++++++++
 1 file changed, 292 insertions(+)
 create mode 100644 package/pppd/0006-utils-add-mkdir_recursive.patch

diff --git a/package/pppd/0006-utils-add-mkdir_recursive.patch b/package/pppd/0006-utils-add-mkdir_recursive.patch
new file mode 100644
index 000000000000..293d913d8301
--- /dev/null
+++ b/package/pppd/0006-utils-add-mkdir_recursive.patch
@@ -0,0 +1,292 @@
+From b0e7307b3569a5dad0f2606d2736cc8317851598 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Alexis=20Lothor=C3=A9?= <alexis.lothore@bootlin.com>
+Date: Wed, 30 Aug 2023 11:46:01 +0900
+Subject: [PATCH 2/3] utils: add mkdir_recursive
+
+From: Dominique Martinet <dominique.martinet@atmark-techno.com>
+
+This will be used in the next commit.
+
+A test file for utils has also been added to check mkdir works as
+intended.
+
+Upstream: https://github.com/ppp-project/ppp/commit/b0e7307b3569a5dad0f2606d2736cc8317851598
+Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
+---
+ pppd/Makefile.am    |   6 ++
+ pppd/pppd-private.h |   1 +
+ pppd/utils.c        |  82 ++++++++++++++++++++++++++
+ pppd/utils_utest.c  | 139 ++++++++++++++++++++++++++++++++++++++++++++
+ 4 files changed, 228 insertions(+)
+ create mode 100644 pppd/utils_utest.c
+
+diff --git a/pppd/Makefile.am b/pppd/Makefile.am
+index 7cb30053322e..c5fe10776ede 100644
+--- a/pppd/Makefile.am
++++ b/pppd/Makefile.am
+@@ -20,6 +20,12 @@ utest_pppcrypt_LDFLAGS =
+ 
+ check_PROGRAMS += utest_crypto
+ 
++utest_utils_SOURCES = utils.c utils_utest.c
++utest_utils_CPPFLAGS = -DUNIT_TEST
++utest_utils_LDFLAGS =
++
++check_PROGRAMS += utest_utils
++
+ if WITH_SRP
+ sbin_PROGRAMS += srp-entry
+ dist_man8_MANS += srp-entry.8
+diff --git a/pppd/pppd-private.h b/pppd/pppd-private.h
+index 2883e4622acb..46ce0c8bdceb 100644
+--- a/pppd/pppd-private.h
++++ b/pppd/pppd-private.h
+@@ -437,6 +437,7 @@ int  sifproxyarp(int, u_int32_t);
+ int  cifproxyarp(int, u_int32_t);
+ 				/* Delete proxy ARP entry for peer */
+ u_int32_t GetMask(u_int32_t); /* Get appropriate netmask for address */
++int  mkdir_recursive(const char *); /* Recursively create directory */
+ int  lock(char *);	/* Create lock file for device */
+ int  relock(int);		/* Rewrite lock file with new pid */
+ void unlock(void);	/* Delete previously-created lock file */
+diff --git a/pppd/utils.c b/pppd/utils.c
+index c1bdbbbfe4e5..c47192e67fef 100644
+--- a/pppd/utils.c
++++ b/pppd/utils.c
+@@ -781,6 +781,88 @@ complete_read(int fd, void *buf, size_t count)
+ }
+ #endif
+ 
++/*
++ * mkdir_check - helper for mkdir_recursive, creates a directory
++ * but do not error on EEXIST if and only if entry is a directory
++ * The caller must check for errno == ENOENT if appropriate.
++ */
++static int
++mkdir_check(const char *path)
++{
++    struct stat statbuf;
++
++    if (mkdir(path, 0755) >= 0)
++	return 0;
++
++    if (errno == EEXIST) {
++	if (stat(path, &statbuf) < 0)
++	    /* got raced? */
++	    return -1;
++
++	if ((statbuf.st_mode & S_IFMT) == S_IFDIR)
++	    return 0;
++
++	/* already exists but not a dir, treat as failure */
++	errno = EEXIST;
++	return -1;
++    }
++
++    return -1;
++}
++
++/*
++ * mkdir_parent - helper for mkdir_recursive, modifies the string in place
++ * Assumes mkdir(path) already failed, so it first creates the parent then
++ * full path again.
++ */
++static int
++mkdir_parent(char *path)
++{
++    char *slash;
++    int rc;
++
++    slash = strrchr(path, '/');
++    if (!slash)
++	return -1;
++
++    *slash = 0;
++    if (mkdir_check(path) < 0) {
++	if (errno != ENOENT) {
++	    *slash = '/';
++	    return -1;
++	}
++	if (mkdir_parent(path) < 0) {
++	    *slash = '/';
++	    return -1;
++	}
++    }
++    *slash = '/';
++
++    return mkdir_check(path);
++}
++
++/*
++ * mkdir_recursive - recursively create directory if it didn't exist
++ */
++int
++mkdir_recursive(const char *path)
++{
++    char *copy;
++    int rc;
++
++    // optimistically try on full path first to avoid allocation
++    if (mkdir_check(path) == 0)
++	return 0;
++
++    copy = strdup(path);
++    if (!copy)
++	return -1;
++
++    rc = mkdir_parent(copy);
++    free(copy);
++    return rc;
++}
++
+ /* Procedures for locking the serial device using a lock file. */
+ static char lock_file[MAXPATHLEN];
+ 
+diff --git a/pppd/utils_utest.c b/pppd/utils_utest.c
+new file mode 100644
+index 000000000000..cdca97e6d025
+--- /dev/null
++++ b/pppd/utils_utest.c
+@@ -0,0 +1,139 @@
++#include <fcntl.h>
++#include <string.h>
++#include <sys/stat.h>
++#include <unistd.h>
++
++#include "pppd-private.h"
++
++/* globals used in test.c... */
++int debug = 1;
++int error_count;
++int unsuccess;
++
++/* check if path exists and returns its type */
++static int
++file_type(char *path)
++{
++    struct stat statbuf;
++
++    if (stat(path, &statbuf) < 0)
++	return -1;
++
++    return statbuf.st_mode & S_IFMT;
++}
++
++int
++test_simple() {
++    if (mkdir_recursive("dir"))
++	return -1;
++
++    if (file_type("dir") != S_IFDIR)
++	return -1;
++
++    rmdir("dir");
++    return 0;
++}
++
++int
++test_recurse() {
++    if (mkdir_recursive("dir/subdir/subsubdir"))
++	return -1;
++
++    if (file_type("dir/subdir/subsubdir") != S_IFDIR)
++	return -1;
++
++    rmdir("dir/subdir/subsubdir");
++
++    /* try again with partial existence */
++    if (mkdir_recursive("dir/subdir/subsubdir"))
++	return -1;
++
++    if (file_type("dir/subdir/subsubdir") != S_IFDIR)
++	return -1;
++
++    rmdir("dir/subdir/subsubdir");
++    rmdir("dir/subdir");
++    rmdir("dir");
++    return 0;
++}
++
++int
++test_recurse_multislash() {
++    if (mkdir_recursive("dir/subdir///subsubdir"))
++	return -1;
++
++    if (file_type("dir/subdir/subsubdir") != S_IFDIR)
++	return -1;
++
++    rmdir("dir/subdir/subsubdir");
++    rmdir("dir/subdir");
++
++    /* try again with partial existence */
++    if (mkdir_recursive("dir/subdir/subsubdir///"))
++	return -1;
++
++    if (file_type("dir/subdir/subsubdir") != S_IFDIR)
++	return -1;
++
++    rmdir("dir/subdir/subsubdir");
++    rmdir("dir/subdir");
++    rmdir("dir");
++    return 0;
++}
++
++int
++test_parent_notdir() {
++    int fd = open("file", O_CREAT, 0600);
++    if (fd < 0)
++	return -1;
++    close(fd);
++
++    if (mkdir_recursive("file") == 0)
++	return -1;
++    if (mkdir_recursive("file/dir") == 0)
++	return -1;
++
++    unlink("file");
++    return 0;
++}
++
++int
++main()
++{
++    char *base_dir = strdup("/tmp/ppp_utils_utest.XXXXXX");
++    int failure = 0;
++
++    if (mkdtemp(base_dir) == NULL) {
++	printf("Could not create test directory, aborting\n");
++	return 1;
++    }
++
++    if (chdir(base_dir) < 0) {
++	printf("Could not enter newly created test dir, aborting\n");
++	return 1;
++    }
++
++    if (test_simple()) {
++	printf("Could not create simple directory\n");
++	failure++;
++    }
++
++    if (test_recurse()) {
++	printf("Could not create recursive directory\n");
++	failure++;
++    }
++
++    if (test_recurse_multislash()) {
++	printf("Could not create recursive directory with multiple slashes\n");
++	failure++;
++    }
++
++    if (test_parent_notdir()) {
++	printf("Creating over a file appeared to work?\n");
++	failure++;
++    }
++
++    rmdir(base_dir);
++    free(base_dir);
++    return failure;
++}
+-- 
+2.43.1
+
-- 
2.43.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 3/3] package/pppd: create rundir when it is missing

[Alexis Lothoré via buildroot]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

pppd fails to start on buildroot 2024.02, because of non-existing directory
pppd in /var/run

Upstream: https://github.com/ppp-project/ppp/commit/3a4fec968174fd6f387d5ec872a56343f256f645
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
Backport to: 2024.02.x
---
 ...0007-try-to-create-rundir-if-missing.patch | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 package/pppd/0007-try-to-create-rundir-if-missing.patch

diff --git a/package/pppd/0007-try-to-create-rundir-if-missing.patch b/package/pppd/0007-try-to-create-rundir-if-missing.patch
new file mode 100644
index 000000000000..4c4c61ec2b33
--- /dev/null
+++ b/package/pppd/0007-try-to-create-rundir-if-missing.patch
@@ -0,0 +1,50 @@
+From 3a4fec968174fd6f387d5ec872a56343f256f645 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Alexis=20Lothor=C3=A9?= <alexis.lothore@bootlin.com>
+Date: Thu, 3 Aug 2023 16:37:27 +0900
+Subject: [PATCH 3/3] try to create rundir if missing
+
+From: Dominique Martinet <dominique.martinet@atmark-techno.com>
+
+Runtime dir changed from /run to /run/pppd in commit 66a8c74c3f73 ("Let
+./configure control the paths for pppd") and is likely to not exist on
+some distros, in which case the pppdb will not be created.
+
+See: #419 (lock directory moved in ppp-2.5.0)
+Upstream: https://github.com/ppp-project/ppp/commit/3a4fec968174fd6f387d5ec872a56343f256f645
+Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
+---
+ pppd/tdb.c | 8 ++++++++
+ 1 file changed, 8 insertions(+)
+
+diff --git a/pppd/tdb.c b/pppd/tdb.c
+index 8a563f88b9a6..62644171aa6e 100644
+--- a/pppd/tdb.c
++++ b/pppd/tdb.c
+@@ -60,8 +60,11 @@
+ #include <sys/mman.h>
+ #include <sys/stat.h>
+ #include <signal.h>
++
++#include "pppd-private.h"
+ #include "tdb.h"
+ #include "spinlock.h"
++#include "pathnames.h"
+ 
+ #define TDB_MAGIC_FOOD "TDB file\n"
+ #define TDB_VERSION (0x26011967 + 6)
+@@ -1728,7 +1731,12 @@ TDB_CONTEXT *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
+ 		goto internal;
+ 	}
+ 
++again:
+ 	if ((tdb->fd = open(name, open_flags, mode)) == -1) {
++		if ((open_flags & O_CREAT) && errno == ENOENT &&
++			mkdir_recursive(PPP_PATH_VARRUN) == 0)
++			goto again;
++
+ 		TDB_LOG((tdb, 5, "tdb_open_ex: could not open file %s: %s\n",
+ 			 name, strerror(errno)));
+ 		goto fail;	/* errno set by open(2) */
+-- 
+2.43.1
+
-- 
2.43.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 0/3] package/pppd: fix pppd startup with upstream patches

[Thomas Petazzoni via buildroot]

Hello Alexis,

On Fri, 29 Mar 2024 15:31:35 +0100
Alexis Lothoré via buildroot <buildroot@buildroot.org> wrote:

> The issue has already been detected and fixed upstream (see [1]) and is
> expected to be released on a v2.5.1, but this release seems to be stalled
> for now (see [2]). A mere bump of the package is then not currently
> possible, so I am proposing a pick of the 3 relevant patches to fix this
> startup issue, both for master and 2024.02.x

Thanks for this set of patches. However, the way you formatted the
patches is not quite what we expect.

We expect you to:

 (1) git cherry-pick the commits

 (2) Add an Upstream: tag at the end of the commit message, following
     by your Signed-off-by line

 (3) Use "git format-patch -N" to format the 3 patches

Right now you have more or less "wrapped" the patches into commits you
are the author of, and your Signed-off-by line is missing.

Look at package/waylandpp/0001-add-missing-cstdint-include.patch for an
example of a properly formatted patch. Sergei Trofimovich is the
original author, Bernd Kuhls is the person who brought the patch into
Buildroot.

Could you rework your patch series accordingly?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 0/3] package/pppd: fix pppd startup with upstream patches

[Alexis Lothoré via buildroot]

Hello Thomas,

On 4/10/24 23:20, Thomas Petazzoni wrote:
> Thanks for this set of patches. However, the way you formatted the
> patches is not quite what we expect.
> 
> We expect you to:
> 
>  (1) git cherry-pick the commits
> 
>  (2) Add an Upstream: tag at the end of the commit message, following
>      by your Signed-off-by line
> 
>  (3) Use "git format-patch -N" to format the 3 patches
> 
> Right now you have more or less "wrapped" the patches into commits you
> are the author of, and your Signed-off-by line is missing.
> 
> Look at package/waylandpp/0001-add-missing-cstdint-include.patch for an
> example of a properly formatted patch. Sergei Trofimovich is the
> original author, Bernd Kuhls is the person who brought the patch into
> Buildroot.

Thanks for the feedback and example, and sorry for the mess, I will update
accordingly and send a v2.

Alexis
> 
> Could you rework your patch series accordingly?
> 
> Thanks a lot!
> 
> Thomas

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot