marking tapset files

marking tapset files

[Frank Ch. Eigler]

Hi -

If you recall, scripts located under tapset/* are like libraries and
not mainline scripts people can run individually.  This confusion is
biting several people.  Short of some sort of mechanical enforcement,
do you think it would be worthwhile to put an additional header comment
into all our tapsets, near the copyright header (which they all should
have, hint hint):

      /* This file is part of the tapset library.  It is not meant for
      direct execution, but only for implicit use by other scripts.
      See test case <FILE> for an example. */

- FChE

Re: marking tapset files

[Mike Mason]

I like the idea of adding the comment.  Mechanical enforcement would be even better.  Perhaps a keyword at the top of the file that marks it as part of a specific tapset.  Then stap could print a  clearer error message if someone tries to run it directly.  For example, if tapset "foo" is comprised of three files, then each of those files could contain "tapset foo" near the top. This would also make it clearer which files are part of multi-file tapsets.

- Mike

Frank Ch. Eigler wrote:
> Hi -
> 
> If you recall, scripts located under tapset/* are like libraries and
> not mainline scripts people can run individually.  This confusion is
> biting several people.  Short of some sort of mechanical enforcement,
> do you think it would be worthwhile to put an additional header comment
> into all our tapsets, near the copyright header (which they all should
> have, hint hint):
> 
>       /* This file is part of the tapset library.  It is not meant for
>       direct execution, but only for implicit use by other scripts.
>       See test case <FILE> for an example. */
> 
> - FChE

Re: marking tapset files

[Frank Ch. Eigler]

Hi -


On Fri, Oct 27, 2006 at 08:14:11AM -0700, Mike Mason wrote:
> I like the idea of adding the comment.  Mechanical enforcement would be 
> even better.  Perhaps a keyword at the top of the file that marks it as 
> part of a specific tapset.  [...]

That's a possibility.  Short of language extensions though, the
translator could parse the tapset scripts first, then warn about
duplication.  Heck, it could even track the visited dev/inode pairs in
struct stapfile, and know if the same file has been seen twice.

- FChE

Re: marking tapset files

[Vara Prasad]

Frank Ch. Eigler wrote:

>Hi -
>
>
>On Fri, Oct 27, 2006 at 08:14:11AM -0700, Mike Mason wrote:
>  
>
>>I like the idea of adding the comment.  Mechanical enforcement would be 
>>even better.  Perhaps a keyword at the top of the file that marks it as 
>>part of a specific tapset.  [...]
>>    
>>
>
>That's a possibility.  Short of language extensions though, the
>translator could parse the tapset scripts first, then warn about
>duplication.  Heck, it could even track the visited dev/inode pairs in
>struct stapfile, and know if the same file has been seen twice.
>
>- FChE
>  
>
Yes, translator can find out if same file is visted twice but with that 
can we give a meaningful error message to the user. I think what is 
needed here is when someone tries to execute a tapset they should gate 
message saying you can not execute a tapset library directly, similar to 
an error message given if we execute a library file. It would be even 
better if we can do something with our naming so that users can say the 
difference between the library and the end user script and avoid doing 
that mistake in the first place.

Re: marking tapset files

[Vara Prasad]

Vara Prasad wrote:

> Frank Ch. Eigler wrote:
>
>> Hi -
>>
>>
>> On Fri, Oct 27, 2006 at 08:14:11AM -0700, Mike Mason wrote:
>>  
>>
>>> I like the idea of adding the comment.  Mechanical enforcement would 
>>> be even better.  Perhaps a keyword at the top of the file that marks 
>>> it as part of a specific tapset.  [...]
>>>   
>>
>>
>> That's a possibility.  Short of language extensions though, the
>> translator could parse the tapset scripts first, then warn about
>> duplication.  Heck, it could even track the visited dev/inode pairs in
>> struct stapfile, and know if the same file has been seen twice.
>>
>> - FChE
>>  
>>
> Yes, translator can find out if same file is visted twice but with 
> that can we give a meaningful error message to the user. I think what 
> is needed here is when someone tries to execute a tapset they should 
> gate message saying you can not execute a tapset library directly, 
> similar to an error message given if we execute a library file. It 
> would be even better if we can do something with our naming so that 
> users can say the difference between the library and the end user 
> script and avoid doing that mistake in the first place.
>
>
>
Hi Frank,

Any resolution on this. I had to deal with two users in the last week 
who ran to this very problem of directly executing tapsets as scripts. 
My proposal would be let us change the extension of tapsets as .tap and 
scripts to remain as .stp, any objections to that. Next step we could 
enhance translator to give better error messages.

