Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lock directory moved in ppp-2.5.0 #419

Closed
floppym opened this issue Apr 29, 2023 · 16 comments · Fixed by #435
Closed

lock directory moved in ppp-2.5.0 #419

floppym opened this issue Apr 29, 2023 · 16 comments · Fixed by #435

Comments

@floppym
Copy link
Contributor

floppym commented Apr 29, 2023

With ppp-2.4.9, serial device lock files were always created in /var/lock.

Since 66a8c74, the lock files get created in PPP_PATH_VARRUN "/lock", which evaluates to a value like /var/run/pppd/lock or /run/pppd/lock depending on the value of runstatedir passed to configure.

Is this change of location intentional? There are other programs that use serial ports that expect the lock files to be in /var/lock.

cc @enaess

@paulusmack
Copy link
Collaborator

It wasn't intentional. I suppose some people didn't realize that the lock file is not intended to be private to pppd and other people weren't sufficiently vigilant.

What do distros do these days about coordinating serial port access between different programs? Do modern distros even know what serial ports are? :)

@floppym
Copy link
Contributor Author

floppym commented May 1, 2023

On Gentoo at least, /var/lock is a symlink to /run/lock, for compatibility with "legacy" applications.

I would guess this is the case on most systemd-based distros. When sysv compatibility is enabled, systemd installs a tmpfiles drop-in that creates the /run/lock directory and the /var/lock symlink.

https://github.com/systemd/systemd/blob/v253/tmpfiles.d/legacy.conf.in#L13-L14

A couple of alternatives on Linux:

  1. Use ioctl(2) with TIOCEXCL to enable exclusive mode on the tty device, and TIOCGEXCL to respect it as root.
  2. Use advisory locks on the tty device via flock(2).

@enaess
Copy link
Contributor

enaess commented May 1, 2023

Q: Looks like the default value for runtime directory was set to /var/run/pppd, this seems to only manage where pppd2.tdb and the lock directory. Would you be able to specify --with-runtime-dir=/var/run or just /run as the value achieve the same functionality.

You are probably right that there something buggy in this code. The lock directory should be the same (/var/lock) but not imply the pppd run directory. In this case, only pppd2.tdb would be in this directory (/run/pppd/pppd2.tdb).

@floppym
Copy link
Contributor Author

floppym commented May 1, 2023

I think it makes more sense to keep runtime-dir set to ${runstatedir}/pppd. That path is also used for PID files in main.c.

I am currently working around the lock directory issue with this tmpfiles.d drop-in which creates a symlink from /run/pppd/lock to /run/lock.

https://gitweb.gentoo.org/repo/gentoo.git/commit/net-dialup/ppp/files/pppd.tmpfiles?id=8f6433c8eb43faef9a1b87de9534c54c0bc4c07f

@enaess
Copy link
Contributor

enaess commented May 1, 2023

Okay. Still sound like something we need to fix in 2.5.1, right?

Just a heads up for debian @rfc1036

@floppym
Copy link
Contributor Author

floppym commented May 1, 2023

Still sound like something we need to fix in 2.5.1, right?

Agreed.

@rfc1036
Copy link
Contributor

rfc1036 commented May 1, 2023

Indeed, this is obviously wrong and I have reverted the change for the 2.5.0 packaging.

PPP_PATH_LOCKDIR on Linux must be /var/lock/ (which I think is still the specified ABI, even if everybody that matters now makes it a symlink to /run/lock/), no configuration needed.

(Edited: lock not locks!)

@floppym
Copy link
Contributor Author

floppym commented May 1, 2023

I assume the "s" in "/var/locks" is a typo?

@floppym
Copy link
Contributor Author

floppym commented May 1, 2023

@enaess
Copy link
Contributor

enaess commented May 1, 2023

I find it interesting that distros wants /run/locks, while the FHS states it is /var/lock. In many cases, the distro symlinks /var to /run. Do we know if FHS is out of date with the current direction the community is taking and that /run/lock is the new de-facto standard?

While it could be as simple as to set PPP_PATH_LOCKDIR to LOCALSTATEDIR /lock and let ./configure set it appropriately to /var or /run. I like to not have to expose another configure parameter for it.

@rfc1036
Copy link
Contributor

rfc1036 commented May 1, 2023

The FHS is dead and buried and log superseded by the events, but I suspect that everybody formally is still using /var/lock/ and nobody is fretting to remove the symlink. It does not really matter, distributions will adjust it if needed.

@enaess
Copy link
Contributor

enaess commented May 1, 2023

