Bill Sommerfeld wrote: > On Wed, 2007-10-03 at 16:21 -0400, James Carlson wrote: > >> Perhaps we're getting a bit too close to the point of design or code >> review instead of architecture, but other than simply disabling the >> security check, I don't see how the new feature contributes towards >> additional security. >> > >
I'd argue that the change that went in for autofs was not correct and removed security. What I want to do is level the playing field here. When a trigger mount occurs, we expect that the stat() and fstat() results will be different. We have the choice of accepting that or redoing the stat() to see if anything else has changed. > There is architecture here. > > When an interface is added for security reasons, it is essential to have > a crisp definition of the threat it addresses in order to allow people > to evaluate whether the interface actually addresses the threat. > > I don't see that here. > > - Bill > > Is this an artifact of the existing code inside walk() or from the new feature? I.e., I can't find a crisp definition of what threats the original code dealt with - I've heard that it handles symlinks and when a subtree is moved. The new feature does not address a threat - the code in walk() does. What the new feature does is allow an exception to that threat. It deterministically lets us know that we are about to encounter an exception. The intent of the original code is that what points to a file is the same as where the file thinks it is located. When the exception was made for autofs, an assumption was made that directory entries inside autofs were carved in stone and safe. What I am arguing, and I'm the first to admit it is hand waving, is that we should not blindly accept that even after we detect the exception will occur, that this directory entry gets a free pass. I think we are talking implementation here - how we handle the exception once we detect it. We know prior customer expectation is that we allow applications which call nftw() to proceed as if a problem had not just occurred. As far as the implementation goes, I don't have a stake in the ground as to whether we record the fact in a boolean or refresh the stat buffer. I'm perfectly willing to ask James to be a code reviewer for the final fix.
