On Thu, 7 Sept 2023 at 22:10, Ninad Palsule <ninad@xxxxxxxxxxxxx> wrote:Yes, I have improved the comment.
This commit fails user write operation if previous write operation isThe driver already has this code:
still pending.
As per the driver design write operation only prepares the buffer, the
actual FSI write is performed on next read operation. so if buggy
application sends two back to back writes or two parallel writes then
that could cause memory leak.
I do not see any issue with this rejection. I thought user may wants to send reset while command is hung but that case is not valid as pending command will hold the lock. User can always close the connection and reopen if required. How do I find if this could cause the regression?
Signed-off-by: Ninad Palsule <ninad@xxxxxxxxxxxxx>That's an unusual return code. I guess it makes sense in this context.
---
drivers/fsi/fsi-sbefifo.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index b771dff27f7f..824e2a921a25 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -874,6 +874,12 @@ static ssize_t sbefifo_user_write(struct file *file, const char __user *buf,
mutex_lock(&user->file_lock);
+ /* Previous write is still in progress */
+ if (user->pending_cmd) {
+ mutex_unlock(&user->file_lock);
+ return -EALREADY;
It's good to fix the potential memory leak, and we should add code to
catch that case.
This will change the behaviour of the character device from "overwrite
the previous operation" to "reject operation until a read is
performed". Do you think there's existing code that depends on the old
behaviour?
+ }
+
/* Can we use the pre-allocate buffer ? If not, allocate */
if (len <= PAGE_SIZE)
user->pending_cmd = user->cmd_page;
--
2.39.2