Bug#1063474: insserv messages about loops are too obtuse
On 2024-02-08 14:12, Mark Hindley wrote: What are your thoughts? Is this something you can improve or address upstream? I agree, the suggested out from Jakob makes more sense and would be more useful. I will look at insserv's code and see how it handles this. I'm not sure if this is a quick-n-easy change or if it's going to involve digging through insserv's curious brand of arcane magic. (Hopefully for the former.) I'm also happy to accept patches that do something like what Jakob is suggesting if someone already has an idea of how they'd like to implement this change. - Jesse
Bug#1047054: sysvinit: Fails to build source after successful build
Thank you, I'll add this patch upstream for the next version. - Jesse On 2023-08-19 4:28 p.m., Mark Hindley wrote: > Control: tags -1 pending patch > > Lucas, > > Thanks for raising this. Fixed version is pending. > > Jesse, > > My patch for man/Makefile to fix the clean recipe is attached. You may want > it, > or something similar, upstream. > > Thanks. > > Mark
Bug#1033311: sysvinit-utils: pidof not always returning a pid, when using the full path to a program
> > I think I have found the offending commit: > 0b695c7e0b1cac60ed77c56f224e296f023b652e uses memset *after* realpath which > wipes > the canonical resolved path. > > I suggest this fix as a starting point. > I'd agree. I was running into some situations where a symlink wasn't recognized and this corrects the behaviour for me. Committed this patch upstream. - Jesse
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
On 2023-03-28 10:56 a.m., Markus Fischer wrote: > I did a few more tests of my own and didn't find any other issues. > > - Markus > Thank you. I'm planning to do the same and then publish an update to sysvinit.
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
On 2023-03-28 10:40 a.m., Mark Hindley wrote: > On Thu, Mar 23, 2023 at 11:25:02AM -0300, Jesse Smith wrote: >>> $ ls -l $(which vi) >>> lrwxrwxrwx 1 root root 20 Jan 11 04:16 /usr/bin/vi -> >>> /etc/alternatives/vi >>> $ ls -l /etc/alternatives/vi >>> lrwxrwxrwx 1 root root 17 Jan 11 04:16 /etc/alternatives/vi -> >>> /usr/bin/vim.tiny >>> $ ls -l /usr/bin/vim.tiny >>> -rwxr-xr-x 1 root root 1713240 Jan 11 04:16 /usr/bin/vim.tiny >> >> Okay, yes, this makes sense. The symbolic links are making multiple >> jumps so it won't work. This is expected behaviour because there is no >> executable running called /usr/bin/vi or /etc/alternatives/vi. Running >> "pidof vi.tiny" would work. Or if /usr/bin/vi was a link to >> /usr/bin/vim.tiny then "pidof $(which vi)" would work. pidof won't >> follow aliases or symbolic links with multiple hops and different names. > As Thorsten pointed out, multiple layers of symlinks is used in several places > in Debian, not least the alternatives system. > > Shouldn't src/killall.c readproc() run readlink(2) until it gets the canonical > executable path? > > Alternatively, if you really want to keep the behaviour change then pidof.8 > needs updating to reflect the new behaviour. > > I would say the manual page is already accurate. It says symbolic links to executables will return a match. This is true. It doesn't say symbolic links to symbolic links will return a match. Though perhaps this should be explicitly stated to clarify. I'll update the wording. pidof isn't going to chase down multiple layers of symbolic links. If the operator is dealing with situations like Debian's alternatives system then they should just use the real name of the executable (gcc vs cc, for example). Using a name that links to a name that links to another name is likely to bite people.
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
On 2023-03-24 6:44 a.m., Markus Fischer wrote:
> I think I've figured it out. With the following patch I don't see the
> unexpected behaviour anymore:
>
> --- a/src/killall5.c
> +++ b/src/killall5.c
> @@ -662,6 +662,7 @@ int readproc()
> /* Try to stat the executable. */
> snprintf(path, sizeof(path), "/proc/%s/exe", d->d_name);
> p->pathname = (char *)xmalloc(PATH_MAX);
> + memset(p->pathname, 0, PATH_MAX);
> if (readlink(path, p->pathname, PATH_MAX) == -1) {
> p->pathname = NULL;
> }
>
This patch looks good to me. I'm adding it upstream.
Will run some more tests before publishing 3.07. And would appreciate it
if everyone following this issue could test too and provide feedback.
- Jesse
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
On 2023-03-23 12:04 p.m., Markus Fischer wrote: > I could also reproduce it with emacs. I've used emacs-gtk to avoid the > symlink. > > ```shell 1 > $ emacs-gtk -nw -fn helvetica > ``` > > ```shell 2 > $ ./pidof emacs-gtk > 24727 > $ ./pidof $(which emacs-gtk) > $ ls -l $(which emacs-gtk) > -rwxr-xr-x 1 root root 5294300 Mar 14 21:30 /usr/bin/emacs-gtk > ``` This is interesting. Is there anything else you can share that might shed light on why this happens while I can't get the same results? You mentioned you're using two separate shells. Are they with the same user account? Is there any PID hiding feature of Debian enabled? Does it still happen if you run "pidof -z $(which emacs-gtk)"?
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
On 2023-03-23 11:36 a.m., Markus Fischer wrote: > Alright. Then there is still the issue with gdb, which is no symlink. > A full example for that: > > ```shell 1 > > $ type gdb > gdb is /usr/bin/gdb > $ gdb --core=corefile > > ``` > > ```shell 2 > > $ ./pidof gdb > 23125 > $ ./pidof $(which gdb) > $ ls -l $(which gdb) > -rwxr-xr-x 1 root root 9904496 Feb 24 22:58 /usr/bin/gdb > This one I have not been able to duplicate. Does it happen with any other programs which accept arguments or just gdb?
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
On 2023-03-23 11:20 a.m., Markus Fischer wrote: > ```shell 1 > > $ type vi > vi is /usr/bin/vi > $ vi > > ``` > > ```shell 2 > > $ cd ~/src/sysvinit-upstream/src/ > $ ls -l pidof > lrwxrwxrwx 1 ivanhoe ivanhoe 8 Mar 22 14:56 pidof -> killall5 > $ ./pidof vi > 21945 > $ ./pidof $(which vi) > $ ls -l $(which vi) > lrwxrwxrwx 1 root root 20 Jan 11 04:16 /usr/bin/vi -> > /etc/alternatives/vi > $ ls -l /etc/alternatives/vi > lrwxrwxrwx 1 root root 17 Jan 11 04:16 /etc/alternatives/vi -> > /usr/bin/vim.tiny > $ ls -l /usr/bin/vim.tiny > -rwxr-xr-x 1 root root 1713240 Jan 11 04:16 /usr/bin/vim.tiny Okay, yes, this makes sense. The symbolic links are making multiple jumps so it won't work. This is expected behaviour because there is no executable running called /usr/bin/vi or /etc/alternatives/vi. Running "pidof vi.tiny" would work. Or if /usr/bin/vi was a link to /usr/bin/vim.tiny then "pidof $(which vi)" would work. pidof won't follow aliases or symbolic links with multiple hops and different names.
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
Could you give a full example of pidof not detecting the vi process? I did all my testing as a regular user and both regular executables and symlinks are showing up in pidof process listings. With and without arguments. The only thing I can think of which would throw this off would be if the program being run was an alias. For example, if "vi" was an alias to "vim" rather than a symlink. On 2023-03-23 3:38 a.m., Markus Fischer wrote: > I can confirm Mark's observation that "pidof $(which vi)" still does > not work, at least when vi is running as normal user. However, when I > run vi as root both pidof 3.06 and 3.07 work as expected. > > Also my 2nd issue (which might have gone unnoticed because I didn't cc > anybody) is still open. I'm going to paste it here again: > >> I just could reproduce another case where pidof with the argument being >> a full path to a binary doesn't return a pid. It is not related to the >> argument being a symlink. >> >> This time it seems to be related to the program having been started with >> arguments but I don't see the unexpected behaviour with just any >> arguments, just some. >> >> For example, when having gdb running with "gdb --core=corefile" "pidof >> $(which gdb)" does not return a pid but when I start gdb with "gdb >> --quiet" "pidof $(which gdb)" behaves as expected. > > I also noticed that as with vi above, when gdb was run as root "pidof > $(which gdb)" works as expected with both 3.06 and 3.07. > > - Markus > > > Am 22.03.23 um 16:38 schrieb Jesse Smith: >> On 2023-03-22 8:31 a.m., Mark Hindley wrote: >>> Markus, >>> >>> Thanks for this. >>> >>> On Wed, Mar 22, 2023 at 08:40:20AM +0100, Markus Fischer wrote: >>>> Package: sysvinit-utils >>>> Version: 3.06-2 >>>> Severity: normal >>>> X-Debbugs-Cc: none >>>> >>>> Dear Maintainer, >>>> >>>> Passing the full path of a binary to the pidof command does not always >>>> return a pid although the process is running and the man page of the >>>> pidof command explicitly notes that it can be used that way. >>>> >>>> This might be related to the fact that all programs with which I >>>> tested >>>> this and which show this unexpected behaviour were symlinks (i.e., >>>> "which " returned a symlink). >>> Yes, I just tried with vim.basic which is not a symlink on my system >>> and both >>> >>> pidof vim.basic >>> pidof $(which vim.basic) >>> >>> work as expected. >>> >>>> However, on Debian Bullseye the >>>> behaviour is as I expected it. >>> So this appears to be a change in behaviour. I suspect this is an >>> inadvertent >>> side-effect of 0b695c7e0b1cac60ed77c56f224e296f023b652e. >>> >>> Jesse, or was it intentional? >>> >> >> I made a fix for this and have pushed it to the 3.07 branch of the SysV >> init repository. I'll publish a new version in a couple of days with >> this fix. There were some other changes to killall5 which are also in >> the queue, so this will fix a few issues. >> >> Would be great to have someone check the updated pidof and report if >> it's working okay for them too. >> >> - Jesse >>
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
On 2023-03-22 8:31 a.m., Mark Hindley wrote: > Markus, > > Thanks for this. > > On Wed, Mar 22, 2023 at 08:40:20AM +0100, Markus Fischer wrote: >> Package: sysvinit-utils >> Version: 3.06-2 >> Severity: normal >> X-Debbugs-Cc: none >> >> Dear Maintainer, >> >> Passing the full path of a binary to the pidof command does not always >> return a pid although the process is running and the man page of the >> pidof command explicitly notes that it can be used that way. >> >> This might be related to the fact that all programs with which I tested >> this and which show this unexpected behaviour were symlinks (i.e., >> "which " returned a symlink). > Yes, I just tried with vim.basic which is not a symlink on my system and both > > pidof vim.basic > pidof $(which vim.basic) > > work as expected. > >> However, on Debian Bullseye the >> behaviour is as I expected it. > So this appears to be a change in behaviour. I suspect this is an inadvertent > side-effect of 0b695c7e0b1cac60ed77c56f224e296f023b652e. > > Jesse, or was it intentional? > I made a fix for this and have pushed it to the 3.07 branch of the SysV init repository. I'll publish a new version in a couple of days with this fix. There were some other changes to killall5 which are also in the queue, so this will fix a few issues. Would be great to have someone check the updated pidof and report if it's working okay for them too. - Jesse
Bug#1033311: sysvinit-utils: pidof not always returning a pid when using the full path to a program
On 2023-03-22 8:31 a.m., Mark Hindley wrote: > Markus, > > Thanks for this. > > On Wed, Mar 22, 2023 at 08:40:20AM +0100, Markus Fischer wrote: >> Package: sysvinit-utils >> Version: 3.06-2 >> Severity: normal >> X-Debbugs-Cc: none >> >> Dear Maintainer, >> >> Passing the full path of a binary to the pidof command does not always >> return a pid although the process is running and the man page of the >> pidof command explicitly notes that it can be used that way. >> >> This might be related to the fact that all programs with which I tested >> this and which show this unexpected behaviour were symlinks (i.e., >> "which " returned a symlink). > Yes, I just tried with vim.basic which is not a symlink on my system and both > > pidof vim.basic > pidof $(which vim.basic) > > work as expected. > >> However, on Debian Bullseye the >> behaviour is as I expected it. > So this appears to be a change in behaviour. I suspect this is an inadvertent > side-effect of 0b695c7e0b1cac60ed77c56f224e296f023b652e. > > Jesse, or was it intentional? > It's a side-effect of the attempt to make pidof less prone to hanging when checking sleeping processes and zombies. I'll look into it as the symlink should probably work, or at least we should issue a warning. - Jesse
Bug#1027945: sysvinit: [INTL:pt] Portuguese translation of MANPAGE
On 2023-01-05 5:40 p.m., Américo Monteiro wrote: > A quinta-feira, 5 de janeiro de 2023 17:52:03 WET Jesse Smith escreveu: >> On 2023-01-05 12:57 p.m., Mark Hindley wrote: >>> Control: tags -1 upstream >>> >>> Américo, >>> >>> On Wed, Jan 04, 2023 at 10:16:31PM +, Américo Monteiro wrote: >>>> Package: sysvinit >>>> Version: 3.04-1 >>>> Tags: l10n, patch- >>>> Severity: wishlist >>>> >>>> Updated Portuguese translation for sysvinit's manpage >>>> Translator: Américo Monteiro >>>> Feel free to use it. >>>> >>>> This translation was already send by me in bug 1019168 >>>> that was closed but my work was not added to the package >>>> Please include my translation >>> Thanks for pointing this out and please accept my apologies. It was >>> included upstream, but then seems to have got lost again prior to the >>> current release. >>> >>> Jesse, >>> >>> I think it was added as man/pt.po and then removed again in >>> 2354f1f73566544741a6a2594411a070e0898bfb. Could you add it back as >>> man/po/pt.po again upstream? >>> >>> Thanks >>> >>> Mark >> I went back through the old commits and found the original pt.po file >> and re-added it. When that is done the translations fail to build when >> "make" is run. I've reverted the change. The translation can be added in >> again if it is patched to build properly. >> >> - Jesse > Hi > > Please try this file now > I believe it's working now > > Thank you, I have added this file into the man/po/ directory and merged it into the main branch. - Jesse
Bug#1027945: sysvinit: [INTL:pt] Portuguese translation of MANPAGE
On 2023-01-05 12:57 p.m., Mark Hindley wrote: > Control: tags -1 upstream > > Américo, > > On Wed, Jan 04, 2023 at 10:16:31PM +, Américo Monteiro wrote: >> Package: sysvinit >> Version: 3.04-1 >> Tags: l10n, patch- >> Severity: wishlist >> >> Updated Portuguese translation for sysvinit's manpage >> Translator: Américo Monteiro >> Feel free to use it. >> >> This translation was already send by me in bug 1019168 >> that was closed but my work was not added to the package >> Please include my translation > Thanks for pointing this out and please accept my apologies. It was included > upstream, but then seems to have got lost again prior to the current release. > > Jesse, > > I think it was added as man/pt.po and then removed again in > 2354f1f73566544741a6a2594411a070e0898bfb. Could you add it back as > man/po/pt.po > again upstream? > > Thanks > > Mark I went back through the old commits and found the original pt.po file and re-added it. When that is done the translations fail to build when "make" is run. I've reverted the change. The translation can be added in again if it is patched to build properly. - Jesse
Bug#1027945: sysvinit: [INTL:pt] Portuguese translation of MANPAGE
On 2023-01-05 12:57 p.m., Mark Hindley wrote: > Control: tags -1 upstream > > Américo, > > On Wed, Jan 04, 2023 at 10:16:31PM +, Américo Monteiro wrote: >> Package: sysvinit >> Version: 3.04-1 >> Tags: l10n, patch- >> Severity: wishlist >> >> Updated Portuguese translation for sysvinit's manpage >> Translator: Américo Monteiro >> Feel free to use it. >> >> This translation was already send by me in bug 1019168 >> that was closed but my work was not added to the package >> Please include my translation > Thanks for pointing this out and please accept my apologies. It was included > upstream, but then seems to have got lost again prior to the current release. > > Jesse, > > I think it was added as man/pt.po and then removed again in > 2354f1f73566544741a6a2594411a070e0898bfb. Could you add it back as > man/po/pt.po > again upstream? > I'll be happy to re-add the file if someone wants to email it to me. Every time we try to merge a translation file in GitHub it ends up getting corrupted. I'll add it manually to my source tree. Same goes for any other translation files which aren't in the latest release. - Jesse
Bug#1026911: Outdated de.po both in Debian package and in repository
Thank you, I've updated the po/de.po file in my branch and committed it. It looks like GitHub merged old copies of the files back into two locations somehow. Makes me wonder if any other translations were affected. Merry Christmas to you too! Jesse On 2022-12-23 4:18 p.m., Helge Kreutzmann wrote: > Hello Jesse, > On Fri, Dec 23, 2022 at 04:14:50PM -0400, Jesse Smith wrote: >> Please email me the file. Every time we do a git pull request or merge >> it breaks the man/ directory and it's a mess to try to revert. I'll >> commit it directly instead of merging/patching. > > Thanks for your ultra-fast response. > > Please find the file attached. (Decompress before committing). > > Merry Christmas and a Happy New Year. > >Helge >
Bug#1026911: Outdated de.po both in Debian package and in repository
Please email me the file. Every time we do a git pull request or merge it breaks the man/ directory and it's a mess to try to revert. I'll commit it directly instead of merging/patching. - Jesse On 2022-12-23 4:08 p.m., Helge Kreutzmann wrote: > Hello Jesse, > in our Debian overview I just saw that de.po is quite outdated (i.e. > an older version than the one I submitted to you). I just reported > this, see https://bugs.debian.org/1026911 > > I don't know how this happend, the fix is trivial, revert to the > latest version. You can find this attached to the bug report, but if > necessary I can send it to you directly as well. > > (I'm not keen doing this git merge stuff again, especially since it > resulted in no de.po last time and a broken one now). > > If you could prepare a fixed release soon (and hopefully the Debian > maintainer could do so as well), this would be great, as Debian is > going to freeze end of January. > > Thanks again. > > I wish you a nice holiday season, a good years turnover and a great > 2023! > > Greetings > > Helge >
Bug#997851: Update on doas package status
Thanks for the update. Having a Debian package (even an unofficial one) would be nice. I can still post install instructions for it in our README file. Is it possible to rename the existing package or add warning to it to let people know the "doas" package in Debian is actually OpenDoas and not the slicer69/doas package. I'd like to avoid confusion on that front. Jesse On 2022-05-06 8:04 p.m., Scupake wrote: > Hello, > > A little update on this issue. I found out that I am not be able to > upload both packages into the Debian repositories since the two programs are > too similar to each other. However I can provide packages for slicer69/doas on > a git repo since the packaging for it is pretty much done. > I am sorry about not being able to include slicer69/doas in the Debian > repos. >
Bug#997851: Update on doas package status
On 2022-04-17 11:49 a.m., Scupake wrote: > Hello, > > I'm very very sorry for taking an unreasonable time to reply and resolve > this issue. I will try to be more active from now on. > > I'm also currently packaging slicer69/doas, though I still haven't > decided on a name, I would like to use "doas-portable" if it is at least > mentioned in upstream's README file. > I like the name "doas-portable". I'd be happy to add that to the README file so Debian users know about it and can find it under that name. - Jesse
Bug#997851: Update on doas package status
This issue has been open for several months now without an update and I'd like to encourage its resolution. The upstream doas project is still getting issue reports [1] which are resulting from confusion in the naming between "doas" versus "OpenDoas". Ideally this package should have its name changed or point to the upstream corresponding with the package name. I'd also like to respond to the post linked to by Andrea Pappacoda in the previous comment if I may. I have two thoughts on that. One is that the bugs reported as concerns were fixed and aren't relevant anymore and were handled before this Debian bug report was filed. I thin it should be considered out of date. The other is that one of the supposed flaws reported was incorrect (due to a misunderstanding on the filer's part on how UIDs were handled by doas) and, when this was explained to the person filing the issue they went on a misinformation and harassment campaign across multiple social media platforms against the slicer69/doas port. Blog posts like the one linked to above about "slicer69-doas" were the result and based on the misunderstanding of security issues by the original person filing the bug report. I'm not saying there weren't bug issues during the porting process, just that some of them which were (in the above post's words "treated superficially") were actually dismissed for being incorrect or not what the issue reporter claimed. None of them were particular serious either as they'd require root access to exploit. This may not be entirely relevant to the decision on how to name Debian packages, but I wanted to clear the record as I feel the article linked to by Pappacoda isn't representative of the real situation in terms of doas vs OpenDoas. Both projects are good at what they do and both have had some flaws fixed. I don't think one should be considered "better" or "more of a true doas" over the other. But I do think Debian's naming approach should reflect upstream to avoid confusion. I do agree with Pappacoda that having Debian "doas" be a virtual package with "opendoas" and "doas-portable" being the underlying real packages makes sense. 1. https://github.com/slicer69/doas/pull/99
Bug#1001908: manpages-de: manpage of "shutdown" not up-to-date
> While creating the Po4a framework, I found many issues in the man > pages worth to fix. See the attachment. As a temporary playground, > I've forked the Savannah repo at Github [1], where you can also view > the patch [2] > > [1] https://github.com/mariobl/sysvinit > [2] > https://github.com/mariobl/sysvinit/commit/2bfc3618a0fa343856581ed3b39cc5dac215ace8 > > The patch for the markup in the manual pages has been applied upstream in the Savannah repository. - Jesse
Bug#1001908: manpages-de: manpage of "shutdown" not up-to-date
On 2021-12-25 3:35 p.m., Mario Blättermann wrote: > OK, it is in my favor to both keep existing man page translations > alive (acknowledging the work of the previous translators) and > integrate Po4a-based translations into upstream projects, instead of > maintaining them in an external translation project. Give me some days > to prepare a Po4a framework and migrate the existing .po files. I've > already checked out the Git repo at savannah.nongnu.org. I would > create the changes in one single (and quite big) Git diff, is this OK > for you? Or is there even a mirror at Github or Gitlab or wherever > else where I could create a pull request? > Hi Mario. I'm the upstream maintainer for sysvinit. You can e-mail me the git patch if you like and I'll test & apply it upstream. One big text file produced from "git diff" works fine for me. At this time there isn't a mirror for sysvinit. I've been thinking about it, or even migrating to a more popular platform as the Savannah infrastructure has some drawbacks. For now though the easiest way to send changes upstream is to e-mail them to me, or open a bug report on Savannah. Best regards, Jesse
Bug#1001908: manpages-de: manpage of "shutdown" not up-to-date
On 2021-12-22 1:26 p.m., Mark Hindley wrote: > Control: tags -1 patch > > Helge, > > On Wed, Dec 22, 2021 at 05:27:34PM +0100, Helge Kreutzmann wrote: >> clone 1001908 -1 >> reassign -1 sysvinit-core > Thanks for this. > >> Yes, this is the first bug, and it is in the original man page from >> sysvinit-core: >> >> SYNOPSIS >>/sbin/shutdown [-akrhPHfFnc] [-t sec] time [warning message] >> >> … >> >>-q Reduce the number of warnings shutdown displays. Usually >> shutdown displays warnings every 15 minutes and then every minute in the >> last 10 minutes of the countdown until time is reached. When -q is specified >> shutdown only >> warns at 60 minute intervals, at the 10 minute mark, at the 5 >> minue mark, and when the shutdown process actually happens. >> >>-Q Silence warnings prior to shutting down. Usually shutdown >> displays warnings every 15 minutes and then every minute in the last 10 >> minutes of the countdown until time is reached. When -Q is specified >> shutdown only warns >> when the shutdown process actually happens. All other warning >> intervals are suppressed. >> >> (Btw. also the lower case "-q" is not in the SYNOPSIS). >> >> This is the first bug, to be dealt with in sysvinit-core and once it >> has been fixed there, translations may follow. > I have queued a fix for this for the next upload. Proposed patch attached. > > Jesse, presumably you will want to include this upstream too? > > Thanks all. > Definitely. Thank you Helge and Mark for sending along this patch. It has been applied upstream and will appear in sysvinit-3.02. Jesse
Bug#997851: The doas package should be called opendoas
On 2021-11-28 10:08 a.m., Scupake wrote: > I like the idea of giving slicer69/doas a diffrent name, I still haven't > decided on a name yet so I'll work on renaming this package to "opendoas". > > Thanks a lot and sorry for the late reply. > Thanks for the update and for changing this package's name. Please let me know when you settle on a name for the slicer69/doas version of doas? - Jesse
Bug#997851: The doas package should be called opendoas
Thanks very much for getting back to me and being open to discussing the labelling for this Debian package. > I considered naming this package "opendoas" but thought that users might > not realize that that a version of doas is packaged under that name. I can see why you'd take that approach. It's easier to find. I even found it by accident when checking for the other version of doas on Debian. So you're right, it's very easy to locate the Debian doas package. :) My main concern with the current naming convention is that calling OpenDoas "doas" means upstream slicer69/doas receives bug reports intended for OpenDoas. This causes some confusion on the part of the users and means the OpenDoas project isn't always getting the feedback from users. That's something I'd like to improve upon if possible. I'm open to various ways of doing that. > I have also considered packaging Slicer69's doas but at the end decided > not to package it because it does not support the persist feature which > I think is a very important feature. Some people do desire the "persist" function, so that makes sense. Though I have some security-related concerns with the OpenDoas persist feature. Putting that aside for a moment, people who want persistent doas access can use "doas -s" on the slicer69/doas port, it's the equivalent of "sudo -i". > Maybe the best way to handle this would be to package both versions and > make it very clear that only opendoas has persistence. > I like this approach of having both versions of doas packaged. I'd definitely support this. My preference here would be to have OpenDoas packaged as "opendoas" and slicer69/doas packaged as "doas". However, I think a working alternative would be to call slicer69/doas something descriptive (rather than using the upstream account name). Something like "doas-portable" or "doas-universal" since it runs on virtually every Unix-like platform, including the BSDs, Solaris, ilumos, macOS, Linux, etc. Please let me know your thoughts on these ideas and what your plan is for the doas package(s). - Jesse
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgrade
On 2021-11-01 4:23 p.m., Mark Hindley wrote: > -- System Information: >> Debian Release: 11.0 >> APT prefers unreleased >> APT policy: (500, 'unreleased'), (500, 'buildd-unstable'), (500, >> 'unstable'), (100, 'experimental') >> Architecture: x32 (x86_64) >> Foreign Architectures: i386, amd64 > I realise this may be clutching at straws, but is there any chance the x32 > arch > is the trigger for this? > > I don't see any other discrepancies. That's an interesting idea. Given everything else we've tried I'm open to the possibility. I didn't realize until you mentioned it that Debian even supported x32 builds these days.
Bug#997851: The doas package should be called opendoas
Package: doas Version: 6.8 I believe the Debian package "doas" should be renamed to "opendoas". The upstream project Debian packages is called "OpenDoas" [1] and it is a competing port to another project called "doas" [2] that has similar functionality. However, the two tools are not entirely compatible. This is likely to confuse users (like myself) who installed the "doas" port, expecting to get upstream doas, but instead got an incompatible OpenDoas. This is made more confusing because other Linux distributions (such as Arch and Void) as well as FreeBSD also have a package called "doas" which is based on upstream doas. Some of them, at least Void and Arch, also have a separate package [4][5] called "opendoas" which corresponds to the alternative upstream. In short, Debian calling its package "doas" instead of "opendoas" is a bit misleading and likely to confuse people coming from other projects like FreeBSD [3], Arch, and Void. Comparing the two projects we see the configuration file is kept in a different place supports different flags, and has different permission restrictions. It's also possible this naming conflict is going to result in bug reports getting filed with the wrong upstream project. As an alternative solution, I'd like to suggest Debian switch its upstream source to use doas [2] instead of OpenDoas. This would have two benefits: First, it would match what other distributions and operating systems are doing, making life easier for end users who use multiple OSes. Second, it looks like doas is more actively developed than OpenDoas, based on recent commits and releases, so we'd have a more actively managed upstream. Thank you for your consideration. 1. https://github.com/Duncaen/OpenDoas 2. https://github.com/slicer69/doas 3. https://www.freshports.org/security/doas/ 4. https://voidlinux.org/packages/?arch=x86_64=opendoas 5. https://aur.archlinux.org/packages/?O=0=doas
Bug#926896: sysvinit-utils: pidof is unreliable
On 2021-10-22 6:52 p.m., Svante Signell wrote: > Hi Jesse, > > On Thu, 2021-10-21 at 14:51 -0300, Jesse Smith wrote: >> Please give the attached patch a try and confirm it's working. It's >> working here for normal and zombie processes and it seems to be okay >> for uninterruptable sleep processes too, but I'd like to have someone >> else confirm everything looks right before I push this upstream. > Please don't use PATH_MAX in your code, that will eventually cause > problems. Especially using PATH_MAX will make builds for GNU/Hurd to > FTBFS. I inherited it that way. Though in this case it doesn't break for GNU Hurd systems because there is explicitly a check for that and, if it's not defined, PATH_MAX is declared in the code. So this code is GNU Hurd safe. Jesse
Bug#926896: sysvinit-utils: pidof is unreliable
> The RedHat bug that was the similar issue to #719273 (i.e. that
> resulted in the behaviour of pidof being changed) took a slightly
> different approach -
> https://bugzilla.redhat.com/show_bug.cgi?id=138788 (patch is
> https://bugzilla.redhat.com/attachment.cgi?id=113650=diff );
> did that ever make its way to upstream?
The Red Hat patch was a bit older, but I believe I've got it adjusted
and merged in okay. I've done some tests here and it looks like
everything is working with the newly patched pidof.
Here is what should happen now:
"pidof " should, without flags, display any processes
matches it finds, _except_ zombie processes. Basically anything running,
sleeping, or in uninterruptable sleep should now show up.
"pidof -z " should return all matching processes,
including those in the zombie state.
The attached patch also cleans up some code we don't need as a result of
this change and updates the man page.
Please give the attached patch a try and confirm it's working. It's
working here for normal and zombie processes and it seems to be okay for
uninterruptable sleep processes too, but I'd like to have someone else
confirm everything looks right before I push this upstream.
- Jesse
diff --git a/man/pidof.8 b/man/pidof.8
index ebe5f55..84ed1e4 100644
--- a/man/pidof.8
+++ b/man/pidof.8
@@ -66,9 +66,12 @@ a status of true or false to indicate whether a matching PID was found.
Scripts too - this causes the program to also return process id's of
shells running the named scripts.
.IP \-z
-Try to detect processes which are stuck in uninterruptible (D) or zombie (Z)
+Try to detect processes which are stuck in zombie (Z)
status. Usually these processes are skipped as trying to deal with them can cause
-pidof to hang.
+pidof or related tools to hang. Note: In the past pidof would ignore processes
+in the uninterruptable state (D), unless the \-z flag was specified. This is no
+longer the case. The pidof program will find and report processes in the D state
+whether \-z is specified or not.
.IP "-d \fIsep\fP"
Tells \fIpidof\fP to use \fIsep\fP as an output separator if more than one PID
is shown. The default separator is a space.
diff --git a/src/killall5.c b/src/killall5.c
index 22d29dc..b0728fa 100644
--- a/src/killall5.c
+++ b/src/killall5.c
@@ -67,9 +67,6 @@
#endif
#define STATNAMELEN 15
-#define DO_NETFS 2
-#define DO_STAT 1
-#define NO_STAT 0
/* Info about a process. */
typedef struct proc {
@@ -79,8 +76,6 @@ typedef struct proc {
char *argv1; /* Name as found out from argv[1] */
char *argv1base; /* `basename argv[1]` */
char *statname; /* the statname without braces*/
- ino_t ino; /* Inode number */
- dev_t dev; /* Device it is on */
pid_t pid; /* Process ID. */
pid_t sid; /* Session ID. */
char kernel; /* Kernel thread or zombie. */
@@ -481,19 +476,17 @@ int readarg(FILE *fp, char *buf, int sz)
* Read the proc filesystem.
* CWD must be /proc to avoid problems if / is affected by the killing (ie depend on fuse).
*/
-int readproc(int do_stat)
+int readproc()
{
DIR *dir;
FILE *fp;
PROC *p, *n;
struct dirent *d;
- struct stat st;
char path[PATH_MAX+1];
char buf[PATH_MAX+1];
char *s, *q;
unsigned long startcode, endcode;
int pid, f;
- ssize_t len;
charprocess_status[11];
/* Open the /proc directory. */
@@ -600,12 +593,8 @@ int readproc(int do_stat)
p->kernel = 1;
fclose(fp);
if ( (! list_dz_processes) &&
- ( (strchr(process_status, 'D') != NULL) ||
- (strchr(process_status, 'Z') != NULL) ) ){
- /* Ignore zombie processes or processes in
- disk sleep, as attempts
- to access the stats of these will
- sometimes fail. */
+ (strchr(process_status, 'Z') != NULL) ) {
+ /* Ignore zombie processes */
if (p->argv0) free(p->argv0);
if (p->argv1) free(p->argv1);
if (p->statname) free(p->statname);
@@ -672,55 +661,10 @@ int readproc(int do_stat)
/* Try to stat the executable. */
snprintf(path, sizeof(path), "/proc/%s/exe", d->d_name);
-
- p->nfs = 0;
-
- switch (do_stat) {
- case DO_NETFS:
- if ((p->nfs = check4nfs(path, buf)))
-goto link;
-/* else fall through */
- case DO_STAT:
- if (stat(path, ) != 0) {
-char * ptr;
-
-len = readlink(path, buf, PATH_MAX);
-if (len <= 0)
- break;
-buf[len] = '\0';
-
-ptr = strstr(buf, " (deleted)");
-if (!ptr)
- break;
-*ptr = '\0';
-len -= strlen(" (deleted)");
-
-if (stat(buf, ) != 0)
- break;
-p->dev = st.st_dev;
-p->ino = st.st_ino;
-p->pathname = (char *)xmalloc(len + 1);
-memcpy(p->pathname, buf,
Bug#926896: sysvinit-utils: pidof is unreliable
On 2021-10-21 1:40 p.m., Matthew Vernon wrote: > Hi, > > On 20/10/2021 15:29, Jesse Smith wrote: >> Is there a reason for wanting to revert this behaviour instead of using >> the "-z" flag on the command line? If you use pidof a lot and expect to >> see processes that are in the uninterruptable sleep state then making an >> alias of pidof='pidof -z' seems like a straight forward approach. >> >> Reverting the change entirely means the default behaviour could hang in >> NFS environments and I'm not sure non-functioning is a better situation >> than skipping sleeping processes. > > The RedHat bug that was the similar issue to #719273 (i.e. that > resulted in the behaviour of pidof being changed) took a slightly > different approach - > https://bugzilla.redhat.com/show_bug.cgi?id=138788 (patch is > https://bugzilla.redhat.com/attachment.cgi?id=113650=diff ); > did that ever make its way to upstream? > I don't think this Red Hat patch ever made its way upstream. I'll give it a test run. If it doesn't break anything it may prove to be a good solution that makes everyone happy.
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgradel
On 2021-10-20 4:59 p.m., Mark Hindley wrote: > I am unclear as to the significance of the reordering of .depends.* that > happens on the first run. Jesse, is that expected. Does it point to something? I suspect the initial reordering probably indicates one of two things: 1. There is something about the host system that is causing insserv to toggle ordering back and forth which we haven't been able to reproduce. This would result in insserv switching things back to "normal" when run on another system, and then leave it be. 2. Something on the host system changed init script dependencies, resulting in a reshuffling. This isn't necessarily a bad thing, it can happen when a script is added or removed from /etc/init.d, possibly by the package manager. It usually just indicated something changed since the last time insserv was run. Jesse
Bug#926896: sysvinit-utils: pidof is unreliable
Is there a reason for wanting to revert this behaviour instead of using the "-z" flag on the command line? If you use pidof a lot and expect to see processes that are in the uninterruptable sleep state then making an alias of pidof='pidof -z' seems like a straight forward approach. Reverting the change entirely means the default behaviour could hang in NFS environments and I'm not sure non-functioning is a better situation than skipping sleeping processes. Jesse On 2021-10-20 6:32 a.m., Mark Hindley wrote: > Tim, > > Thanks > > To me this seems a coherent argument for reverting this change. > > What do other people think? > > Mark >
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgrade
On 2021-09-26 8:55 p.m., Thorsten Glaser wrote: > On Sun, 26 Sep 2021, Jesse Smith wrote: > >> I checked out the init.d directories provided by Thorsten. One of the >> features of insserv allows it to test init scripts in an alternative >> directory or chroot. > This seems to be broken: > > tglase@tglase:~ $ insserv -p etc-stripped > insserv: fopen(/etc/init.d/.depend.boot): Permission denied > > (Having extracted the tarball as regular user so there are no > permission issues.) > I just realized what the problem is. On the version of insserv you are using, the command should be "insserv -p etc-stripped/init.d -i etc-stripped/init.d". The 1.21.0 version of insserv has a second flag for where to send dependency information. Jesse
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgrade
On 2021-09-26 8:55 p.m., Thorsten Glaser wrote: > On Sun, 26 Sep 2021, Jesse Smith wrote: > >> I checked out the init.d directories provided by Thorsten. One of the >> features of insserv allows it to test init scripts in an alternative >> directory or chroot. > This seems to be broken: > > tglase@tglase:~ $ insserv -p etc-stripped > insserv: fopen(/etc/init.d/.depend.boot): Permission denied > > (Having extracted the tarball as regular user so there are no > permission issues.) Hmm, that's interesting. What happens if you run "insserv -n -p etc-stripped"? And then, if it suggests changing the symlinks from K02 to K01, doing that manually? eg "mv etc-stripped/rc0.d/K02avahi-daemon etc-stripped/rc0.d/K01avahi-daemon" and then re-running insserv? >> If others see the same behaviour I do, then I'm guessing there is >> something else on the reporting system with the error which causes a >> conflict. But if other people are seeing the "toggle" between K01 and > Do you want strace(1)s or so? > Not yet, let's see what the above tests reveals first and go from there.
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgrade
On 2021-09-26 3:25 p.m., Thorsten Glaser wrote: > On Sun, 26 Sep 2021, Jesse Smith wrote: > >> behaviour. I've tried both the latest version of insserv (1.23.0) and >> the version which shipped with Debian 10 (1.18.0). I did notice having > This is Debian 11 so 1.21.0-1.1 (including Debian patches). > >> Thorsten, I wonder if you could give the latest version of insserv a try > No, this is a remote machine where I prefer to not risk bootability. > I checked out the init.d directories provided by Thorsten. One of the features of insserv allows it to test init scripts in an alternative directory or chroot. I ran insserv versions 1.18.0, 1.21.0, and 1.23.0 against the init.d scripts provided in the "etc-stripped" directory Thorsten uploaded. In each case insserv detected that the avahi-daemon script should be marked as K01 instead of K02. Which is good, that is consistent with what I have on my system and it looks to be correct. Once this change was made none of the three versions of insserv suggested switching avahi-daemon back to K02. ie On my system insserv does not perform the "toggle" action when given the uploaded init scripts. Which means when any recent version of insserv is run against just the init.d scripts provided it sets avahi-daemon to be K01 and leaves it that way, future runs don't toggle the symlink back. At least that's been my finding. Can anyone else confirm that running "insserv -p etc-stripped/init.d" on the uploaded scripts always sets avahi-daemon to K01, or do you get the "toggle" behaviour in the bug report? If others see the same behaviour I do, then I'm guessing there is something else on the reporting system with the error which causes a conflict. But if other people are seeing the "toggle" between K01 and K02 on the provided scripts then I'm guessing things are just working correctly for me due to an oversight or quirk of my system. Any feedback on confirming or disputing my findings is appreciated. - Jesse
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgrade
On 2021-09-26 2:37 p.m., Thorsten Glaser wrote: > On Sun, 26 Sep 2021, Jesse Smith wrote: > >> did last time. This time please run" >> >> # insserv -v -s >> >> This should set avahi-daemon to K01. Then run > Erm, well, it doesn’t. Apparently, the presence of -s prevents this. You're right, that's my mistake. I forgot the "-s" flag will show what insserv plans to do, but not actually update the symlinks. >> # insserv -v -s -n >> >> This should tell us whether insserv wants to toggle avahi-daemon back to >> K02, but without actually making any changes. And hopefully it will give >> a clue as to why this is happening.Thank you. > Maybe the attached will still suffice as clue? > Based on the output you shared it does indeed look like that whenever avahi-daemon is set to K02, insserv wants to switch it back to K01. But then when insserv is run again, without the -s or -n flags indicating a dry run, it tries to switch things back from K01 to K02. This is indeed strange. I've tried this again on my own machine and cannot reproduce the behaviour. I've tried both the latest version of insserv (1.23.0) and the version which shipped with Debian 10 (1.18.0). I did notice having other dependency trouble when I tested with insserv 1.21.0 which I believe is the version being used when the bug is triggered? Thorsten, I wonder if you could give the latest version of insserv a try (version 1.23.0) which can be downloaded here: https://download.savannah.nongnu.org/releases/sysvinit/insserv-1.23.0.tar.xz Perhaps this is related to a previous dependency issue we already fixed in 1.21.0. Or, if you're still experiencing the problem with insserv 1.23.0 then we've got a bigger mystery. - Jesse
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgrade
On 2021-09-26 1:54 p.m., Thorsten Glaser wrote: > On Sun, 26 Sep 2021, Jesse Smith wrote: > >> Something that might be useful here is seeing the output from "insserv >> -v -s -n". This will show in what order insserv intends to assign each >> service in each runlevel. No changes will be made to the system when >> insserv is run with the "-n" flag. > Attached. > >> It might also be useful to have the LSB header from the avahi-daemon >> service file in /etc/init.d/ > root@tglase:/etc # head -15 /etc/init.d/avahi-daemon > #!/bin/sh > ### BEGIN INIT INFO > # Provides: avahi avahi-daemon > # Required-Start:$remote_fs dbus > # Required-Stop: $remote_fs dbus > # Should-Start: $syslog > # Should-Stop: $syslog > # Default-Start: 2 3 4 5 > # Default-Stop: 0 1 6 > # Short-Description: Avahi mDNS/DNS-SD Daemon > # Description: Zeroconf daemon for configuring your network > #automatically > ### END INIT INFO > > PATH=/sbin:/bin:/usr/sbin:/usr/bin > Thanks, Thorseten, this is very helpful. So if I understand correctly if you do the "apt upgrade" process now (or a regular insserv call) avahi-daemon will get set to K01. Which looks right to me. But then if you run insserv a second time it toggles back to K02? Could you try this for me? Send me the same style of recored output you did last time. This time please run" # insserv -v -s This should set avahi-daemon to K01. Then run # insserv -v -s -n This should tell us whether insserv wants to toggle avahi-daemon back to K02, but without actually making any changes. And hopefully it will give a clue as to why this is happening.Thank you. - Jesse
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgrade
On 2021-09-26 1:12 p.m., Thorsten Glaser wrote: > On Sun, 26 Sep 2021, Mark Hindley wrote: > >> Thorsten's original report[1] suggests it happens on every upgrade. Not only every upgrade, but if I'm reading the report correctly it looks like insserv is toggling between K01 and K02 with every time it is run, even if no "apt upgrade" happens. Assuming I'm reading that report correctly that means insserv is giving different output from the same input every time. I've tried this on my own machine with avahi-daemon installed and I'm not seeing the same thing, in my case it always gets assigned to K01avahi-daemon. Something that might be useful here is seeing the output from "insserv -v -s -n". This will show in what order insserv intends to assign each service in each runlevel. No changes will be made to the system when insserv is run with the "-n" flag. It might also be useful to have the LSB header from the avahi-daemon service file in /etc/init.d/ Jesse
Bug#989284: insserv: toggles rc0.d/{K02avahi-daemon => K01avahi-daemon} with every upgrade
On 2021-09-26 7:10 a.m., Mark Hindley wrote:
> Jesse,
>
> On Mon, May 31, 2021 at 04:40:17AM +0200, Thorsten Glaser wrote:
>> Package: insserv
>> Version: 1.21.0-1.1
>> Severity: normal
>> X-Debbugs-Cc: t...@mirbsd.de
>>
>> I believe this is insserv; if not, feel free to reassign.
>>
>> With every update, I have etckeeper churn:
>>
>> [master 8a1a489] committing changes in /etc made by "apt-get --purge
>> dist-upgrade"
>> Author: mirabilos <…>
>> 4 files changed, 91 insertions(+), 9 deletions(-)
>> rename rc0.d/{K02avahi-daemon => K01avahi-daemon} (100%)
>> rename rc1.d/{K02avahi-daemon => K01avahi-daemon} (100%)
>> rename rc6.d/{K02avahi-daemon => K01avahi-daemon} (100%)
>>
>> Basically, avahi-daemon toggles between K01 and K02 every time.
> I can't see where/how this is happening in insserv. Can you identify the
> cause?
>
Hi Mark,
It's certainly possible insserv is renaming the shortcuts for
avahi-daemon from K02 to K01. Though this raises a few questions, I believe:
1. Why do you think avahi-daemon should be prefixed with K02 instead of
K01? (Or does it switch back and force every time insserv is run?) On my
system it is set as K01 and this looks to be correct.
2. Is there a service on the system that is getting
added/removed/changed during the apt upgrade process that is affecting
the order in which avahi-daemin is getting shutdown?
I'm suggesting it's likely insserv is making the change, I'm just not
sure if it's a bug without knowing what else is changing on the system.
Jesse
Bug#990019: #990019,initscripts: single user mode broken with newer versions of systemd
I was a little confused by the subject of this bug since this has nothing to do with systemd. Neither the script nor the init call has any connection to systemd. The issue here is that the init script (/etc/init.d/single) is trying to call SysV init with the "-t1" flag, which is not valid. There is no -t flag or -t1 flag for init. I suspect what the script was intended to do is call "/sbin/telinit -t 1 s". This would switch to single user mode after a delay of just one second. - Jesse
Bug#971713: sysstat: init or systemd file has overlapping runlevels
On 2021-02-19 4:38 a.m., Matthew Vernon wrote: > On 15/12/2020 21:34, Jesse Smith wrote: >> On 2020-12-15 5:04 p.m., Trek wrote: >>> On Tue, 15 Dec 2020 12:45:40 -0400 >>> Jesse Smith wrote: >>> >>>> I gave the patch a test run and, while I like what it does in theory, >>>> in practise I'm running into trouble with it. When I use the attached >>>> patch and then run "make check" in the insserv source directory, >>>> inssrev segfaults after about eight tests. I haven't had a chance yet >>>> to hunt down what is causing the problem, but I'm guessing a variable >>>> is getting used before it is given a value/initialized. >>> well, moving the Start_Stop_Overlap call broke the assumption that >>> start_levels and stop_levels are never undefined, so this additional >>> patch should fix the regression >>> >>> make check now passes all 240 tests >>> >>> thanks for the review and happy hacking! :) >> >> Thank you for the updated patch. I have confirmed it works here and >> passes all checks. I've committed the change for the upcoming 1.23.0 >> branch of insserv. People are welcome to test it out and, when the next >> version gets published, this fix (and update to the manual page) will be >> included. > > Are you in a position to publish this version? insserv still has this > grave severity bug open against it, which it'd be good to resolve ASAP > before insserv gets pulled from testing... A new version of SysV init and innserv have been published today. The new version of insserv (1.23.0) fixes the above issue. It can be downloaded from here: http://download.savannah.nongnu.org/releases/sysvinit/ - Jesse
Bug#971713: sysstat: init or systemd file has overlapping runlevels
On 2020-12-15 5:04 p.m., Trek wrote: > On Tue, 15 Dec 2020 12:45:40 -0400 > Jesse Smith wrote: > >> I gave the patch a test run and, while I like what it does in theory, >> in practise I'm running into trouble with it. When I use the attached >> patch and then run "make check" in the insserv source directory, >> inssrev segfaults after about eight tests. I haven't had a chance yet >> to hunt down what is causing the problem, but I'm guessing a variable >> is getting used before it is given a value/initialized. > well, moving the Start_Stop_Overlap call broke the assumption that > start_levels and stop_levels are never undefined, so this additional > patch should fix the regression > > make check now passes all 240 tests > > thanks for the review and happy hacking! :) Thank you for the updated patch. I have confirmed it works here and passes all checks. I've committed the change for the upcoming 1.23.0 branch of insserv. People are welcome to test it out and, when the next version gets published, this fix (and update to the manual page) will be included. - Jesse
Bug#971713: sysstat: init or systemd file has overlapping runlevels
On 2020-12-15 11:31 a.m., Trek wrote: > @Jesse Smith: I attached a possible fix, but it slightly changes the > behavior, as now it prints the overlapping warning when loading all the > init.d scripts and not only for the ones in the commandline > > this because if script_inf.default_start and stop are empty, they are > overwritten by the levels gathered from rc.d links and I do not fully > understand the implications of removing those overwrites, so I just > moved the code before this part > > the second patch is for the manpage about empty fields, that are > allowed by insserv, but probably it could be explained better > > please to tell me if you need some more info or work to be done :) Thanks for taking a look at this and sending in the patch. I gave the patch a test run and, while I like what it does in theory, in practise I'm running into trouble with it. When I use the attached patch and then run "make check" in the insserv source directory, inssrev segfaults after about eight tests. I haven't had a chance yet to hunt down what is causing the problem, but I'm guessing a variable is getting used before it is given a value/initialized. The output from the test run is included at the bottom of this e-mail. If you could please take a look at it and make sure the new insserv code passes the "make check" tests, I'll give it another test run here and get the new patch merged upstream. - Jesse info: test normal boot sequence scripts, and their order insserv: Could not read script nolsbheader: No such file or directory insserv: warning: script 'nolsbheader' missing LSB tags insserv: Could not read script nolsbheader: Success insserv: warning: script 'nolsbheader' missing LSB tags suite: line 104: 11856 Segmentation fault $insserv $debug -c $insconf -i $insservdir -p $initddir -o $overridedir $script success: 0 test executed, 0 nonfatal tests failed. make: *** [Makefile:165: check] Error 139
Bug#973561: sysvinit-utils: the pidof program doesn't find, processes in the 'D' state
The behaviour described, pidof not seeing processes in the "D" state, is expected, correct, and documented in the pidof manual page. As it says in the manual page, if yo uwant to see processes which are uninterruptable or zombies then it is required to pass the "-z" command line flag. - Jesse
Bug#967752: sopwith: depends on deprecated GTK 2
I'm the upstream maintainer for Sopwith. I'd like to point out that while Sopwith _can_ use GTK2 for its graphical interface, the preferred method as been SDL for several years. We had too many problems with GTK2 and deprecated it as a front-end. Sopwith does not have a hard dependency on GTK and so this recommended dependency can simply be removed from the Debian package. There isn't any need for us to port to GTK3 because we haven't been maintaining the GTK2 port up to this point. The GTK build option can just be disabled downstream. Jesse
Bug#926896: sysvinit-utils: pidof is unreliable
David, I appreciate you doing some clean-up on the code and addressing the zombie/sleeping process issue. When I was applying the patch the part addressing the missing zombie process did not apply and when I looked closer at the code it appears that you are fixing is something that was already fixed in the most recent version of pidof. The code you removed to make sure pidof checks for processes in the D or Z states is already skipped in the most recent version when the "-z" flag is used. I'm still applying the checks you put in for memory freeing and removing old comments. The bit about checking for zombies/sleeping processes though we've already addressed upstream in version 2.96. - Jesse
Bug#926896: sysvinit-utils: pidof is unreliable
On 10/22/19 11:07 PM, Hoyer, David wrote:
> We have also been experiencing this problem since moving to Buster. We
> never saw this with Jessie. I believe it comes down to the following code
> in readproc:
>
> if ( (strchr(process_status, 'D') != NULL) ||
> (strchr(process_status, 'Z') != NULL) ){
>/* Ignore zombie processes or processes in
> disk sleep, as attempts
> to access the stats of these will
> sometimes fail. */
> if (p->argv0) free(p->argv0);
> if (p->argv1) free(p->argv1);
> if (p->statname) free(p->statname);
> free(p);
> continue;
> }
>
> Our scenario is similar although not with the same process that is described
> below.It seems like this makes pidof not as effective for finding the pid
> of a process if D states are skipped. Are there alternatives to pidof?
> (BTW the -z option does not appear to help since this code snippet will
> remove it from the process list).
>
This seems odd to me because, prior to Buster, pidof would _always_ skip
processes in the zombie or sleeping (Z or D) states. The -z flag was
introduced to allow it to not skip them. So everything else being equal,
I'd expect to see the same number of gaps, or more gaps in the output on
older releases. Unless the process being watched (acupsd in this case)
is doing a lot more uninterruptable sleeping.
Bug#926896: sysvinit-utils: pidof is unreliable
> Jesse, >any ideas how it could be possible for process to be discovered by >ps(1), but not pidof(1)? > I can think of a few possibilities, though they seem unlikely. One is that the process could be crashing and restarting, making it a zombie for brief periods of time. Testing pidof with the "-z" flag would fill in the "holes" in the test output if that theory is correct. Alternatively, if the executable is on a remote network share, like NFS, that might explain the gaps. While it doesn't explain the gaps in output, I am curious if the gaps also appear if the pidof test is run without the "-s" flag. So instead of "pidof -s apcupsd" what happens if you run "pidof -z apcupsd"? - Jesse
Bug#556893: #556893,say which 'defaults' are which better
On 10/13/19 4:33 PM, Michael Biebl wrote: > Control: tags -1 + moreinfo > > On Sat, 28 Sep 2019 15:21:17 -0300 Jesse Smith > wrote: >> This has been addressed upstream in insserv by allowing the program to >> accept changes like this silently. All we need now is for update-rc.d to >> be updated to use the new behaviour (enabled with the -q flag) and this >> issue can be closed. > Is it -q or --silent? Or are they the same? -q and --silent do the same thing. These are now documented in the manual page for insserv. > > Will insserv fail if the version is too old, i.e. do we need a versioned > dependency (or rather versioned Breaks)? Yes, older versions of insserv will fail if --silent is specified as it will not be a recognized option. There should probably be a version check. Or a check in the calling script to try with insserv --silent and if it fails, try again without the flag. > What exactly is suppressed by -q/--silent? > Only this specific error about "defaults" or other error messages as > well? Do we want to suppress all error messages? The --silent flag suppresses warnings (non-fatal errors). Basically anything that is a "heads up" warning is suppressed. Fatal errors and issues which require user attention are still printed.
Bug#556893: #556893,say which 'defaults' are which better
This has been addressed upstream in insserv by allowing the program to accept changes like this silently. All we need now is for update-rc.d to be updated to use the new behaviour (enabled with the -q flag) and this issue can be closed. - Jesse
Bug#668523: sysvinit: Returning from single user leaves current tty disfunctional
I have tried to duplicate this issue with Debian 9 and have been unable to duplicate the tty issue. Seems this was fixed in a release since the report. Unless someone can duplicate this issue with a current version of Debian/init, I suggest this report can be closed. - Jesse
Bug#932290: insserv: Script has overlapping Default-Start and Default-Stop runlevels
When I applied the patch provided above it causes insserv to segfault when running the testsuite (make check). Looks like the program is crashing using one of the LSB header checks. I haven't looked into it in any detail yet, but it seems moving that check to a sooner point causes a memory issue. - Jesse
Bug#588666: boot message stuck onto next message
On Sun, 25 Aug 2019 16:07:30 + Dmitry Bogatov wrote: > > > Each line a process sends: > > > * should be prefixed by the name of the process that sent it. > > > * should end with a newline. > > > > And (#398269) each line should be either from a process or the kernel, > > distinguishable enough. > > > > I fully agree, that should be basic and not need discussion. > > Okay, reassigning then. > > @Jesse, yet another feature request for startpar :) I have added a command line flag (-n) which causes the name of the sending process to prefix output, unless the command is interactive. It looks like this: startpar -n -t 1 process-a process-b process-a: Starting process... process-b: Starting process... process-b: Done. process-a: Done. This will be in the next release of startpar. I'm looking at adding a newline to the end of output too. - Jesse
Bug#935302: insserv: bootlogd stops too early
>> @Jesse Is is possible to specify dependencies between two scripts >> that both depends on $all? What we might consider is adding new expandable dependency variables to insserv.conf. We already have things like $all, $time and $network. Maybe we need another variable like $rc-local to better define when scripts are supposed to run? Then bootlogd could depend on $rc-local instead of $all? - Jesse
Bug#935302: insserv: bootlogd stops too early
On 8/23/19 8:22 AM, Dmitry Bogatov wrote: > > control: tags -1 +confirmed > control: reassign -1 initscripts > control: severity -1 normal > > [2019-08-21 15:22] Thorsten Glaser >> Package: insserv >> Version: 1.20.0-2 >> Severity: minor >> >> Unsure which package, but: >> >> [???] >> Starting OpenBSD Secure Shell server: sshd. >> Stopping boot logger: bootlogd. >> Starting TLS tunnels: >> Setting sysfs variables >> Starting Tomcat 9 servlet engine:. >> User-mode networking switch disabled ... (warning). >> Starting Micro Name Service Cache Daemon: unscd. >> Starting libvirt logging daemon: virtlogd. >> Starting libvirt management daemon: libvirtd. >> Starting Remote Desktop Protocol server: xrdp-sesman xrdp[20190821-15:20:30] >> [DEBUG] Testing if xrdp can liste >> n on 0.0.0.0 port 3389. >> [20190821-15:20:30] [DEBUG] Closed socket 7 (AF_INET6 :: port 3389) >> . >> Running local boot scripts (/etc/rc.local) >> <13>Aug 21 15:20:33 rc.local[4234]: All done. >> >> This is clearly too early. > > Definitely. > > I added "Required-Start: $all" into /etc/init.d/stop-bootlogd and run > insserv(8). After that `stop-bootlogd` got ordered last. But there is > one issue. > > This way, both `rc.local' and `bootlogd` depend on $all, but their > relation undefined. Well, it is lexicographic, but it plainly wrong. > > Attempt to add "Required-Start: $all rc.local" causes multitude error > messages from insserv: > > insserv: Starting stop-bootlogd depends on rc.local and therefore on > system facility `$all' which can not be true > > @Thorsten Does it solves problem for you? > > @Jesse Is is possible to specify dependencies between two scripts that >both depends on $all? > > I don't think it is possible to fine-tune dependencies between scripts when "$all" is used. The "$all" variable is supposed to be a very special case when everything else is started or handled. I'm not sure if we have a good way to specify two or more scripts that are all handled last.
Bug#613605: patch -b and -V options, overwrites file.orig despite manpage description (additional info)
I looked at the documentation and the examples provided here and they are working as documented. I don't think there is a bug here. Now, in theory, it might be nice to have a flag which prevents old .orig files from being overwritten. I can see how that would be beneficial. But the examples provided here are working as intended. - Jesse
Bug#931977: startpar: uninitialized variable
control: tags -1 +fixed-upstream This issue has been fixed upstream and will be resolved in startpar-0.64. - Jesse
Bug#934945: startpar: insserv attemps to write to /etc/.boot.* even with -p option
On 8/17/19 3:29 PM, Ian Jackson wrote:
> Jesse Smith writes ("Bug#934945: startpar: insserv attemps to write to
> /etc/.boot.* even with -p option"):
>> The change has been made upstream so this should be fixed when the next
>> version comes out, probably around the end of September. That would save
>> the trouble of patching the test script.
> For Debian, this test failure is regarded as an RC bug. So we need to
> fix it there ASAP, by forward porting the commit. (A simple git
> cherry pick I assume. Plus the test dependency change if we need it,
> of course, but we need to keep that.
>
Attached is the patch to correct the bug. That can be used until
startpar-0.64 is published.
diff --git a/testsuite/runtests b/testsuite/runtests
index 5776c78..1204592 100755
--- a/testsuite/runtests
+++ b/testsuite/runtests
@@ -14,7 +14,7 @@ touch etc/insserv.conf
cp testscript etc/init.d/test
chmod a+rx etc/init.d/test
-"$INSSERV" -p etc/init.d test
+"$INSSERV" -p etc/init.d -i etc/init.d test
if $STARTPAR -d etc/init.d -e etc -P S -R 2 -M start 2>&1 | grep -q 'is
running' ; then
echo Test 1 success: the test script in init.d was running
else
Bug#934945: startpar: insserv attemps to write to /etc/.boot.* even with -p option
On 8/17/19 2:14 PM, Ian Jackson wrote:
> Jesse Smith writes ("Bug#934945: startpar: insserv attemps to write to
> /etc/.boot.* even with -p option"):
>> On a related note, in the startpar build log it shows the version of
>> startpar being tested is located at /lib/startpar/startpar. Upstream
>> we use the startpar executable in the local directory to make sure
>> we are using the fresh copy and not the system copy. It looks like
>> maybe Startpar is being installed before the test is run? In which
>> case it works out to be the same thing.
> Debian's CI at ci.debian.net wants to test *packages*, in the context
> of the whole system, so it tests everything as-installed. So this
> seems correct.
Sounds good.
>> This is a pretty minor change, in the Startpar "testsuite/runtests" script
>> change the line which reads:
>>
>> $INSSERV" -p etc/init.d test
>>
>> to
>>
>> $INSSERV" -p etc/init.d -i etc/init.d test
> It sounds like we should make this change to the test suite, and also
> add a test dependency saying we need at least whatever insserv gained
> the -i option ?
The change has been made upstream so this should be fixed when the next
version comes out, probably around the end of September. That would save
the trouble of patching the test script.
I'm not sure of the specific version where the behaviour change took
place, but any version of insserv >= 1.20.0 should be a good match for
startpar going forward.
- Jesse
Bug#934945: startpar: insserv attemps to write to /etc/.boot.* even with -p option
> Folks, we have serious regression. Not sure, whether it is bug in > startpar test suite, which invokes `insserv' with `-p', but without > `-i', or it is regression in `insserv'. It's sort of a fore-gression. The Startpar testsuite was set up to work with older versions of insserv, like insserv-1.14.0. I've updated the test script to work with insserv-1.20.0. This will be fixed in the next version of Startpar. This is a pretty minor change, in the Startpar "testsuite/runtests" script change the line which reads: $INSSERV" -p etc/init.d test to $INSSERV" -p etc/init.d -i etc/init.d test On a related note, in the startpar build log it shows the version of startpar being tested is located at /lib/startpar/startpar. Upstream we use the startpar executable in the local directory to make sure we are using the fresh copy and not the system copy. It looks like maybe Startpar is being installed before the test is run? In which case it works out to be the same thing. - Jesse
Bug#672361: bootlogd: escape sequences should be filtered out
On 8/13/19 2:06 PM, Dmitry Bogatov wrote: > [2019-08-12 17:45] Jesse Smith >> Certainly. Attached is a file with the last 20 lines of the >> /var/boot/log file on a test machine. When I view the log with "less" >> the output looks normal. When I run the file through "head" "tail" or >> "cat" the "[ ok" part of the message line appears at the beginning of >> the text. This happens in both the text console and in a virtual >> terminal window. > Ah, everything is clear now. Log file contains following escape > sequence: > >[ 1 G > > It moves cursor to first column of terminal, so text after displaces > text (timestamp) at beginning of line. I came to a similar conclusion. I found I could edit the character sequences in vim and pick which ones to keep or erase. As you pointed out, editing or filtering some sequences makes for fragile formatting. We could remove just those sequences in bootlogd, but I think the better solution is (as you wrote) to fix the formatting in the program sending us the log messages.
Bug#672361: bootlogd: escape sequences should be filtered out
On 8/12/19 4:30 PM, Dmitry Bogatov wrote: > [2019-08-11 18:08] Jesse Smith >>> No. When you output raw escape sequences on terminal, they will be >>> interpreted by terminal, so `head', `tail' and `grep' output will be >> readable and colored. Pager `less' with -R option also interpret escape >> symbols. >> >> I wasn't speaking theoretically. > No offence intended. Sorry if I sounded harsh. No offence taken. I can understand why you'd expect the output to look normal. > >> I tried it and the "head", "cat" and "tail" commands mangle the lines >> of the log file when escape sequences are not escaped. Output from >> "less" is clean though and looks correct. > Interesting. Can you please send text that shows this behaviour? > Certainly. Attached is a file with the last 20 lines of the /var/boot/log file on a test machine. When I view the log with "less" the output looks normal. When I run the file through "head" "tail" or "cat" the "[ ok" part of the message line appears at the beginning of the text. This happens in both the text console and in a virtual terminal window. This output was achieved by turning off escape character filtering in bootlogd. The contents of the file are just written out raw, as the data comes into bootlogd. - Jesse Sat Aug 10 20:32:12 2019: nm-dispatcher: req:2 'connectivity-change': new request (2 scripts) Sat Aug 10 20:32:12 2019: nm-dispatcher: req:2 'connectivity-change': start running ordered scripts... Sat Aug 10 20:32:15 2019: [] Starting SMB/CIFS daemon: smbd[?25l[?1c7[1G[[32m ok [39;49m8[?25h[?0c. Sat Aug 10 20:32:15 2019: Applying power save settings...done. Sat Aug 10 20:32:15 2019: Setting battery charge thresholds...done. Sat Aug 10 20:32:15 2019: Radio device states restored. Sat Aug 10 20:32:15 2019: nm-dispatcher: req:3 'up' [eth0]: new request (2 scripts) Sat Aug 10 20:32:15 2019: nm-dispatcher: req:3 'up' [eth0]: start running ordered scripts... Sat Aug 10 20:32:16 2019: ModemManager[2258]: Couldn't check support for device at '/sys/devices/pci:00/:00:03.0': not supported by any plugin Sat Aug 10 20:32:16 2019: Sat Aug 10 20:32:18 2019: [] Starting VirtualBox AdditionsVBoxService 5.2.24_Debian r128122 (verbosity: 0) linux.amd64 (Jan 24 2019 21:06:36) release log Sat Aug 10 20:32:18 2019: 00:00:00.000361 main Log opened 2019-08-10T23:32:18.177888000Z Sat Aug 10 20:32:18 2019: 00:00:00.000582 main OS Product: Linux Sat Aug 10 20:32:18 2019: 00:00:00.000627 main OS Release: 4.13.0-1-amd64 Sat Aug 10 20:32:18 2019: 00:00:00.000665 main OS Version: #1 SMP Debian 4.13.13-1mx17 (2017-11-18) Sat Aug 10 20:32:18 2019: 00:00:00.000704 main Executable: /usr/sbin/VBoxService Sat Aug 10 20:32:18 2019: 00:00:00.000706 main Process ID: 2615 Sat Aug 10 20:32:18 2019: 00:00:00.000707 main Package type: LINUX_64BITS_GENERIC (OSE) Sat Aug 10 20:32:18 2019: 00:00:00.007391 main 5.2.24_Debian r128122 started. Verbose level = 0 Sat Aug 10 20:32:18 2019: [?25l[?1c7[1G[[32m ok [39;49m8[?25h[?0c.
Bug#672361: bootlogd: escape sequences should be filtered out
On 8/12/19 4:20 AM, Vincent Lefevre wrote: > On 2019-08-11 17:57:18 -0300, Jesse Smith wrote: >> For situations like this where you already have a log with escape >> characters in it, you can fix the output by using the readbootlog >> program, which is part of the sysvinit suite. It cleans out most of the >> control characters so the log looks like a normal, plain-text file. > It does not seem to be in Debian (not installed by default, not found > by "apt-file search readbootlog"). > Which version of sysvinit do you have installed? You'd need a version from around 2.90 or newer. >> There might be problems with some implementations of these utilities >> due to binary data, but AFAIK, coreutils implementation is fine with >> escape sequences. They are not. As I mentioned above, I tested this. >> If this is correct with "less", then there should be no issues with a >> decent implementation of "cat", "head" and "tail". Sadly, that is not the case.
Bug#672361: bootlogd: escape sequences should be filtered out
On Sun, 11 Aug 2019 17:47:17 + Dmitry Bogatov wrote: > > I'd like to point out though that with such an option enabled, it is > > going to result in some weird output. If all escape sequences are > > printed to the file, tools like "less" can handle it, but other (more > > raw) text manipulation tools such as "head" and "tail" will end up > > mangling the lines. > > No. When you output raw escape sequences on terminal, they will be > interpreted by terminal, so `head', `tail' and `grep' output will be readable and colored. Pager `less' with -R option also interpret escape symbols. I wasn't speaking theoretically. I tried it and the "head", "cat" and "tail" commands mangle the lines of the log file when escape sequences are not escaped. Output from "less" is clean though and looks correct. > Looks like your terminal interpret '\r' incorrectly. What makes you think that? The lines look exactly as I would expect them to look. > What does following command print on your terminal? > $ bash -c "printf 'foo\rbar'" It prints "bar", just as it should.
Bug#672361: bootlogd: escape sequences should be filtered out
> Please, don't. Issue with '\r' can be resolved by removal of '\r' in > `bin:lsb-base'. Okay, I'll do an option for completely unfiltered and other tools and be adjusted to match. > With filtering as implemented now `/var/boot/log' looks bad both in editor > and terminal. Example: > > Fri Feb 22 18:42:36 2019: [] Not activating swap on logical volume. > ...??7[warn8?? (warning). > Fri Feb 20 18:42:36 2019: [] Starting early crypto disks...sda3_crypt > (running)...??7[ ok 8??done. > For situations like this where you already have a log with escape characters in it, you can fix the output by using the readbootlog program, which is part of the sysvinit suite. It cleans out most of the control characters so the log looks like a normal, plain-text file.
Bug#672361: bootlogd: escape sequences should be filtered out
On Thu, 08 Aug 2019 20:21:50 + Dmitry Bogatov wrote: > > control: tags -1 +upstream > > [2019-08-07 05:13] Adam Borowski > > [...] > > > a /var/log/boot.log file is > > > generated where nothing is filtered out, so that the file is readable > > > with "cat" or "less" (and text is colored). > > > > I don't think files in /var/log/ should be anything but plain text -- at > > least unless colorized-logs becomes essential :þ and/or less defaults to > > -R. But until a solution is implemented, I agree that leaving binaryish > > control codes intact is better than corrupting them. > > Jesse, there seems to be demand on turning-off escape sequence filtering > in bootlogd. Can you please make it configurable? It is pretty easy to make an option for printing the escape sequences to the log file. This will allow tools like "less" to print the boot log with its colour codes. I'd like to point out though that with such an option enabled, it is going to result in some weird output. If all escape sequences are printed to the file, tools like "less" can handle it, but other (more raw) text manipulation tools such as "head" and "tail" will end up mangling the lines. This is partly because escape characters include positional instructions like '\r' for carriage-return. In other words, if we make the boot log completely unfiltered, lines in "less" will display properly, but using "cat", "head" or "tail" will result in mangled lines that look like this: Thu Aug 8 19:06:30 2019: [ ok ug 8 19:06:30 2019: [... starting ] I'm not sure we want to do that. Perhaps the ideal would be a small degree of filtering to remove the positional control characters (like '\r') while leaving the rest in to allow for colour to be displayed? - Jesse
Bug#564832: exim4-daemon-heavy: exim4-base 4.71.3 - start and stop runlevels changed, warnings
I'm not entirely sure, but based on the information provided here it looks like there may have been an override file in place which later got removed. If this bug is still occurring and/or reproducible then I'll happily look into it. However, if it is not longer happening then I suspect this was an override/config error and the bug could be closed. - Jesse
Bug#931976: startpar: does testsuite of startpar actually test anything?
I have looked into this and, as Dmitry pointed out, there were a few things amiss with the test suite. 1. There was the way the test script was being created on the fly. I have addressed this by creating a standalone file in the testsuite directory. 2. Multiple tests were being run, but this was not clear from the test suite's output. It was reporting failure in one test and success in another. I have addressed this by numbering the tests. From now one the test suite will print "Test 1 failed, Test 2 success..." 3. There was an error in the way startpar was handling command line arguments, passed to it from the test suite. This has been fixed upstream too. In the next version of startpar, running "make check" will run the test suite and print out test number and their status. As of this moment, both tests pass on my system. I believe we can mark this bug as fixed upstream, unless there are more concerns with the test suite checks. - Jesse
Bug#743274: insserv: warns about configuration changes
I have added a silent mode flag for insserv. Running the program with
either "-q" or "--silent" will surpress most warning messages. insserv
will only print fatal errors, when it is about to exit, when in silent mode.
A patch for the new behaviour is attached. Maybe we can get update-rc.d
patched to match the new behaviour.
This change will be available in insserv-1.21.0.
- Jesse
diff --git a/insserv.8.in b/insserv.8.in
index bc8f351..0a4b0cf 100644
--- a/insserv.8.in
+++ b/insserv.8.in
@@ -264,6 +264,11 @@ Currently the following options are recognized by insserv:
.BR \-v ,\ \-\-verbose
Perform operation with more diagnotic messages printed on stderr.
.TP
+.BR \-q ,\ \-\-silent
+Perform operations silently. This blocks warning messages
+from being printed to stderr. Only fatal error messages
+are printed.
+.TP
.BR \-c\ ,\ \-\-config\
Specify path to the insserv.conf file and the insserv.conf.d
directory. Useful for testing.
diff --git a/insserv.c b/insserv.c
index c9d8c69..ef8025f 100644
--- a/insserv.c
+++ b/insserv.c
@@ -176,7 +176,7 @@ static char buf[LINE_MAX];
/* When to be verbose, and what level of verbosity */
static int verbose = 0;
-
+static boolean silent_mode = false;
static boolean dryrun = false;
/* When paths set do not add root if any */
@@ -1233,6 +1233,9 @@ void error (const char *restrict const fmt, ...)
*/
void warn (const char *restrict const fmt, ...)
{
+if (silent_mode)
+return;/* do not print warnings in silent mode */
+
va_list ap;
va_start(ap, fmt);
_logger(fmt, ap);
@@ -2762,6 +2765,7 @@ static struct option long_options[] =
{"path", 1, (int*)0, 'p'},
{"override",1, (int*)0, 'o'},
{"upstart-job", 1, (int*)0, 'u'},
+{"silent", 0, (int*)0, 'q'},
{"recursive", 0, (int*)0, 'e'},
{"showall",0, (int*)0, 's'},
{"show-all",0, (int*)0, 's'},
@@ -2778,6 +2782,7 @@ static void help(const char *restrict const name)
printf(" -r, --remove Remove the listed scripts from all
runlevels.\n");
printf(" -f, --force Ignore if a required service is missed.\n");
printf(" -v, --verboseProvide information on what is being done.\n");
+printf(" -q, --silent Do not print warnings, only fatal errors.\n");
/* printf(" -l, --legacy-path Place dependency files in /etc/init.d
instead of /lib/insserv.\n"); */
printf(" -i, --insserv-dir Place dependency files in a location other
than /lib/insserv\n");
printf(" -p , --path Path to replace " INITDIR ".\n");
@@ -2830,7 +2835,7 @@ int main (int argc, char *argv[])
for (c = 0; c < argc; c++)
argr[c] = (char*)0;
-while ((c = getopt_long(argc, argv, "c:dfrhvni:o:p:u:es", long_options,
(int *)0)) != -1) {
+while ((c = getopt_long(argc, argv, "c:dfrhqvni:o:p:u:es", long_options,
(int *)0)) != -1) {
size_t l;
switch (c) {
case 'c':
@@ -2848,6 +2853,9 @@ int main (int argc, char *argv[])
case 'f':
ignore = true;
break;
+case 'q':
+silent_mode = true;
+break;
case 'v':
verbose ++;
break;
Bug#743274: insserv: warns about configuration changes
This is an interesting issue. I think this is what is happening: 1. update-rc.d wants to disable the osspd service. To do this is creates a file called /etc/init/osspd.override. 2. update-rc.d then calls insserv. insserv sees that the osspd script normally starts in runlevels "2 3 4 & 5", but there is an override in place preventing the script from starting. 3. insserv then warns about the situation, to let the user know osspd's default runlevels have been altered. What makes this tricky, in my mind, is that if insserv were run as a stand-alone program from the command line, it would be entirely correct to warn the user that a script's defaults were overridden and its behaviour changed. If I were to simply run "insserv" on its own, this is what I would want. But in this case the update-rc.d script is calling insserv, insserv isn't being run stand-alone. Since we just told update-rc.d to disable osspd, and we want that override behaviour, the warning seems entirely out of place. In other words, I believe both update-rc.d and insserv are behaving correctly, and this would be proper behaviour if insserv were run on its own. The problem is mixing the two together. I have two suggestions to improve the current behaviour: 1. update-rc.d can be edited to catch the warning. Piping output from insserv and running it through sed or grep would avoid warnings about scripts update-rc.d had just changed. 2. We can add a quiet/silent flag (maybe -q) to insserv. When that flag is present, the program does not print out warnings. We would still print serious errors, but not minor warnings for override situations. Then the update-rc.d script can just call "insserv -q" to avoid extra output like this. I'm open to feedback but I think #2 is probably the best way forward for everyone. Thoughts? - Jesse
Bug#932124: insserv: test suite does not work with installed binary
The assumption here, that the user can run "make check insserv=/sbin/insserv" and have it run the testsuite against the installed binary is correct. This does work. The problem encountered in the bug report is an older version of insserv is installed at /sbin/insserv. The older copy does not recognize all of the flags current used in the testsuite which is why it is failing. If a relatively newer version of insserv were installed, the testsuite would work with it. You can test this by running "make && cp insserv /tmp && make check insserv=/tmp/insserv". I think we can close this bug as the testsuite is working as expected, the locally installed version of insserv is just too old to work with the tests. - Jesse
Bug#622970: Reassigned to insserv #622970
On Sun, 17 Apr 2011 20:09:40 +0200 Thomas Hood wrote: > I have just reassigned this report from resolvconf to > insserv because I believe that the insserv maintainers > will have more insight into the problem than I have. > > Here is a summary of what we have found out, as > described earlier in the report's message log. > > Resolvconf was misbehaving on the submitter's two fresh > squeeze machines. This was because resolvconf's initscript > had been installed in rcS.d at the traditional sequence > number S38 rather than at the appropriate sequence number > for the submitter's systems, S13. This was because, > although the submitter's systems had evidently been > subjected to boot sequence reordering by (I presume) > insserv, the /etc/init.d/.legacy-bootordering flag file was > still present on his systems. > > This incongruous situation --- reordered boot sequence, but > legacy flag file present --- may have resulted from the > presence of initscripts on the system lacking LSB headers. > -- > Thomas Hood This is an interesting bug. Though I'm not sure if it is still a problem. As far as I know, current versions of update-rc.d and insserv do not check for the ".legacy-bootordering" file flag (or create or remove it). I could be mistaken, but I think this was either resolved quietly in the past, or is outside our scope. I tried running the latest insserv with and without /etc/init.d/.legacy-bootordering in place and didn't encounter differences in behaviour. I think we can probably mark this one as closed unless someone comes up with a way insserv is misbehaving in this situation. - Jesse
Bug#633858: -v not as safe as it sounds
The patch has been applied upstream and will appear in insserv 1.21.0. - Jesse
Bug#931977: startpar: uninitialized variable
I have added an initialization for the "tail" variable. I also make the error more verbose and put in an exit() call in case we are unable to allocate any memory. It's a bit crude, but I figure if we ever run into a situation where we can't allocate a tiny structure, the system has bigger issues to deal with. In this case I don't think malloc(sizeof(*tail)) will work as that would only allocate the size of the pointer rather than the full structure (struct console). In principle, I agree malloc() is the cleaner function, but I'm going to leave the original as it seems to be doing the job without any problems. This bug will be fixed upstream in 0.64. - Jesse
Bug#549260: Appears to be fixed
This issue appears to be fixed in recent versions of Debian, such as Jessie and Stretch. Suggest it can be closed. - Jesse
Bug#619409: insserv: /etc/init.d/.depend.* represent state, not configuration; please move to /lib
On Wed, 23 Mar 2011 09:33:46 -0700 Josh Triplett wrote: > > insserv generates makefile snippets in /etc/init.d/.depend.* with > dependency information for init scripts. This doesn't represent > configuration information, nor can the user modify it. Thus, it should > go somewhere else on the root filesystem, such as /lib/insserv/depend.* > .. That would avoid the need to ignore them in etckeeper, or care about > the differences between successive versions. > After some consideration and debate, I believe this bug should be closed as wontfix. For several reasons: 1. The makefiel-style dependency information is configuration data, not a library or executable, so it doesn't belong in /lib. It may not belong in /etc, but /lib would be a clear violation of FHS. 2. Despite the above claim, it is configuration information. 3. Despite the initial concern, the user can modify the data, it's completely editable and human-readable. I'm open to trying another location that might better suite everyone's needs and FHS, but /lib is not a good match. - Jesse
Bug#601054: Move /etc/init.d/.depend.* files under /var
As an update on this bug, since /var may not be mounted at boot time, causing startpar to fail, this change has been reverted upstream. I am going to suggest this bug can be closed. - Jesse
Bug#929063: init: delegate selinux operation to separate binary
On Thu, 23 May 2019 16:27:00 + Dmitry Bogatov wrote: > > > Also, every WITH_FOO flag doubles number of configurations your program > have. Once you have dozen of flags, you no longer can test all of > configurations. Why not? > > I am surprised, that there is so much controversy on whether it is good > to have some feature of program pluggable without re-compilation. The > only real concern that was raised, as I see it, is how SELinux interacts > with extra fork/exec. The proposed patch doesn't seem to make the feature pluggable, it just moves the feature to a second binary. There isn't anything here which toggles the feature on/off, or lets the admin enable/disable it. That being said, I'd be open to applying this upstream, with four conditions or changes: 1. The selinux-check binary should probably be renamed so it is clear this program is part of the SysV init suite. Maybe calling it something like /sbin/sysvinit-selinux-check? I'm open to suggestions as to what it should be called. 2. The patch should respect the compile-time define WITH_SELINUX in both init.c and in the check-selinux.c code. Otherwise the build will either fail or attempt to perform unnecessary fork/exec on systems without SELinux, like Hurd and kFreeBSD. In fact, the Makefile should skip building & installing check-selinux.c completely if SELinux is not available on that platform. Otherwise we are making things more resource hungry and complex on systems where SELinux is not included, rather than less. 3. The new binary/helper program should have a man page. 4. There should be a test or script presented that demonstrates SELinux still works properly with this approach. I don't want to open a security issue by moving SELinux support to a non-PID1 binary.
Bug#929063: init: delegate selinux operation to separate binary
On Wed, 22 May 2019 13:28:39 +0200 (CEST) Thorsten Glaser wrote: > > (I’m not quite convinced the effort is worth it, but given that > this would be changed upstream, and that there are likely other > users of the same upstream code who’re _not_ using SELinux, this > would be very welcomed by those, so I’m okay with it.) I'd like to point out that init already has compile-time defines in the code which check for the existence of SELinux (using the variable WITH_SELINUX). If WITH_SELINUX is not defined at compile time, then the SELinux code isn't built into init. So other projects, perhaps Debian Hurd or FreeBSD, can already build init without SELinux features.
Bug#929063: Moving SELinux check
On 5/22/19 11:57 AM, Thorsten Glaser wrote: > On Wed, 22 May 2019, Jesse Smith wrote: > >> I don't think removing the SELinux dependency from init actually saves >> us any RAM. Several other services link to these libraries too, so the > Maybe, maybe not. (I’m fairly sure I’ve got some VMs without.) > > Other services can, however, be more easily restarted than the entire > system, in case of a security fix for that library. > > How do you think an attacker would exploit a flaw in a SELinux library through init? SysV init doesn't interact with the user, doesn't read any files directly after it's up and running, doesn't listen on any sockets. About the only way to interact with PID1 is through a pipe that can only be written to by root.
Bug#929063: Moving SELinux check
On 5/21/19 8:45 PM, Dmitry Bogatov wrote: > [2019-05-18 16:14] Jesse Smith >> From a practical perspective, I'm curious if there is any benefit or >> drawback. Is this patch fixing a known bug, >> does it significantly reduce the size of PID 1 in memory? > Not that I really care about 1Mb of RAM, but: > > 152K /lib/x86_64-linux-gnu/libselinux.so.1 > 692K /lib/x86_64-linux-gnu/libsepol.so.1 > 460K /lib/x86_64-linux-gnu/libpcre.so.3.13.3 I don't think removing the SELinux dependency from init actually saves us any RAM. Several other services link to these libraries too, so the libraries are loaded into RAM anyway and should be shared between the various services. Unless SELinux is culled from every low-level daemon that 1MB RAM is still going to be used.
Bug#619409: insserv: /etc/init.d/.depend.* represent state, not configuration; please move to /lib
I brought this up in bug #601054 as well, but thought I'd mention it here since this seems to be the same issue. Upstream we're experimenting with using /var/lib/insserv/ as the location for storing boot-time dependency data. However, I'm not sure we can rely on /var being mounted when startpar runs. Which makes me this this bug, which requests the data be placed under /lib/insserv/ might be the better approach. Thoughts on this? I'm not sure it's safe to put boot-time configuration data under /var, but I'm not sure if it's a practical concern or just a theoretical one. - Jesse
Bug#601054: Move /etc/init.d/.depend.* files under /var
This has been done upstream and is now in the latest beta release [1]. Something I'm curious about is: what happens if /var is not mounted when startpar runs? Let's say /var is on another disk, or mounted over NFS. What then? I'm guessing some people are using a setup where /var is not on the root file system, so what kind of fallback is in place to make sure insserv and startpar work when their files are stored under /var instead of /etc? Another bug report against insserv suggested we use /lib/insserv/ instead of /var/lib/insserv/. Maybe we should switch the location before the next stable release, since I'm pretty sure /lib needs to be part of the root filesystem. - Jesse 1 - http://lists.nongnu.org/archive/html/sysvinit-devel/2019-05/msg2.html
Bug#929063: Moving SELinux check
I've looked over the patch and the logic seems straight forward enough. Philosophically, I can see arguments for doing this (simplify the core of init, remove a dependency) and against this idea (it adds a new program to the sysvinit package and start-up process). So from a philosophical stand point I'm fairly neutral on this new approach. >From a practical perspective, I'm curious if there is any benefit or drawback. Is this patch fixing a known bug, does it significantly reduce the size of PID 1 in memory? Is there a flaw in libselinux is known to cause problems if init is linked to it? I'd like to hear some options on why we might apply this (or not) upstream. - Jesse
Bug#622878: insserv: support user-configurable ignored filename suffixes
This feature has been added upstream and will be featured in insserv-1.20.0. The manual page will provide instructions for how to set up file extensions to be filtered automatically when insserv runs. - Jesse
Bug#695751: depending on a script that depends on $all (including by not having LSB headers) creates loop
I believe this issue has been fixed. We have a test script which checks for script dependencies with "$all" and it passes. This report can probably be closed. - Jesse
Bug#544229: insserv should require virtual facilities to expand to at least one facility when they are in required-{start,stop}
I'm not sure I agree that insserv should completely abort when an unmet virtual dependency is unmet. There could be cases where one facility is missing, but its function is provided by another service. I do agree though that insserv should not silently fail. With this in mind, I have patched insserv upstream to print a warning when a virtual facility is missing. This lets us know there is a potential problem without forcing a specific service to be installed for the initscript to work. The fix will be available in insserv-1.20.0. - Jesse
Bug#926547: insserv: tests/run-tests are not used
I went through the test suite and identified places where either the test suite was probably out of date (or working with old documentation) or insserv was giving improper results. Most of the test cases which were failing can, I believe, be addressed by warnings rather than outright failures or blocking. A missing virtually facility, for example, could be provided by another script and results in a warning rather than insserv refusing to add the script to a runlevel. I feel this is the more kind approach. The end result is running "make check" in the insserv source tree now runs the test suite. If everything goes well (which is does on my machine) the check completes successfully and prints a summary. If an error is encountered, the test suite should bomb out with a warning and leave the test data intact so it can be reviewed. (In the past test data was erased, making it harder to investigate.) The test suite now works with insserv's new output directory location (/var/lib/insserv) so that's good news. Running "make distclean" cleans up any test data. I think this bug can probably be marked as fixed upstream. I hope to publish a new beta around the end of May and a new stable release of insserv in June. With that release of insserv-1.20.0 we should be able to properly close this bug. - Jesse
Bug#926547: insserv: tests/run-tests are not used
On 5/1/19 7:42 AM, Dmitry Bogatov wrote: > [2019-04-28 21:05] Jesse Smith >> I have been looking into the issues with the insserv test scripts. There >> are a few problems here, in brief: >> [...] >> >> In other words, I think there is some difference in expectations between >> what I think insserv should be doing and what the scripts we inherited >> from downstream think insserv should be doing. I'm interested in getting >> some feedback on this. If a script has no LSB header, should it be given >> preset defaults, or should it be ignored? If the latter, I think we can >> remove this series of tests. Which will mean there are just two errors >> left for me to sort out. > In my opinion, insserv should print warning about missing LSB headers > and ignore them. I agree, which is good, that is what insserv currently does during the test. I'll mark this test as "passed".
Bug#926547: insserv: tests/run-tests are not used
I have been looking into the issues with the insserv test scripts. There are a few problems here, in brief: 1. The test scripts were looking in the old area for insserv output and missing some dependency information that way. I should have updated them earlier. (This has been fixed.) 2. The scripts were not well integrated into the Makefile process. That is to say running "make check" and "make distclean" should have been handling the scripts better. (This has also been fixed.) 3. The scripts were causing insserv to try to overwrite the host system's start-up dependency information in some instances, which could cause problems if run as root/sudo. (This seems unlikely to happen, but I've fixed this too.) 4. Some of the tests are expecting output which is different than what insserv provides and, possibly, should provide. For instance, some scripted tests fail if scripts without LSB headers are not inserted into runlevels in a certain order. This one seems strange to me because, at the moment, insserv does not try to place non-LSB scripts in runlevels in any order, considering it something to be skipped over. In other words, I think there is some difference in expectations between what I think insserv should be doing and what the scripts we inherited from downstream think insserv should be doing. I'm interested in getting some feedback on this. If a script has no LSB header, should it be given preset defaults, or should it be ignored? If the latter, I think we can remove this series of tests. Which will mean there are just two errors left for me to sort out. - Jesse
Bug#926547: insserv: tests/run-tests are not used
Thanks for the ping, I did not see this one come in for some reason. I have added the testsuite to the Makefile's "check" target and addressed the unset variable issue. Going through the tests, figuring out which ones are failing and why will take a little longer. Or, for that matter, there may be cases where failing is a good thing as some of the tests seem to be dealing with situations where the data is problematic and we need to bomb out. I'll check. - Jesse
Bug#538304: insserv: start and stop in same runlevel
On 4/11/19 7:43 AM, Dmitry Bogatov wrote: > [2019-01-24 19:42] Jesse Smith >> I'm looking into this bug in insserv and want to make sure I'm >> understanding the issue correctly. As I read it, the problem is that if >> the user specifies the same runlevel in both the Default-Start and >> Default-Stop fields, insserv will set up the "Stop" symbolic link, but >> will not complain about it? >> >> Ideally, insserv should probably still use this behaviour, but print a >> warning that the same runlevels should not be listed in both the >> Default-Start and Default-Stop fields. Is this a correct summary of the >> above request? > Yes, seems so. I have addressed this upstream. The next version of insserv will print a warning when an init.d script has the same runlevel specified in the Default-Start and Default-Stop fields. The output looks like this: insserv: Script bluetooth has overlapping Default-Start and Default-Stop runlevels (2 3 4 5) and (0 1 3 6). This should be fixed. Apart from the warning, insserv functions as before. - Jesse
Bug#711853: insserv: Design bug: rcN.d unstable and not, maintainable
>> When update-rc.d calls insserv, the rcN.d directories are rebuilt >> without taking into consideration any adjustment that might have >> been set up locally. That seems to be done on the assumption that >> the dependencies coded in the LSB blocks are universally accurate. >> Now, LSBs are a great improvement over numeric priorities, but to >> hamper local system tuning, which is not necessarily related to >> LSBs, IMHO is an insult to the legendary versatility of SysV init. > > I believe it is not how things work now. Last time I checked, call > `insserv foo` does not update any links to `foo' if there is already at > least one of them. > > @Jesse, am I right? I just ran a quick test and can confirm that if I have an existing link to a service, for example /etc/rc5.d/S05bluetooth, then running the command "insserv bluetooth" will attempt to remove the old symlink and replace it with one that conforms to the LSB headers. In my case, removing the original link and creating /etc/rc5.d/S04bluetooth. Now, as to whether this should be considered a bug or a desired effect is open to debate. On the one hand it is understandable people might not want insserv to overwrite their changes. On the other hand, in my case insserv is fixing a mistake in my symlinks, and adjusting them to match their LSB headers. My thought on this is if someone wants to improve their start-up routine it makes more sense for them to edit their script's LSB header and re-run insserv rather than editing links by hand.
Bug#374039: #374039 shutdown -h -H: warn that then cannot poweroff perhaps
On 4/8/19 12:38 PM, Dmitry Bogatov wrote: > control: tags -1 +upstream > > [ Please keep attribution ] > > [2019-04-07 11:12] Jesse Smith >> >> That is what halt means - to stop running the system without powering >> off. > Maybe, but many of us are accustomed that /sbin/halt turns off the computer, > so here comes confusion. That is certainly true, but I'd like to point out that /sbin/halt only turns off the computer because Debian modifies halt's behaviour. If you run /sbin/halt without Debian's modifications, the traditional action (stop without powering off) occurs. I'd almost consider this a bug since /sbin/halt should be used to stop the system while /sbin/poweroff should be used to, well, turn off the power to the system. >> Halting is often used to run through the shutdown process and leave >> output on the screen for debugging purposes. Or when you want the OS to >> stop, but leave the power on. There is no negative side-effect to using >> the -H option, no loss of data. There isn't any reason to print an extra >> warning. > Okay, what about including this explanation into manpage? I know, Unix > is about sharp tools, but before I started working on sysvinit, I > believed, that "halt == turn-off", so extra explanation, that it is > different things would be nice to user. I'm in favour of this change and can expand on the explanation in the manual page for the next release.
Bug#374039: #374039 shutdown -h -H: warn that then cannot poweroff perhaps
> Halt action is to halt or drop into boot monitor on systems that > support it." is not enough to convey, that in many cases it brings > machine into state, when it is still on, display still showing > letters, but no interation (except physical poweroff) is possible. That is what halt means - to stop running the system without powering off. > "Maybe -H is actually produces useful behaviour in some cases (no > idea), but please add into manpage warning like "Do not use -H option, > unless you really know what are you doing." Halting is often used to run through the shutdown process and leave output on the screen for debugging purposes. Or when you want the OS to stop, but leave the power on. There is no negative side-effect to using the -H option, no loss of data. There isn't any reason to print an extra warning. Which brings me back to the original bug report which read: "Well, I tried "-h -H", and guess what, it put my IBM Thinkpad in a unrebootable state... no button would respond, no way to poweroff either. I had to remove the AC cord, and remove the battery, to finally poweroff. So perhaps add a warning to the man page." The user could, in that case, just press and hold the power button for five seconds to power off the machine. There is no need to remove the AC power or take out the battery. Holding the power button works on all workstations and laptops made in the past 20 years. It's common practise for any time a machine hangs, has a kernel panic or stops responding to keyboard input.
Bug#924792: pidof: unsanitized user input makes pidof crash
I have removed the "-f" flag for formatting (and the custom string substitution patch). It has been replaced by the patch from KatolaZ which allows for a custom field separator. This has been applied upstream in the 2.95 branch. - Jesse
Bug#924792: pidof: unsanitized user input makes pidof crash
On 3/19/19 7:28 PM, KatolaZ wrote: > On Tue, Mar 19, 2019 at 05:54:56PM -0300, Jesse Smith wrote: >> I am certainly open to replacing the "format" flag (-f) with an >> alternative field separator flag. It has a nice Unixy feel to it and >> requires less code. >> >> Personally, I think using -d (or --delimiter) might be the only change >> I'd want to make to the patch KatolaZ provided. Partly because pidof >> already has a lower-case "-s" flag and I want to avoid confusion, and >> because tools like cut also use "-d". >> >> If there are no objections, I'll upstream KatolaZ's patch and remove the >> "-f" flag. >> > No objections on my side. '-d' makes sense to me as well. > > @Jesse: I feel I owe you an apology. I read again my first post in > this thread, and I noticed that it might have sounded harsh, or worse, > patronising. Sorry: that was never my intention. I hope you can > accept my apologies. > Not to worry, and no apology necessary. You raised a valid point about over-engineering pidof and made some constructive criticism. I appreciate you helping to come up with a better solution. Jesse
Bug#924792: pidof: unsanitized user input makes pidof crash
I am certainly open to replacing the "format" flag (-f) with an alternative field separator flag. It has a nice Unixy feel to it and requires less code. Personally, I think using -d (or --delimiter) might be the only change I'd want to make to the patch KatolaZ provided. Partly because pidof already has a lower-case "-s" flag and I want to avoid confusion, and because tools like cut also use "-d". If there are no objections, I'll upstream KatolaZ's patch and remove the "-f" flag. Jesse
Bug#924792: pidof: unsanitized user input makes pidof crash
I have been playing around with this a little and believe I have come up
with a workable solution. The attached patch causes the passed in format
string to be dumbed down so that we only translate instances of %d into
the PID and \n into newline characters. Everything else is treated as a
literal part of the string.
This effectively should neutralize any use of %s %c %f etc to cause a
segfault or dump memory. (Hopefully.)
It means the output string cannot have fancy formatting, but that seems
like a job better suited to sed or awk anyway.
I have tested this on a handful of formatted strings and it seems to fix
the original problem. I'm open to improvements or fixing other corner
cases I may have missed. Assuming people are more or less happy with
this fix it will get applied upstream.
Jesse
diff --git a/man/pidof.8 b/man/pidof.8
index a4295b9..4e83492 100644
--- a/man/pidof.8
+++ b/man/pidof.8
@@ -69,8 +69,10 @@ Tells \fIpidof\fP to omit processes with that process id.
The special
pid \fB%PPID\fP can be used to name the parent process of the \fIpidof\fP
program, in other words the calling shell or shell script.
.IP "-f \fIformat\fP"
-Tells \fIpidof\fP to format the process ids in the given \fIprintf\fP style.
+Tells \fIpidof\fP to format the process IDs in the given \fIprintf\fP style
string.
For example \fB" -p%d"\fP is useful for \fIstrace\fP.
+The "%d" symbol is used as a place holder for the PID to be printed. A "\\n"
can
+be used to cause a newline to be printed. For example \fB" %d\\n"\fP.
.SH "EXIT STATUS"
.TP
.B 0
diff --git a/src/killall5.c b/src/killall5.c
index 8f2ad50..375b4a7 100644
--- a/src/killall5.c
+++ b/src/killall5.c
@@ -985,6 +985,66 @@ void nsyslog(int pri, char *fmt, ...)
#define PIDOF_NETFS0x04
#define PIDOF_QUIET 0x08
+
+/* Replace elements (from) of the original string
+ with new elements (to).
+ Returns the new string on success or NULL on failure.
+ Free the returnedstring after use.
+*/
+char *Replace_String(char *from, char *to, char *original)
+{
+int from_length, to_length;
+int source_length, destination_length;
+int replace_count = 0;
+char *replace_position;
+char *destination_string;
+char *source_position, *destination_position;
+
+if ( (! from) || (! to) || (! original) )
+ return NULL;
+
+from_length = strlen(from);
+to_length = strlen(to);
+source_length = strlen(original);
+replace_position = strstr(original, from);
+/* There is nothing to replace, return original string */
+if (! replace_position)
+return strdup(original);
+/* count number of times we need to perform replacement */
+while (replace_position)
+{
+replace_count++;
+replace_position++;
+replace_position = strstr(replace_position, from);
+}
+/* calculate length and allocate the new string */
+destination_length = source_length + ( (to_length - from_length) *
replace_count);
+destination_string = calloc(destination_length, sizeof(char));
+if (! destination_string)
+ return NULL;
+
+/* Copy source string up to the part we need to replace. Then jump over
the replaced bit */
+source_position = original;
+destination_position = destination_string;
+replace_position = strstr(original, from);
+while (replace_position)
+{
+for ( ; source_position < replace_position; source_position++)
+{
+destination_position[0] = source_position[0];
+destination_position++;
+}
+strcat(destination_position, to);
+source_position += from_length;
+destination_position += to_length;
+replace_position = strstr(source_position, from);
+}
+/* Replaced all the items, now copy the tail end of the original string */
+strcat(destination_position, source_position);
+return destination_string;
+}
+
+
/*
* Pidof functionality.
*/
@@ -1119,7 +1179,22 @@ int main_pidof(int argc, char **argv)
if ( ~flags & PIDOF_QUIET ) {
if (format)
- printf(format, p->pid);
+{
+ char *show_string, *final_string;
+ char my_pid[32];
+ snprintf(my_pid, 32, "%d", p->pid);
+ show_string = Replace_String("%d",
my_pid, format);
+ final_string = Replace_String("\\n",
"\n", show_string);
+ if (show_string) free(show_string);
+ if (final_string)
+ {
+ printf("%s", final_string);
+ free(final_string);
+ }
+
Bug#924792: pidof: unsanitized user input makes pidof crash
> Wouldn't you need to have some process which was passing untrusted data > directly to the `-f` argument, is that likely in the real world? It may not be likely, but anything that makes a command line tool crash or output weird data after being fed unfiltered command line input is not a good situation. I could see a situation where this might be exploited in a script to give bad results or kill the wrong process. So it's probably low risk, but I'd like to reduce that risk further.
