A couple minor comments. Otherwise we should be fine, things aren't
going to get much better for such a messed up protocol design.
> + return dir_emit(ctx, d_name, strlen(d_name),
> + fake_ino, d_type);
> + } else {
> + return dir_emit(ctx,
> + info->name.string.utf8,
> + info->name.length,
> + fake_ino, d_type);
> + }
> + }
Nitpick: no need for an else after a return.
> +static int vboxsf_file_release(struct inode *inode, struct file *file)
> +{
> + struct vboxsf_inode *sf_i = VBOXSF_I(inode);
> + struct vboxsf_handle *sf_handle = file->private_data;
> +
> + filemap_write_and_wait(inode->i_mapping);
Normal Linux semantics don't include writing back data on close, so
if you are doing this to follow other things like NFS CTO semantics
it should have a comment explaining that.
> +
> + mutex_lock(&sf_i->handle_list_mutex);
> + list_del(&sf_handle->head);
> + mutex_unlock(&sf_i->handle_list_mutex);
> +
> + kref_put(&sf_handle->refcount, vboxsf_handle_release);
> + file->private_data = NULL;
There is no need to zero ->private_data on release, the file gets
freed and never reused.
> + * Ideally we would wrap generic_file_read_iter with a function which also
> + * does this check, to reduce the chance of us missing writes happening on the
> + * host side after open(). But the vboxsf stat call to the host only works on
> + * filenames, so that would require caching the filename in our
> + * file->private_data and there is no guarantee that filename will still
> + * be valid at read_iter time. So this would be in no way bulletproof.
Well, you can usually generate a file name from file->f_path.dentry.
The only odd case is opened by unliked files. NFS has a special hack
for those called sillyrename (you can grep for that). How similar to
normal posix semantics are expected from this fs?
> +
> + mutex_lock(&sf_i->handle_list_mutex);
> + list_for_each_entry(h, &sf_i->handle_list, head) {
> + if (h->access_flags == SHFL_CF_ACCESS_WRITE ||
> + h->access_flags == SHFL_CF_ACCESS_READWRITE) {
> + kref_get(&h->refcount);
Does this need a kref_get_unless_zero to deal with races during list
removal?
