This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] enhance quoting for command string


Hi -

hunt wrote:

> > Yes, that could be one way.  staprun/stapio would have to do similar
> > quoting to ensure that the funny strings make it all the way through.
> 
> If the string is quoted correctly why would it need requoted? It seems
> to be working fine with this patch.

Perhaps because the quotation signs may get swallowed up by one
careless copy?

Let's put it this way.  It is important to pass this data verbatim (or
at worst, reject questionable inputs).  How about a testsuite addition
that confirms this from end to end?

> > (There's also the tapset function system() to worry about.)
> Which is a completely different problem because the command line must be
> passed in as a string in systemtap.  However it seems to always do
> exactly what I expect.  Have you found any problems with it?

I haven't used it aggressively.  But the burden of proof is on the
code: could you put in a stress test (e.g.: pass as many different
bytes as possible).


> > For another way, we could explore using plain old exec*(2) instead of
> > system(3) for invoking staprun/stapio/user-specified commands.
> 
> That's what I had in mind when I created 2424, however I reconsidered.
> I find I like doing sequences of commands, like
> stap -c "ls; ps" foo.stp
> Not a useful example

Indeed, as the plain target_pid() will not help.  But of course it can be
done with execve via:

   stap -c "sh -c 'ls; ps'" foo.stp


> This patch seems to correct the quoting problem for all the
> testcases I had.

The only tests I see in the test suite are very gentle in this regard.

> Any objections to committing it?

I would be happier if there were stress tests, and if there were fewer
calls to system().  I'm happy to see that staprun already uses execl
to run the commands passed from the tapset system() function.


- FChE


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]