The FHS is dead and buried and long superseded by the events

Uh oh? is there a different document that supersedes it, or is my gray hairs starting to show?

@floppym
Copy link
Contributor Author

floppym commented May 1, 2023

To be clear, it's /run/lock and /var/lock, NOT /run/locks and /var/locks.

@enaess
Copy link
Contributor

enaess commented May 1, 2023

Dully noted!

@martinetd
Copy link
Contributor

FWIW one issue that's been popping up with this is that pppd does not attempt to create the directories if it's missing:

  • for the rundir, the failure is just a waring and pppd continues happily: Warning: couldn't open ppp database /run/ppp/pppd2.tdb
  • for lock though pppd fails to start (which is probably the right thing to do): Can't create lock file /run/ppp/lock/LCK..ttyUSB3: No such file or directory

That failure bit me in alpine where tmpfiles don't really exist (https://gitlab.alpinelinux.org/alpine/aports/-/issues/15145 - the directory is now created for the pppd service, but not when starting dynamically through e.g. NetworkManager), and it also apparently came up in openwrt given openwrt/openwrt#12848 refers to this.

Moving back to /var/lock can be supposed to be preexisting (although I guess that can be made configurable as well if some distro thinks /var/lock is obsolete), but would it make sense to check the immediate run/lock directory and try to create if if it was missing? Something as simple as if (access(FOO_DIR, F_OK)) mkdir(FOO_DIR) would likely work.

Happy to submit a patch for both (mkdir and changing path) if that'd help

martinetd added a commit to martinetd/ppp that referenced this issue Aug 3, 2023
lock dir changed on linux from /var/log to /run/pppd/lock with
pppd-2.5.0, which makes pppd fail to start if the distribution does not
pre-create the directory.

In the line of configurability, also make the lock dir configurable
through ./configure --with-lock-dir for distributions that prefer
/run/pppd/lock, but change the default for linux back to /var/lock

This commit is bigger than it should be, because the current configure
mechanism for PPPD_RUNTIME_DIR and others is inconditional: pathnames.h
has #else cases for when the variable is not defined but that is not
possible.
This does not work for lock dir, because on non-linux platform the path
is different, and reproducing this logic at configure time is not
trivial.

Instead of the current method, only set the the PPPD_X_DIR variables in
pppdconf.h if it was explicitly set in configure, and otherwise leave it
to the else case.

Note:
 - the else cases were incorrect for runtime dir and log dirs, and have
   been fixed
 - PPPD_PLUGIN_DIR has been kept as is because it is used for rpath in
   makefiles and needs to be set inconditionnaly at configure time

Fixes: 66a8c74 ("Let ./configure control the paths for pppd")
Fixes: ppp-project#419
martinetd added a commit to martinetd/ppp that referenced this issue Aug 3, 2023
lock dir changed on linux from /var/log to /run/pppd/lock with
pppd-2.5.0, which makes pppd fail to start if the distribution does not
pre-create the directory.

In the line of configurability, also make the lock dir configurable
through ./configure --with-lock-dir for distributions that prefer
/run/pppd/lock, but change the default for linux back to /var/lock

This commit is bigger than it should be, because the current configure
mechanism for PPPD_RUNTIME_DIR and others is inconditional: pathnames.h
has #else cases for when the variable is not defined but that is not
possible.
This does not work for lock dir, because on non-linux platform the path
is different, and reproducing this logic at configure time is not
trivial.

Instead of the current method, only set the the PPPD_X_DIR variables in
pppdconf.h if it was explicitly set in configure, and otherwise leave it
to the else case.

Note:
 - the else cases were incorrect for runtime dir and log dirs, and have
   been fixed
 - PPPD_PLUGIN_DIR has been kept as is because it is used for rpath in
   makefiles and needs to be set inconditionnaly at configure time

