Bug#584102: [sysvinit-devel] startpar: Fail if fd 0 is strange tty
On Fri, Jun 04, 2010 at 10:18:18AM +0200, Petter Reinholdtsen wrote: Hi, Werner. Not sure where to send startpar issues, so I try here as I know I can reach you here. :) I proposed a patch for URL: http://bugs.debian.org/584102 , basicly allowing startpar to continue even when tcgetattr(0, tio) by running scripts sequencially. Can you have a look at let me know if it make sense. This issue is also reported as URL: http://savannah.nongnu.org/bugs/?30034 . Btw, why do startpar need stdin to be a tty? That is a guess but nevertheless the attributes are used to set them also to the pty/tty pairs used for parallel boot. As stdout of startpar can be used for a shell script parser (without option -M) the stdio was guessed to be a real tty. Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#584102: startpar: Fail if fd 0 is strange tty
On Mon, Jun 07, 2010 at 04:49:39PM +0200, Petter Reinholdtsen wrote: [Dr. Werner Fink] That is a guess but nevertheless the attributes are used to set them also to the pty/tty pairs used for parallel boot. As stdout of startpar can be used for a shell script parser (without option -M) the stdio was guessed to be a real tty. Right. Any idea how to get parallel booting working also when stdin and stdout is a pipe, like it is with OpenVZ at the moment? Hmmm ... stderr maybe, something like FILE* io[3]; int notty = 1; io[0] = stdin; io[1] = stdout; io[2] = stderr; io[3] = NULL; for (n=0; io[n]; n++) { int fd = isatty(fileno(io[n])); if (tcgetattr(fd, tio) == 0) { notty = 0 break; } } could work. Btw: I've lost your mail Alternative startpar implementation - live-net-startpar please resend. Beside any startpar it could be an option to use a directory based boot scheme as insserv aloready uses a tsort which can be mapped on `directory' based sorting scheme. Could work simliar like the process_path() routine of simpleinit ;) Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#582442: [sysvinit-devel] (fwd) Bug#582442: sysvinit: Failing to interrupt some script after upgrade
On Fri, May 21, 2010 at 11:37:19AM +0200, Petter Reinholdtsen wrote: Werner, any idea how startpar should handle ^c? This is the report in URL: http://bugs.debian.org/582442 . since last upgrade (makefile style migration ?) I got strange behaviour of the boot sequence. If I interupt some script with ctrl+c, the dependency are lost (not fs mounted, no network, ...). With the previous version this worked. I interrupt sometimes fsck (when I don't want to wait), now this doesn't work. Also on my system udev script hang at the end (until there is a timeout). I often hit ctrl+c to avoid waiting the timeout. This now make the system not usable. If ctrl+c is not supported anymore, it should blocked to avoid this strange behaviour. I tested this, and pressing ^c while rcS.d/ scripts are executed break the entire boot. Normally startpar inherit the signal handling of its parent. And it does reset the signals INT, QUIT, SEGV, TERM, and CHLD to the default for each of its own childs. Maybe using a trap on e.g. SIGINT will help to make startpar more tough against Ctrl-C. IMHO Ctrl-C on a running fsck may cause real trouble (in my own experience I've seen corrupted file systems). If such a trap does not work we may set signal handlers for startpar to avoid that startpar its self is stopped. Nevertheless the question is do we want to interrupt the current jobs. Please remember that the messages seen on screen are not in sync with the execution progresses of the jobs. Those messages are buffered to avoid extremly mixed messages. Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#580272: [sysvinit-devel] sysvinit do not enable SELinux when it should
On Fri, May 07, 2010 at 08:25:28AM +0200, Petter Reinholdtsen wrote: According to URL: http://bugs.debian.org/580272 , the sysvinit code to enable SELinux is broken. Werner, you implemented the current version. Do you have any idea how it should be fixed? The only change between the old version is the check for the return value of is_selinux_enabled() ... here the old code: if (getenv(SELINUX_INIT) == NULL !is_selinux_enabled()) { putenv(SELINUX_INIT=YES); if (selinux_init_load_policy(enforce) == 0 ) { execv(myname, argv); } else { if (enforce 0) { /* SELinux in enforcing mode but load_policy failed */ /* At this point, we probably can't open /dev/console, so log() won't work */ printf(Unable to load SELinux Policy. Machine is in enforcing mode. Halting now.\n); exit(1); } } } and now the new code if (getenv(SELINUX_INIT) == NULL) { const int rc = mount(proc, /proc, proc, 0, 0); if (is_selinux_enabled() 0) { putenv(SELINUX_INIT=YES); if (rc == 0) umount2(/proc, MNT_DETACH); if (selinux_init_load_policy(enforce) == 0) { execv(myname, argv); } else { if (enforce 0) { /* SELinux in enforcing mode but load_policy failed */ /* At this point, we probably can't open /dev/console, so log() won't work */ fprintf(stderr,Unable to load SELinux Policy. Machine is in enforcing mode. Halting now.\n); exit(1); } } } if (rc == 0) umount2(/proc, MNT_DETACH); } as it can be seen the check of the return value of selinux_init_load_policy() has not changed but the check of the return value of is_selinux_enabled() this was done due a bug report as is_selinux_enabled() may return -1 on an error (not mounted /proc due not using initrd and the resulting `!-1' leads to a not loaded policy. Just read the short manual page of is_selinux_enabled(3): is_selinux_enabled(3) SELinux API documentation is_selinux_enabled(3) NAME is_selinux_enabled - check whether SELinux is enabled NAME is_selinux_mls_enabled - check whether SELinux is enabled for (Multi Level Securty) MLS SYNOPSIS #include selinux/selinux.h int is_selinux_enabled(); int is_selinux_mls_enabled(); DESCRIPTION is_selinux_enabled returns 1 if SELinux is running or 0 if it is not. is_selinux_mls_enabled returns 1 if SELinux is running in MLS mode or 0 if it is not. SEE ALSO selinux(8) russ...@coker.com.au1 January 2004 is_selinux_enabled(3) and in the source code of I've found that in case of /proc is not mounted the function is_selinux_enabled(3) indeed also returns a -1 (or better if not able to open /proc/filesystems for reading). As selinux_init_load_policy() does also mounting the selinuxfs I guess that we should check for is_selinux_enabled() == 0 Martin? Does this works for you? Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#580272: sysvinit: Does not load SELinux policy
On Fri, May 07, 2010 at 10:17:53AM +0200, Petter Reinholdtsen wrote: [Martin Orr] I am happy to test things. I shall ask on the SELinux list and with init upstream what init should be happening here. Great. I have already asked on the upstream mailing list, see URL: http://lists.nongnu.org/archive/html/sysvinit-devel/2010-05/msg0.html . I'd like to see if it makes a difference to change if (is_selinux_enabled() 0) { to if (is_selinux_enabled() == 0) { as this may enforce the call of selinux_init_load_policy() Martin? Does this change work for you? Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#580564: [sysvinit-devel] insserv no longer understand X-Interactive: true
On Fri, May 07, 2010 at 08:19:20AM +0200, Petter Reinholdtsen wrote: Hi, Werner. Take a look at URL: http://bugs.debian.org/580564 . I've verified this with our test suite, and insserv no longer understand the X-Interactive: true header. This test suite function fail: Please try out the attached patch Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr Index: insserv.c === --- insserv.c (revision 88) +++ insserv.c (working copy) @@ -1388,10 +1388,10 @@ description = empty; } - if (!interactive regexecutor(reg.interact, COMMON_ARGS) == true) { - if (val-rm_so val-rm_eo) { - *(pbuf+val-rm_eo) = '\0'; - interactive = xstrdup(pbuf+val-rm_so); + if (!interactive regexecutor(reg.interact, COMMON_SHD_ARGS) == true) { + if (shl-rm_so shl-rm_eo) { + *(pbuf+shl-rm_eo) = '\0'; + interactive = xstrdup(pbuf+shl-rm_so); } else interactive = empty; }
Bug#542043: insserv: please enable an $all like facility for stop ordering
On Wed, Aug 19, 2009 at 12:10:16PM +0200, Michael Meskes wrote: I'm sorry, my last report was incorrect. I only checked runlevel 1 for which the links were okay. In 0 and 6 however I still have alsa-utils, cryptdisks, fuse, urandom and wpa-ifdown in K01 although none of them uses $all. Hmmm ... if the script using $all is not within the same runlevels this may happens but should not hurt. The topological sort is only applied if the scripts/services uses the same runlevels. This is done by bit masks were each bit is representing a specific runlevel. Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#542043: insserv: please enable an $all like facility for stop ordering
On Wed, Aug 19, 2009 at 01:45:36PM +0200, Michael Meskes wrote: On Wed, Aug 19, 2009 at 12:27:34PM +0200, Dr. Werner Fink wrote: On Wed, Aug 19, 2009 at 12:10:16PM +0200, Michael Meskes wrote: I'm sorry, my last report was incorrect. I only checked runlevel 1 for which the links were okay. In 0 and 6 however I still have alsa-utils, cryptdisks, fuse, urandom and wpa-ifdown in K01 although none of them uses $all. Hmmm ... if the script using $all is not within the same runlevels this may happens but should not hurt. The topological sort is only applied if the scripts/services uses the same runlevels. This is done by bit masks were each bit is representing a specific runlevel. Seems I wasn't precise enough. Of course I also have K01watchdog in levels 0 and 6. Watchdog's init script has: # Required-Stop: $all AFAICT this should make watchdog the only script in K01, wouldn't it? Yep ... please try out the attached patch. Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr --- insserv.c +++ insserv.c 2009-08-19 13:54:39.694401388 +0200 @@ -653,7 +653,7 @@ static inline void all_script(void) if (cur-attr.flags SERV_DUPLET) continue; /* Duplet */ - if ((serv-start-lvl cur-start-lvl) == 0) + if ((serv-stopp-lvl cur-stopp-lvl) == 0) continue; if (cur == serv)
Bug#542043: insserv: please enable an $all like facility for stop ordering
On Wed, Aug 19, 2009 at 01:56:32PM +0200, Werner Fink wrote: On Wed, Aug 19, 2009 at 01:45:36PM +0200, Michael Meskes wrote: On Wed, Aug 19, 2009 at 12:27:34PM +0200, Dr. Werner Fink wrote: On Wed, Aug 19, 2009 at 12:10:16PM +0200, Michael Meskes wrote: I'm sorry, my last report was incorrect. I only checked runlevel 1 for which the links were okay. In 0 and 6 however I still have alsa-utils, cryptdisks, fuse, urandom and wpa-ifdown in K01 although none of them uses $all. Hmmm ... if the script using $all is not within the same runlevels this may happens but should not hurt. The topological sort is only applied if the scripts/services uses the same runlevels. This is done by bit masks were each bit is representing a specific runlevel. Seems I wasn't precise enough. Of course I also have K01watchdog in levels 0 and 6. Watchdog's init script has: # Required-Stop: $all AFAICT this should make watchdog the only script in K01, wouldn't it? Yep ... please try out the attached patch. Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr --- insserv.c +++ insserv.c 2009-08-19 13:54:39.694401388 +0200 @@ -653,7 +653,7 @@ static inline void all_script(void) if (cur-attr.flags SERV_DUPLET) continue; /* Duplet */ - if ((serv-start-lvl cur-start-lvl) == 0) + if ((serv-stopp-lvl cur-stopp-lvl) == 0) continue; if (cur == serv) The patch is for a test 1.13.0 ... the affected line is in first_script(void) in your version and not in all_script(void). Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#541613: insserv: infinite loop with memory leakage when a virtual facility depends on itself
On Mon, Aug 17, 2009 at 02:25:13PM +0200, Werner Fink wrote: On Sat, Aug 15, 2009 at 05:19:41PM +0200, Petter Reinholdtsen wrote: [Raphael Geissert] As mentioned on IRC, when a virtual facility depends on itself insserv enters an infinite loop and starts leaking memory. Thank you. I've written this test case to reproduce the problem. CC to upstream to let him know about the problem. OK, I've added a check to avoid such loops and also check to avoid that a scripts depends on a scripts which uses the system facility `$all' in its Required-Start. Does this work for you? It hadn't worked for my on x86_64 due not initialized flags. Please apply the following patch on top of the last patched version to fix this. Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr --- insserv.c +++ insserv.c 2009-08-18 11:36:07.0 +0200 @@ -222,8 +222,8 @@ typedef struct string { typedef struct repl { list_t r_list; -ushort flags; string_t r[1]; +ushort flags; } __align repl_t; #define getrepl(arg) list_entry((arg), struct repl, r_list) @@ -1880,6 +1880,7 @@ static void scan_conf_file(const char *r if (posix_memalign((void*)subst, sizeof(void*), alignof(repl_t)) != 0) error(%s, strerror(errno)); insert(subst-r_list, r_list-prev); +subst-flags = 0; r = subst-r[0]; if (posix_memalign((void*)r-ref, sizeof(void*), alignof(typeof(r-ref))+strsize(token)) != 0) error(%s, strerror(errno)); @@ -1909,6 +1910,7 @@ static void scan_conf_file(const char *r error(%s, strerror(errno)); insert(subst-r_list, r_list-prev); r = subst-r[0]; + subst-flags = 0; if (posix_memalign((void*)r-ref, sizeof(void*), alignof(typeof(r-ref))+strsize(token)) != 0) error(%s, strerror(errno)); *r-ref = 1;
Bug#542043: insserv: please enable an $all like facility for stop ordering
On Mon, Aug 17, 2009 at 05:47:58PM +0200, Werner Fink wrote: On Mon, Aug 17, 2009 at 05:41:23PM +0200, Petter Reinholdtsen wrote: [Michael Meskes] Just imagine a logger that is supposed to log all actions done while shutting down the system or, as in my case, a daemon whose job it is to monitor other daemons and, if they do not run anymore, either restart that daemon or the whole system. It's fairly easy to make the start action run late enough because there's a hierarchy already there. But it is almost impossible to stop this daemon first without listing all ~1000 facilities as to be stopped later. Werner, what is your opinion on allowing $all to work also for stop dependencies, to be able to stop scripts very early during shutdown? Michael Meskes ran into the need with the watchdog package, where he want to stop the service monitoring before any services are stopped. Hmmm ... this is already on my todo but requires to move already sorted scripts in the hierarchy. Or I increase the order nummer before sorting. Let's see ... how does this patch works for you? Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr --- insserv.c +++ insserv.c 2009-08-18 13:26:15.805902273 +0200 @@ -300,7 +300,10 @@ static void rememberreq(service_t * rest if (strcasecmp(token, $null) == 0) break; if (strcasecmp(token, $all) == 0) { - serv-attr.flags |= SERV_ALL; + if (bit REQ_KILL) + serv-attr.flags |= SERV_FIRST; + else + serv-attr.flags |= SERV_ALL; break; } /* Expand the `$' token recursively down */ @@ -622,8 +625,7 @@ static inline void active_script(void) } /* - * Last but not least the `$all' scripts will be set to the - * end of the current start order. + * The `$all' scripts will be set to the end of the current start order. */ static inline void all_script(void) attribute((always_inline)); static inline void all_script(void) @@ -680,6 +682,50 @@ static inline void all_script(void) } /* + * Last but not least the `$all' scripts will be set to the + * beginning of the current stop order. + */ +static inline void first_script(void) attribute((always_inline)); +static inline void first_script(void) +{ +list_t * pos; + +list_for_each(pos, s_start) { + service_t * serv = getservice(pos); + list_t * tmp; + + if (serv-attr.flags SERV_DUPLET) + continue; /* Duplet */ + + if (!(serv-attr.flags SERV_FIRST)) + continue; + + if (serv-attr.script == (char*)0) + continue; + + list_for_each(tmp, s_start) { + service_t * cur = getservice(tmp); + + if (cur-attr.flags SERV_DUPLET) + continue; /* Duplet */ + + if ((serv-start-lvl cur-start-lvl) == 0) + continue; + + if (cur == serv) + continue; + + if (cur-attr.flags SERV_FIRST) + continue; + + rememberreq(serv, REQ_SHLD|REQ_KILL, cur-name); + } + + setorder(serv-attr.script, 'K', 1, false); +} +} + +/* * Make the dependency files */ static inline void makedep(void) attribute((always_inline)); @@ -3194,6 +3240,11 @@ int main (int argc, char *argv[]) nonlsb_script(); /* + * Move the `$all' stop scripts to the very beginning + */ +first_script(); + +/* * Now generate for all scripts the dependencies */ follow_all(); --- listing.c +++ listing.c 2009-08-18 13:53:47.693917907 +0200 @@ -317,6 +317,7 @@ static void __follow (dir_t *restrict di } for (tmp = dir; tmp; tmp = getnextlink(l_list)) { + const typeof(attof(tmp)-flags) sflags = attof(tmp)-flags; register boolean recursion = true; handle_t * ptmp = (mode == 'K') ? tmp-stopp : tmp-start; uchar * order = ptmp-deep; @@ -382,6 +383,7 @@ static void __follow (dir_t *restrict di np_list_for_each(dent, l_list) { dir_t * target = getlink(dent)-target; handle_t * ptrg = (mode == 'K') ? target-stopp : target-start; + const typeof(attof(target)-flags) kflags = attof(target)-flags; if ((peg-run.lvl ptrg-run.lvl) == 0) continue; /* Not same boot level */ @@ -399,10 +401,18 @@ static void __follow (dir_t *restrict di break;/* Loop detected, stop recursion */ } - if ((mode == 'S') (attof(tmp)-flags SERV_ALL)) { - warn(%s depends on %s and therefore on system facility `$all' which can not be true!\n, - target-script ? target-script : target-name, tmp-script ? tmp-script : tmp-name); - continue; + if (mode == 'K') { + if (kflags SERV_FIRST) { + warn(Stopping %s depends on %s and therefore on system facility `$all' which can not be true!\n, + tmp-script ? tmp-script : tmp-name, target-script ? target-script : target-name); + continue; + } + } else { + if (sflags SERV_ALL) { + warn(Starting %s depends on %s and therefore on system facility `$all' which can not be true!\n, + target-script ? target-script : target-name, tmp-script ? tmp-script :
Bug#541613: insserv: infinite loop with memory leakage when a virtual facility depends on itself
On Sat, Aug 15, 2009 at 05:19:41PM +0200, Petter Reinholdtsen wrote: [Raphael Geissert] As mentioned on IRC, when a virtual facility depends on itself insserv enters an infinite loop and starts leaking memory. Thank you. I've written this test case to reproduce the problem. CC to upstream to let him know about the problem. OK, I've added a check to avoid such loops and also check to avoid that a scripts depends on a scripts which uses the system facility `$all' in its Required-Start. Does this work for you? Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr --- insserv.c +++ insserv.c 2009-08-17 14:00:25.841900763 +0200 @@ -222,6 +222,7 @@ typedef struct string { typedef struct repl { list_t r_list; +ushort flags; string_t r[1]; } __align repl_t; #define getrepl(arg) list_entry((arg), struct repl, r_list) @@ -2060,13 +2061,19 @@ static void expand_faci(list_t *restrict list_for_each_safe(tmp, safe, ptr) { repl_t * rnxt = getrepl(tmp); + if (rnxt-flags 0x0001) { + error(Loop detected during expanding system facilities in the insserv.conf file(s): %s\n, + rnxt-r[0].name); + } if (*rnxt-r[0].name == '$') { if (*deep 10) { warn(The nested level of the system facilities in the insserv.conf file(s) is to large\n); goto out; } (*deep)++; + rnxt-flags |= 0x0001; expand_faci(tmp, head, deep); + rnxt-flags = ~0x0001; (*deep)--; } else if (*deep 0) { repl_t *restrict subst; @@ -2087,9 +2094,12 @@ static inline void expand_conf(void) list_for_each(ptr, sysfaci_start) { list_t * rlist, * safe, * head = getfaci(ptr)-replace; list_for_each_safe(rlist, safe, head) { - if (*getrepl(rlist)-r[0].name == '$') { + repl_t * tmp = getrepl(rlist); + if (*tmp-r[0].name == '$') { int deep = 0; + tmp-flags |= 0x0001; expand_faci(rlist, rlist, deep); + tmp-flags = ~0x0001; } } } --- listing.c +++ listing.c 2009-08-17 14:15:48.893900934 +0200 @@ -364,6 +364,7 @@ static void __follow (dir_t *restrict di * Do not count the dependcy deep of the system facilities * but follow them to count the replacing provides. */ + if (*ptmp-name == '$') warn(System facilities not fully expanded, see %s!\n, dir-name); else if (++deep MAX_DEEP) { @@ -398,6 +399,12 @@ static void __follow (dir_t *restrict di break;/* Loop detected, stop recursion */ } + if ((mode == 'S') (attof(tmp)-flags SERV_ALL)) { + warn(%s depends on %s and therefore on system facility `$all' which can not be true!\n, + target-script ? target-script : target-name, tmp-script ? tmp-script : tmp-name); + continue; + } + if (ptrg-deep = deep) /* Nothing new */ continue; /* The inner recursion */
Bug#542043: insserv: please enable an $all like facility for stop ordering
On Mon, Aug 17, 2009 at 05:41:23PM +0200, Petter Reinholdtsen wrote: [Michael Meskes] Just imagine a logger that is supposed to log all actions done while shutting down the system or, as in my case, a daemon whose job it is to monitor other daemons and, if they do not run anymore, either restart that daemon or the whole system. It's fairly easy to make the start action run late enough because there's a hierarchy already there. But it is almost impossible to stop this daemon first without listing all ~1000 facilities as to be stopped later. Werner, what is your opinion on allowing $all to work also for stop dependencies, to be able to stop scripts very early during shutdown? Michael Meskes ran into the need with the watchdog package, where he want to stop the service monitoring before any services are stopped. Hmmm ... this is already on my todo but requires to move already sorted scripts in the hierarchy. Or I increase the order nummer before sorting. Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org