| Summary: | [PATCH] Adds ability to invoke a script in order to determine a log-file name for a child being forked | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Eliot Moss <moss> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | REPORTED --- | ||
| Severity: | normal | CC: | flo2030, moss, philippe.waroquiers, pjfloyd, skarcos |
| Priority: | NOR | ||
| Version First Reported In: | 3.10 SVN | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | All | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
Patch that adds this feature (modifies one source code file and one documentation file)
Updated patch that addresses comments |
||
Interesting. Are there any security implications of this? I am not asserting that there are, but was just wondering if there were. On 9/26/2014 12:43 PM, Julian Seward wrote: > https://bugs.kde.org/show_bug.cgi?id=339405 > > --- Comment #1 from Julian Seward <jseward@acm.org> --- > Interesting. Are there any security implications of this? I am > not asserting that there are, but was just wondering if there were. I can't think of any, but I'm not a security expert. The script is run via "/bin/bash -c" so does whatever you can do that way. I don't see that anything acquires privileges, etc., that it would not otherwise have. Regards -- Eliot (In reply to Eliot Moss from comment #2) > > I can't think of any, but I'm not a security expert. The script > is run via "/bin/bash -c" so does whatever you can do that way. Note that the patch uses rather /bin/sh -c > I don't see that anything acquires privileges, etc., that it > would not otherwise have. I also do not see how that would give additional privileges to the user launching Valgrind. The only possible problem would be via setuid program, but I guess there is no way to have valgrind running setuid programs (otherwise, that would already be a security problem). Some minor comments on the patch: * /bin/sh -c can run any kind of command, not only scripts. * Maybe the part that launches the program and recuperate the output could be a separate function ? There is already something very similar in m_gdbserver/target.c which launches a program and recuperates the output. Either that could be done with an interface like popen(3) or (assuming this is all for programs producing small output), something like HChar* VG_(launch_a_program_and_allocates_memory_containing_output)( HChar*prog, ...) (with a better name :) or whatever that factorises the 2. * Use HChar* instead of Char* ? (In reply to Eliot Moss from comment #2) > On 9/26/2014 12:43 PM, Julian Seward wrote: > > https://bugs.kde.org/show_bug.cgi?id=339405 > > > > --- Comment #1 from Julian Seward <jseward@acm.org> --- > > Interesting. Are there any security implications of this? I am > > not asserting that there are, but was just wondering if there were. > > I can't think of any, but I'm not a security expert. The script > is run via "/bin/bash -c" so does whatever you can do that way. > I don't see that anything acquires privileges, etc., that it > would not otherwise have. > > Regards -- Eliot + ...... This + specifier is rarely needed, but very useful in certain circumstances, + such as when a program invokes a number of child processes whose logs + need to go to different places based factors difficult to express with + the <option>%p</option> or <option>%q</option> options. parse error :) Minor comment: no TABs in the source code please. On 9/27/2014 9:45 AM, Florian Krohm wrote: > + ...... This > + specifier is rarely needed, but very useful in certain circumstances, > + such as when a program invokes a number of child processes whose logs > + need to go to different places based factors difficult to express with > + the <option>%p</option> or <option>%q</option> options. > > parse error :) Yes, I see the omitted word now. Concerning TABs, I try not to have them, but I guess some got by me. What's the protocol? Should I make the suggested changes or will a valgrind developer do that? Regards -- Eliot Created attachment 89653 [details]
Updated patch that addresses comments
This updates the previous patch by: removing tabs from source code, changing Char to HChar, and fixing English grammar in the doc file. It builds under 14740 (the current head).
In hope that this patch might be included in the upcoming release ... Hello from the future! I'd really appreciate to see this added to a valgrind release. Is it still possible? |
Created attachment 88843 [details] Patch that adds this feature (modifies one source code file and one documentation file) --log-file= current supports expanding a pid or an environment variable to parameterize the name used for logging output. This patch adds the ability to invoke a script that creates and outputs the file name to use. It also patches the documentation, explaining this new feature.