2020-07-16 23:13:07

by Lenny Szubowicz

Subject: [PATCH] configfs: Use flush file op to commit writes to a binary file

Use the flush file operation instead of the release operation to commit
the prior writes to a configfs binary file. This allows any error
status from the commit to be returned as the status of the close.

Both flush and release are invoked during a close, but the status from
release is ignored by the file system layer because the release operation
is not supposed to fail.

For example, prior to this change no error is returned to user space
when acpi_configfs correctly fails a write that attempts to commit an
ACPI aml configfs binary file when kernel lockdown is in effect.
This patch allows an error status to get returned to user space instead
of a misleading success status.

Note that during a close, release is only called on the last reference to
the specified file struct whereas flush is called on every close.
Therefore, to preserve the prior behavior, configfs_flush_bin_file()
doesn't commit the prior writes if there are still multiple references.
Additionally, since configfs does not support the fsync file operation,
a configfs flush only occurs in the context of a close. This makes it
safe to move the commit from release to flush.

Signed-off-by: Lenny Szubowicz <[email protected]>
---
fs/configfs/file.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/fs/configfs/file.c b/fs/configfs/file.c
index fb65b706cc0d..df0a76f7e62b 100644
--- a/fs/configfs/file.c
+++ b/fs/configfs/file.c
@@ -466,9 +466,28 @@ static int configfs_open_bin_file(struct inode *inode, struct file *filp)
return __configfs_open_file(inode, filp, CONFIGFS_ITEM_BIN_ATTR);
}

-static int configfs_release_bin_file(struct inode *inode, struct file *file)
+/**
+ * configfs_flush_bin_file - flush a binary attribute.
+ * @file: file pointer
+ * @id: pointer to files_struct
+ *
+ * Flush is called during close and commits the buffered binary
+ * writes when there are no more shared references to this file
+ * struct.
+ *
+ * Any error returned from the flush will be reflected in the
+ * return value from the close.
+ */
+
+static int configfs_flush_bin_file(struct file *file, fl_owner_t id)
{
struct configfs_buffer *buffer = file->private_data;
+ ssize_t len;
+ int ret = 0;
+
+ /* Only commit the data if no more shared refs to file */
+ if (file_count(file) > 1)
+ return 0;

buffer->read_in_progress = false;

@@ -478,10 +497,11 @@ static int configfs_release_bin_file(struct inode *inode, struct file *file)

down_read(&frag->frag_sem);
if (!frag->frag_dead) {
- /* result of ->release() is ignored */
- buffer->bin_attr->write(buffer->item,
+ len = buffer->bin_attr->write(buffer->item,
buffer->bin_buffer,
buffer->bin_buffer_size);
+ if (len < 0)
+ ret = len;
}
up_read(&frag->frag_sem);
/* vfree on NULL is safe */
@@ -491,8 +511,7 @@ static int configfs_release_bin_file(struct inode *inode, struct file *file)
buffer->needs_read_fill = 1;
}

- configfs_release(inode, file);
- return 0;
+ return ret;
}


@@ -509,7 +528,8 @@ const struct file_operations configfs_bin_file_operations = {
.write = configfs_write_bin_file,
.llseek = NULL, /* bin file is not seekable */
.open = configfs_open_bin_file,
- .release = configfs_release_bin_file,
+ .flush = configfs_flush_bin_file,
+ .release = configfs_release,
};

/**
--
2.27.0

2020-07-20 13:02:14

by Christoph Hellwig

Subject: Re: [PATCH] configfs: Use flush file op to commit writes to a binary file

Thanks,

applied to the configfs tree.