Messages in this thread | | | Date | Mon, 11 Sep 2023 17:42:08 -0500 | Subject | Re: [PATCH v1 1/2] fsi: sbefifo: Remove write's max command length | From | Ninad Palsule <> |
| |
Hi Joel,
On 9/11/23 01:03, Joel Stanley wrote: > On Thu, 7 Sept 2023 at 22:10, Ninad Palsule <ninad@linux.ibm.com> wrote: >> This commit removes max command length check in the user write path. >> This is required to support images larger than 1MB. This should not >> create any issues as read path does not have this check either. >> >> As per the original design cronus server was suppose to break up the >> image into 1MB pieces but it requires restructuring of the driver. > When you say "driver" you mean the kernel driver, or userspace? > > This isn't a great justification for removing a bounds check. I have improved the comment. Added length check back. > >> Today driver sends EOT message on each write request so we will have to >> send it only after all pieces are sent which requires large change hence >> we decided to remove this check. > This paragraph could be clearer. Could you try rephrasing? > > Assuming we want to make this change, what is the expected maximum > transfer? Could we instead make the check be that value (3MB?). Added length check back. I am using 4MB for some cushion.
Thanks for the review.
~Ninad
>> Testing: >> Loaded 3 MB image through cronus server. >> >> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com> >> --- >> drivers/fsi/fsi-sbefifo.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c >> index 9912b7a6a4b9..b771dff27f7f 100644 >> --- a/drivers/fsi/fsi-sbefifo.c >> +++ b/drivers/fsi/fsi-sbefifo.c >> @@ -113,7 +113,6 @@ enum sbe_state >> #define SBEFIFO_TIMEOUT_IN_RSP 1000 >> >> /* Other constants */ >> -#define SBEFIFO_MAX_USER_CMD_LEN (0x100000 + PAGE_SIZE) >> #define SBEFIFO_RESET_MAGIC 0x52534554 /* "RSET" */ >> >> struct sbefifo { >> @@ -870,8 +869,6 @@ static ssize_t sbefifo_user_write(struct file *file, const char __user *buf, >> if (!user) >> return -EINVAL; >> sbefifo = user->sbefifo; >> - if (len > SBEFIFO_MAX_USER_CMD_LEN) >> - return -EINVAL; >> if (len & 3) >> return -EINVAL; >> >> -- >> 2.39.2 >>
| |