[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Bugs in coreutils/timeout.c for systems without MMU
From:       Patrick Hock <patrickrhock () gmail ! com>
Date:       2024-02-29 1:47:58
Message-ID: CA+9tvONjMaB5aGQtut8GKXPakbY9ZoSPc=AXBJdb-pkcCVwO2Q () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


Hi,

consider the example prompt for timeout below:
timeout -s 9 1 sleep 5

In timeout.c line 120-123:
#if !BB_MMU
    argv[optind] = xasprintf("-p%u", parent);
    argv[optind + 1] = NULL;
#endif

This puts a -p:PID argument at the end of the timeout command when it is
re-execed, like this example:
timeout -s 9 1 -p:577
(Note the lack of a argument after the timeout in seconds (1), this is part
of another bug I'll address later.)
The issue is that the way options are parsed with getopt32 at the beginning
of the program, the '+' makes getopt32 stop parsing when it gets to the
first non-option argument (which happens to be the timeout in seconds).
This means when timeout is re-execed with the -p option at the end, the -p
option is never parsed. This is important because on line 109, the presence
of the -p option should make parent true, but it never will.
The solution I propose to this is to put the -p option BEFORE the timeout
in seconds, essentially swapping the position of the two so that the -p
precedes any non option arguments and will be parsed by getopt32
So that would be:
  #if !BB_MMU
    argv[optind] =  argv[optind - 1]
    argv[optind - 1] =  xasprintf("-p%u", parent);
    argv[optind + 1] = NULL;
#endif

Also, when timeout is re-execed with the -p option, you no longer need to
pass in an argument for what program needs to be executed. Timeout just
needs to kill the process specified by -p after the given timeout. So, on
line 98-99:
 if (!argv[optind]) /* no PROG? */
bb_show_usage();

This should be put after the branch goto grandchild on line 110.
Since bb_daemonize_or_rexec redirects output of the re-execed timeout
(without a program argument) to /dev/null, you won't see the usage prompt
printed to the terminal, but effectively timeout won't work because it's
killed prematurely by this.

Also, I'm not sure if it is intentional, but this implementation makes it
possible to pass a -p:PID argument to timeout in the command line, which
makes it kill the specified process ID after the timeout elapses.

Patrick H

[Attachment #5 (text/html)]

<div dir="ltr">Hi,<div><br></div><div>consider the  example prompt for timeout \
below:</div><div>timeout -s 9 1 sleep 5<br><div><br></div><div>In timeout.c line \
120-123:</div><div>#if !BB_MMU<br>      argv[optind] = xasprintf(&quot;-p%u&quot;, \
parent);<br>      argv[optind + 1] = \
NULL;<br>#endif<br></div><div><br></div><div>This puts a -p:PID argument at the  end \
of the timeout command when it is re-execed, like this example:</div><div>timeout -s \
9 1 -p:577</div><div>(Note the lack of a argument after the timeout in seconds (1), \
this is part of another bug I&#39;ll address later.)</div><div>The issue is that the \
way options are parsed with getopt32 at the beginning of the program, the &#39;+&#39; \
makes getopt32 stop parsing when it gets to the first non-option argument (which \
happens to be the timeout in seconds). This means when timeout is re-execed with the \
-p option at the end, the -p option is never parsed. This is important because on \
line 109, the presence of the -p option should make parent true, but it never will.  \
</div><div>The solution I propose to this is to put the -p option BEFORE the timeout \
in seconds, essentially swapping the position of the two so that the -p precedes any \
non option arguments and will be parsed by getopt32</div><div>So that would \
be:</div><div>   #if !BB_MMU</div><div>      argv[optind] =   argv[optind - \
1]</div><div>      argv[optind - 1] =  

 xasprintf(&quot;-p%u&quot;, parent);<br>      argv[optind + 1] = NULL;<br>#endif    \
<br></div><div><br></div><div>Also, when timeout is re-execed with the -p option, you \
no longer need to pass in an argument for what program needs to be executed. Timeout \
just needs to kill the process specified by -p after the given timeout. So, on line \
98-99:</div><div>  if (!argv[optind]) /* no PROG? \
*/<br>		bb_show_usage();<br></div><div><br></div><div>This should be put after the \
branch goto grandchild on line 110. Since  bb_daemonize_or_rexec redirects output of \
the re-execed timeout (without a program argument) to /dev/null, you won&#39;t see \
the usage prompt printed to the terminal, but effectively timeout won&#39;t work \
because it&#39;s killed prematurely by this.</div><div><br></div><div>Also, I&#39;m \
not sure if it is intentional, but this implementation makes it possible to pass a \
-p:PID argument to timeout in the command line, which makes it kill the specified \
process ID after the timeout elapses.</div><div><br></div><div>Patrick \
H</div><div><br></div></div></div>



_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic