Re: [PATCH] Don't initialise new inode generation numbers to zero V2

David Chinner
2008-04-22T05:04:37Z

On Tue, Apr 22, 2008 at 02:05:47PM +1000, Greg Banks wrote:
> David Chinner wrote:
> > Don't initialise new inode generation numbers to zero
> >
> > When we allocation new inode chunks, we initialise the generation
> > numbers to zero. This works fine until we delete a chunk and then
> > reallocate it, resulting in the same inode numbers but with a
> > reset generation count. This can result in inode/generation
> > pairs of different inodes occurring relatively close together.
> >
> > Given that the inode/gen pair makes up the "unique" portion of
> > an NFS filehandle on XFS, this can result in file handles cached
> > on clients being seen on the wire from the server but refer to
> > a different file. This causes .... issues for NFS clients.
> >
> > Hence we need a unique generation number initialisation for
> > each inode to prevent reuse of a small portion of the generation
> > number space. Make this initialiser per-allocation group so
> > that it is not a single point of contention in the filesystem,
> > and increment it on every allocation within an AG to reduce the
> > chance that a generation number is reused for a given inode number
> > if the inode chunk is deleted and reallocated immediately
> > afterwards.
> >
> > Version 2:
> > o remove persistent per-AGI agi_newinogen field and replace with
> > randomly generated 32 bit number for each new cluster. This prevents
> > NFS clients from potentially guessing what the next generation
> > number is going to be.
> >
> I'm confused, why would an NFS client be trying to guess the generation
> number? AFAICS the important thing is to ensure that the (inode,gen)
> tuple isn't reused for a long time to prevent accidental filehandle
> identity issues on clients; whether the gen is predictable or not
> doesn't matter at all.

Yeah, that's exactly what I said to Christoph, but that's the issue he
raised w.r.t a malicious client triggering inode/gen collisions
intentionally. If that's not a problem, then I can just use random32()
for the inode number. If it is a real problem, then it needs to be
a cryptographically secure random number. Personally, I don't care
either way - I just want to get the issue fixed.

Christoph, care to explain how and why this is a problem to everyone?

Cheers,

Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group