In the original commit that introduced that thing, we have
a somewhat strange note in commit message:
Use filp_close instead of open coding. filp_close does a bit more than
just release the locks and put the filp. It also calls ->flush and
dnotify_flush, both of which should be done here anyway.
How could dnotify_flush() possibly catch anything here? In the current
form the value we pass as id is
(fl_owner_t)lockowner(stp->st_stateowner)
and lockowner is container_of(so, struct nfs4_lockowner, lo_owner);
dnotify_flush() looks for matches on dn->dn_owner == id; anything
not matching is left alone. And ->d_owner is set only by attach_dn(),
which gets the value from
fl_owner_t id = current->files;
If we ever see a match here, we are in deep trouble - the same address
being used as struct files_struct * and struct nfs4_lockowner * at
the same time...
Another interesting question is about FUSE ->flush() - how is the
server supposed to use the value it gets from
inarg.lock_owner = fuse_lock_owner_id(fm->fc, id);
in fuse_flush()? Note that e.g. async write might be followed by
close() before the completion. Moreover, it's possible to start
async write and do unshare(CLONE_FILES); if the descriptor table
used to be shared and all other threads exit after our unshare,
it's possible to get
async write begins, fuse_send_write() called with current->files as owner
flush happens, with current->files as id
what used to be current->files gets freed and memory reused
async write completes
Miklos, could you give some braindump on that?
