This is the mail archive of the cygwin-developers mailing list for the Cygwin 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: Avoid collisions between parallel installations of Cygwin


To sum up: I think Corinna's approach is fine -- one big struct
containing all the resources, and a single magic string, so long as

(1) the script or binary tool that modifies the properties checks the
version of the properties structure, and prints an error if doesn't
match the specific version that the script knows about.

(2) I don't think it's a good idea to encourage people to modify the
cygwin DLL's properties while other cygwin apps are running. Hence,
avoid the cmd / trick in the script.

(3) Putting the manipulator functionality in cygcheck would make it
harder for users to evade the version check in (1), but wouldn't
necessarily avoid the issue in (2) (?)


> On Oct 21 12:10, Charles Wilson wrote:
>> Coinna wrote:
>> > Do you think this is sufficient for now?
>> 
>> Unfortunately, I don't.  First, because of alignment issues, it's almost
>> a given that the offset from the start of the string to the data you
>> want to change will be different in each build of the DLL, if it's just
> 
> Which would speak against using resources.

Unless the resource "value" was a binary blob, which we interpret on our
own.  Which is really the same deal you have with storing that binary
blob (struct) anywhere else: a specific .section, a single variable in
.rdata, etc.

The difference is, with resources, the name of the resource is the
"unique string" and the pointer you get back from FindResource points to
the beginning of a struct which does not contain that unique string.

But, given the difficulties with using UpdateResource from a foreign
tool (e.g. cygcheck), we have to put the unique string into the struct
itself so that (e.g. cygcheck) can find the location of the beginning of
the struct without using FindResource, and accurately calculate the
offsets to the important stuff.

Once you do that...then there's no reason to use .rsrc.  Might as well
make it a global variable (whether it lives in .rdata or
.some_unique_section doesn't really matter, except that ensuring it gets
loaded into memory in the latter case might require more fiddling --
makefiles, objcopy, etc. Nothing we don't do already).

>> ...unless the magic ID strings themselves were also each part of the
>> struct.  In THAT case, I suppose the offsets between beginning of the
>> string and the data would depend only on the length of the string, the
>> packing options (-ms-struct?, pragma pack?) used during the compile, and
>> the specific alignment requirements of the data -- none of which would
>> change.
> 
> Indeed.  That was the sole idea.

Well, kinda. You've gone back to one-struct-with-all-values, and a
single magic string, instead of what I thought was an array of different
structs, each with their own magic value/name.  That is, you've
abandoned extensibility in the tools (which is OK, as long as we
document it). If contents of the resource struct change, then older
tools won't be able to affect the newer elements -- and that's assuming
there were no incompatible changes in the exising elements of the struct.

Newer tools would need to know which elements were safe to touch, in
older DLLs.

Or, they could all just check the struct version number and refuse to do
anything at all with a DLL that is different from the one they know
about (newer OR older).  The script wouldn't necessarily be tied to a
single specific cygwin release or build, but to the sequence that all
use the same struct version.

Which ought to be a pretty long sequence because I don't see too many
items that really belong in it, rather than in $CYGWIN.

>> Hmm.  Well, that IS a benefit -- can we quantify how much of one?
> 
> No.  But accessing a global int is almost certainly a lot faster than
> calling three Win32 functions.

Agreed.

> I think you're making this too complicated, really. 

Wouldn't be the first time.

> If I have to
> tweak the DLL in any binary fashion, it does not the least matter
> at which point in the binary blob the data is.  And the advantage of
> using a global structure with defined, *simple* content should be quite
> clear, compared to some resource layout which is not exactly under
> our control.

OK.

>> there's only one
>> FindResource/LoadResource/LockResource episode per process tree.
> 
> Huh?  That's the case anyway with my code, I thought that's clear from
> the way the shared memory initilization works.  What suffers is only the
> startup of the first process in a process tree, or, in other words,
> running Cygwin appications from a non-Cygwin parent.

Right. OK, so...

> Sorry, but I really don't understand what's the problem.  This is the
> global datastructure:
> 
>   struct cygwin_props_t
>   {
>     char magic_string[158];
>     unsigned long version_number;
>     unsigned long disable_key;
>     [...]
>   } cygwin_props = {
>     "This is a magic string value which can be used by any external tool "
>     "to find the values to change in the global Cygwin properties structure, "
>     "really, I mean it",
>     1,
>     0,
>     [...]
>   };
> 
> It's pretty simple to find the location searchin for the magic string
> and it's as simple to find the subsequent data.

Correct, so long as
$tool cygwin_props_t::version_number ==
$dll cygwin_props_t::version_number

Anything else, and you're just asking for trouble.

>> The big concern about brute force manipulation of binary (packed?) data
>> structs is keeping track of the offsets, and the data sizes of each
>> element
> 
> What's the problem with the above structure?

IF you want to allow tools from one cygwin release to manipulate the
properties of a different cygwin release, THEN:
  as long as the struct definition never changes, and we never add
  any new values or change the semantics of old values, nothing.

But that's a pretty big if.  And you can't completely solve it by
including a whole bunch of
   char reserved[XXX]
buffers.

Or we say "this cygcheck supports only version X of the cygwin_props_t
structure; it will report an error if you attempt to display or modify
the cygwin_props of a DLL that uses a different version".

Which is an argument for putting that into the cygcheck binary -- where
it's harder for somebody to edit the script source code and remove the
version check...

OTOH, do we really need to be THAT paranoid about protecting malicious
users from themselves?  So long as the stock script DOES contain the
version check?

> The size of the DLL is > 1 Meg and growing as we add functionality.
> What's the problem to add a couple of bytes

You're right. The message I sent just after the one you're replying to
here agrees with you.

>> I hesitate to suggest Yet Another Addition before 1.7.1 -- and cgf is
>> gonna kill me [...]
> 
> Not only cgf.  I have absolutely no idea what XDR has to do with
> a simple piece of DLL data.  Sledgehammer - nail?

Right. We don't need it now; but it could be something to think about
for later just in general (XDR is a fairly useful facility; I'd like to
see it in newlib someday).

If it WERE already available in the cygwin dll, AND we wanted to allow
on-the-fly extensibility (so that old cygcheck could print and modify
the values of new cygwin1.dll resources, or vice versa), then I'd push
for using it in this case.  E.g.

typedef struct  {
   char[SNSZ] short_name;
   char[LDSZ] long_description;
   enum cygprop_record_union_descriminator type;
   union { ... } value;
} cygprop_record;

struct {
   char[MSSZ] magic_string;
   long cygprops_version_number;
   long number_of_cygprop_records;
   struct cygprop_record props[]; /* struct hack */
}

...is easy-peasy to handle using XDR to put into/read from a binary blob
at a specific file offset on disk, or at a specific location in memory
-- by a native win32 tool that modifies cygwin1.dll or by cygwin1.dll
itself.

But it isn't (already available), and we don't (want/need to allow
on-the-fly extensibility), so we shouldn't (discuss XDR any futher).

--
Chuck


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