Fixes: 66a8c74 ("Let ./configure control the paths for pppd")
Fixes: ppp-project#419
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 3, 2023
PR ppp-project#435 makes lockdir default go back to /var/lock, but runtime dir
still changed from /run to /run/pppd in commit 66a8c74 ("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.

This is not a big problem but might as well just try to create the
directory if it is missing.

Return code of mkdir does not need to be checked as the following open
will fail anyway if mkdir failed.

See: ppp-project#419
Signed-off-by: Dominique Martinet <[email protected]>
@martinetd
Copy link
Contributor

I've created two separate PRs for each issue (move back to /var/lock in #435 and try to create dir if missing in #436)

martinetd added a commit to martinetd/ppp that referenced this issue Aug 3, 2023
lock dir changed on linux from /var/log to /run/pppd/lock with
pppd-2.5.0, which makes pppd fail to start if the distribution does not
pre-create the directory.

In the line of configurability, also make the lock dir configurable
through ./configure --with-lock-dir for distributions that prefer
/run/pppd/lock, but change the default for linux back to /var/lock

This commit is bigger than it should be, because the current configure
mechanism for PPPD_RUNTIME_DIR and others is inconditional: pathnames.h
has #else cases for when the variable is not defined but that is not
possible.
This does not work for lock dir, because on non-linux platform the path
is different, and reproducing this logic at configure time is not
trivial.

Instead of the current method, only set the the PPPD_X_DIR variables in
pppdconf.h if it was explicitly set in configure, and otherwise leave it
to the else case.

Note:
 - the else cases were incorrect for runtime dir and log dirs, and have
   been fixed
 - PPPD_PLUGIN_DIR has been kept as is because it is used for rpath in
   makefiles and needs to be set inconditionnaly at configure time

Fixes: 66a8c74 ("Let ./configure control the paths for pppd")
Fixes: ppp-project#419
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 7, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

Return code of mkdir does not need to be checked as the following open
will fail anyway if mkdir failed.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 30, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

Return code of mkdir does not need to be checked as the following open
will fail anyway if mkdir failed.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 30, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

Return code of mkdir does not need to be checked as the following open
will fail anyway if mkdir failed.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 30, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

Return code of mkdir does not need to be checked as the following open
will fail anyway if mkdir failed.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 30, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

Return code of mkdir does not need to be checked as the following open
will fail anyway if mkdir failed.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 30, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

Return code of mkdir does not need to be checked as the following open
will fail anyway if mkdir failed.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 31, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Aug 31, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
fpokryvk pushed a commit to NetworkManager/NetworkManager-ci that referenced this issue Sep 20, 2023
issue being worked around: ppp-project/ppp#419

OSs affected: Fedora-39, Fedora-Rawhide

Feel free to revert once not needed anymore.
fpokryvk pushed a commit to NetworkManager/NetworkManager-ci that referenced this issue Sep 20, 2023
issue being worked around: ppp-project/ppp#419

OSs affected: Fedora-39, Fedora-Rawhide

Feel free to revert once not needed anymore.
martinetd added a commit to martinetd/ppp that referenced this issue Oct 6, 2023
lock dir changed on linux from /var/log 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/run/lock.

Note the standard path for locks is /var/lock, but there is no
ready-made variable for it and we've always been using /var/run/lock,
so this picks the safer alternative of reverting to something that's
always been working.

Fixes: 66a8c74 ("Let ./configure control the paths for pppd")
Fixes: ppp-project#419
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Oct 6, 2023
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: 66a8c74 ("Let ./configure control the paths for pppd")
Fixes: ppp-project#419
Signed-off-by: Dominique Martinet <[email protected]>
paulusmack pushed a commit that referenced this issue Oct 10, 2023
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: 66a8c74 ("Let ./configure control the paths for pppd")
Fixes: #419

Signed-off-by: Dominique Martinet <[email protected]>
Co-authored-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Oct 10, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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.

Lockdir has a similar problem depending on the configuration, which is
even more important as failure to create a lock file leads to pppd not
starting.

See: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Oct 10, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Oct 10, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Oct 10, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
martinetd added a commit to martinetd/ppp that referenced this issue Oct 10, 2023
Runtime dir changed from /run to /run/pppd in commit 66a8c74 ("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: ppp-project#419 (lock directory moved in ppp-2.5.0)
Signed-off-by: Dominique Martinet <[email protected]>
arnout pushed a commit to buildroot/buildroot that referenced this issue May 9, 2024
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]). Bump on current master, which currently reflects what
will likely be the 2.5.1.

[1] ppp-project/ppp#419
[2] ppp-project/ppp#460

Signed-off-by: Alexis Lothoré <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
arnout pushed a commit to buildroot/buildroot that referenced this issue May 31, 2024
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]). Bump on current master, which currently reflects what
will likely be the 2.5.1.

[1] ppp-project/ppp#419
[2] ppp-project/ppp#460

Signed-off-by: Alexis Lothoré <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
(cherry picked from commit c1b04a3)
Signed-off-by: Peter Korsgaard <[email protected]>
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Oct 19, 2024
No longer necessary since the lock path was reverted upstream.

Bug: ppp-project/ppp#419
Signed-off-by: Mike Gilbert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants