lkml.org 
[lkml]   [2023]   [Sep]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v1 1/2] fsi: sbefifo: Remove write's max command length
From
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
>>

\
 
 \ /
  Last update: 2023-09-12 03:46    [W:0.067 / U:0.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site