Bug#190732: Bugs in libdiskfs/dir-rename.c

[Ognyan Kulev]

While trying to solve #190732 (libdiskfs/dir-renamed.c), I found some
suspicous code in dir-rename.c.  I'm not sure if all are bugs, so I post
them here to be discussed, if needed.

1. All directory renames are serialized by renamedirlock.  It looks like
this (in a kind of pseudo-code):

again:
 lookup (fromname);
 if (fromname is directory) {
   if (! try_lock (renamedirlock)) {
     lock (renamedirlock);
     goto again;
   }
   ...
 }

As you see, if renamedirlock is held by somebody else, then no
serialization will occur.

2. When directory is renamed, dir-renamed.c::diskfs_rename_dir is
called.  After that, there are some diskfs_file_update calls when
disk_synchronous.  But these functions seem to be called in
diskfs_rename_dir too, so calling them should be redundant.

Another issue is if they have to be called when renaming regular file,
not directory.  This is not done by dir-rename.c::diskfs_S_dir_rename.

As far as I understand, diskfs_node_update is called when stat fields
are changed, and diskfs_file_update is called when (usually both) stat
fields and file content are changed.  Then why there are some
diskfs_file_update in dir-renamed.c on FNP and TNP?  (The question is
not about FDP and TDP.)  Their content is not changed, only stat fields
are changed.

3. When checking if target exists and EXCL is set ("!err && excl"), the
function doesn't return and some locks are not released.  These lines
should be appended too:

diskfs_drop_dirstat (tdp, ds);
diskfs_nrele (fnp);
mutex_unlock (&tdp->lock);
return err;

4. The "diskfs_node_update (fnp, 1)" is not guarded by "if
(diskfs_synchronous)".

5. In the "tnp && S_ISDIR (tnp)" case, one more lock should be released:

diskfs_nput (tnp);

6. In the "fnp.st_nlink == max-1" case, one more lock should be released:

if (tnp)
  diskfs_nput (tnp);


Regards
--
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782