bye,
Vara Prasad

Re: marking tapset files

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

Hi -

varap wrote:

> [...]  Any resolution on this.

One minor tweak already done is the inclusion of a README file in
/usr/share/systemtap/tapset which says "don't run these!" (with more
words).

> I had to deal with two users in the last week who ran to this very
> problem of directly executing tapsets as scripts.  My proposal would
> be let us change the extension of tapsets as .tap and scripts to
> remain as .stp

Do you believe that this measure, by itself, would have been enough to
keep your users from running them directly?

> Next step we could enhance translator to give better error messages.

Let's open a bug report for that.

- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: marking tapset files

[Vara Prasad]

Frank Ch. Eigler wrote:

Sorry for the delay in replying to your email.

>Hi -
>
>varap wrote:
>
>  
>
>>[...]  Any resolution on this.
>>    
>>
>
>One minor tweak already done is the inclusion of a README file in
>/usr/share/systemtap/tapset which says "don't run these!" (with more
>words).
>  
>
Thanks for the addition to the README.
I know users are expected to read the instructions but as you know many 
don't, so we need to make it better than that.

>  
>
>>I had to deal with two users in the last week who ran to this very
>>problem of directly executing tapsets as scripts.  My proposal would
>>be let us change the extension of tapsets as .tap and scripts to
>>remain as .stp
>>    
>>
>
>Do you believe that this measure, by itself, would have been enough to
>keep your users from running them directly?
>  
>

I went back and discussed with all the users who ran into this problem 
and the answer i got back was unanimous yes.  One user even said i was 
just looking for anything with an extension .stp as an example to try 
and i cam across tapset file and i have executed without even thinking 
twice.

>  
>
>>Next step we could enhance translator to give better error messages.
>>    
>>
>
>Let's open a bug report for that.
>  
>

I have opened the bug 3648 for this.

>- FChE
>  
>

Re: marking tapset files

[Frank Ch. Eigler]

Hi -

On Mon, Dec 04, 2006 at 01:56:54PM -0800, Vara Prasad wrote:
> [...]
> > > [...]  My proposal would
> > > be let us change the extension of tapsets as .tap and scripts to
> > > remain as .stp
> >
> >Do you believe that this measure, by itself, would have been enough to
> >keep your users from running them directly?
>
> I went back and discussed with all the users who ran into this problem 
> and the answer i got back was unanimous yes.  [...]

I wonder if that answer would change, if we were to have both .tap and
.stp files sprinkled around. [...]

- FChE

Re: marking tapset files

[Vara Prasad]

Frank Ch. Eigler wrote:

>Hi -
>
>On Mon, Dec 04, 2006 at 01:56:54PM -0800, Vara Prasad wrote:
>  
>
>>[...]
>>    
>>
>>>>[...]  My proposal would
>>>>be let us change the extension of tapsets as .tap and scripts to
>>>>remain as .stp
>>>>        
>>>>
>>>Do you believe that this measure, by itself, would have been enough to
>>>keep your users from running them directly?
>>>      
>>>
>>I went back and discussed with all the users who ran into this problem
>>and the answer i got back was unanimous yes.  [...]
>>    
>>
>
>I wonder if that answer would change, if we were to have both .tap and
>.stp files sprinkled around. [...]
>  
>
Well they said they like the way the current tapsets are all under one 
directory although they would prefer a sub directory structure below it. 
I would think the answer would be even resounding yes to have different 
extensions for scripts and tapsets if we have them mixed.

>- FChE
>  
>

Re: marking tapset files

[Vara Prasad]

Frank Ch. Eigler wrote:

>Hi -
>
>If you recall, scripts located under tapset/* are like libraries and
>not mainline scripts people can run individually.  This confusion is
>biting several people.  Short of some sort of mechanical enforcement,
>do you think it would be worthwhile to put an additional header comment
>into all our tapsets, near the copyright header (which they all should
>have, hint hint):
>
>      /* This file is part of the tapset library.  It is not meant for
>      direct execution, but only for implicit use by other scripts.
>      See test case <FILE> for an example. */
>
>- FChE
>  
>

How about we change the extension type of these tapsets something like 
.tap instead of .stp that is used for end user scripts. I think we 
should also have the name of the tapsets start with lib*. I think we 
should also add some kind of signature inside the tapset at the top 
equivalent to the magic number of executables. If this convention is 
followed translator could recognize the difference between scripts and 
tapsets and error out if someone tries to execute them directly.

If some user renames these files and tampers with the magic number we 
may not recognize it as a tapset and run into the same problem that we 
are facing today, but that is acceptable i guess if someone tampers with 
it.