I agree that John's patch is worth doing. It satisfies both his requirements and the stated design goals of fastpath.
The remaining issue is whether something called "ns_returnfile" which takes a pathname as a parameter should have some guarantee that you will return what at least at some point was the contents of a file with that pathname. It's perfectly acceptable in dealing with caching systems that the cached value could be out of sync, but not that the cached value could be for something entirely different from what you were looking for. Even with the mtime fix there's no guarantee that systems which muck around with mtime (such as tar) won't cause separate files to collide. For a contrived example: 1. tar xf foo.tar (creating two files "a" and "b" with the same size and same mtime) 2. ns_returnfile b 3. Delete files "a" and "b" 4. tar xf foo.tar 5. ns_returnfile b (this could return the contents of "a" because the inode was reused) I don't think this example violates any of the stated principles of using ns_returnfile for only "static" data. Both "a" and "b" could have completely stable contents and due to some minor issue of system administration (for example) their inodes could end up swapped and the cache poisoned. So I think we need both fixes, one to eliminate caching unless a certain criterion of "static-ness" has been met, and the other to prevent the cache from returning completely unrelated data. Other caveats about ns_returnfile use still apply, and the documentation should reflect them. Now the only people this wouldn't satisfy are those who are concerned about pathnames taking up space in the cache or slowing it down. The option has been suggested to make pathname inclusion optional, though I would advise against it unless the configuration option is named in such a way as to indicate its "unsafe"-ness. Titi Ala'ilima Lead Architect MedTouch LLC 1100 Massachusetts Avenue Cambridge, MA 02138 617.621.8670 x309 > -----Original Message----- > From: AOLserver Discussion [mailto:[EMAIL PROTECTED] On > Behalf Of Tom Jackson > Sent: Thursday, August 21, 2008 12:25 PM > To: AOLSERVER@LISTSERV.AOL.COM > Subject: Re: [AOLSERVER] Data "corruption" with fastpath caching > > On Thu, 2008-08-21 at 11:14 -0400, Dossy Shiobara wrote: > > 4) I see the simplest (best?) solution here being a configurable > > parameter that controls fastpath's cache key generation. As Jim > points > > out, one can quickly test whether this would solve the problem at > hand > > by temporarily #define'ing _WIN32 in the appropriate place. If this > > proves successful, we change it from using #ifdef's to regular if() > > statements and define a new configuration parameter. End of > discussion. > > I have responded twice to John's newest patch idea, which is a one line > patch. It appears to completely eliminate any problem with cache > poisoning. It is simple, it doesn't change the semantics of the command > or anything else. It simply works around a known limitation of the stat > mtime granularity. > > The only security issue that was exposed was the misuse of > ns_returnfile. All of the data put into cache were entirely under the > control of the AOLserver process. The developer / maintainer of that > process is responsible for everything the process does. ns_returnfile > is > an inherently dangerous API, there is no handholding involved. You have > to understand what it is doing and why it exists. > > In fact, John even pointed out that the original code which wrote out > the contents of the file reused the same name over and over. Assuming > that you can know that the contents of a file have not changed just > because it has the same name, same mtime and same size is an invalid > assumption, it will always be invalid. All caches have the same > limitation. By definition they are not in sync with the true copy. > Anyone who uses a cache needs to understand this. > > So, this is important, John is not interested in the cache, he actually > wants to avoid the cache. So talking about how stuff is stored in the > cache, and under what key, is unimportant for John. He wants to keep > his > newly created file from ever getting into the cache. > > And this is where he has a point, a very good one. Why put newly > created > files into a cache, if the point of the cache is to handle static > files? > We can wait for evidence that it is static. In this case, we can wait > until it is a few seconds old, at least. John's patch does exactly this > and nothing more. It is actually a very ingenious change. > > > There is no difference between the inode and the filename under unix. > Both offer equal opportunity to screw up due to a race condition. It > can > still happen even in the patched ns_returnfile. Jim mentioned this. > After a file is stat'ed, the open might find a different (maybe > truncated) file. There is no guarantee that you won't get something > else, especially if you have multiple processes/threads creating files > in an non-synchronized way. It is not part of ns_returnfile to > guarantee > that the contents/age of a file remains unchanged during the course of > execution, and when you throw in an external process it is nearly > impossible to come up with any code which can provide that guarantee. > If > data integrity is really important to you, don't try to provide it > using > named files as temporary storage. > > tom jackson > > > -- > AOLserver - http://www.aolserver.com/ > > To Remove yourself from this list, simply send an email to > <[EMAIL PROTECTED]> with the > body of "SIGNOFF AOLSERVER" in the email message. You can leave the > Subject: field of your email blank. -- AOLserver - http://www.aolserver.com/ To Remove yourself from this list, simply send an email to <[EMAIL PROTECTED]> with the body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject: field of your email blank.
