All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH-next v2] loop: loop_set_status_from_info() check before assignment
@ 2023-02-06  2:07 Zhong Jinghua
  2023-02-06  2:55 ` Hou Tao
  2023-02-06  3:19 ` Yu Kuai
  0 siblings, 2 replies; 4+ messages in thread
From: Zhong Jinghua @ 2023-02-06  2:07 UTC (permalink / raw)
  To: axboe
  Cc: linux-block, linux-kernel, zhongjinghua, yi.zhang, yukuai3,
	houtao1, yangerkun

In loop_set_status_from_info(), lo->lo_offset and lo->lo_sizelimit should
be checked before reassignment, because if an overflow error occurs, the
original correct value will be changed to the wrong value, and it will not
be changed back.

Modifying to the wrong value logic is always not quiet right, we hope to
optimize this.

Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
---
 v1->v2: Modify note: overflowing -> overflow 
 drivers/block/loop.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 1518a6423279..1b35cbd029c7 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -977,13 +977,13 @@ loop_set_status_from_info(struct loop_device *lo,
 		return -EINVAL;
 	}
 
+	/* Avoid assigning overflow values */
+	if (info->lo_offset > LLONG_MAX || info->lo_sizelimit > LLONG_MAX)
+		return -EOVERFLOW;
+
 	lo->lo_offset = info->lo_offset;
 	lo->lo_sizelimit = info->lo_sizelimit;
 
-	/* loff_t vars have been assigned __u64 */
-	if (lo->lo_offset < 0 || lo->lo_sizelimit < 0)
-		return -EOVERFLOW;
-
 	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
 	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
 	lo->lo_flags = info->lo_flags;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH-next v2] loop: loop_set_status_from_info() check before assignment
  2023-02-06  2:07 [PATCH-next v2] loop: loop_set_status_from_info() check before assignment Zhong Jinghua
@ 2023-02-06  2:55 ` Hou Tao
  2023-02-06  3:02   ` Hou Tao
  2023-02-06  3:19 ` Yu Kuai
  1 sibling, 1 reply; 4+ messages in thread
From: Hou Tao @ 2023-02-06  2:55 UTC (permalink / raw)
  To: Zhong Jinghua, axboe
  Cc: linux-block, linux-kernel, yi.zhang, yukuai3, yangerkun



On 2/6/2023 10:07 AM, Zhong Jinghua wrote:
> In loop_set_status_from_info(), lo->lo_offset and lo->lo_sizelimit should
> be checked before reassignment, because if an overflow error occurs, the
> original correct value will be changed to the wrong value, and it will not
> be changed back.
>
> Modifying to the wrong value logic is always not quiet right, we hope to
> optimize this.
>
> Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
LGTM
> ---
>  v1->v2: Modify note: overflowing -> overflow 
>  drivers/block/loop.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 1518a6423279..1b35cbd029c7 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -977,13 +977,13 @@ loop_set_status_from_info(struct loop_device *lo,
>  		return -EINVAL;
>  	}
>  
> +	/* Avoid assigning overflow values */
> +	if (info->lo_offset > LLONG_MAX || info->lo_sizelimit > LLONG_MAX)
> +		return -EOVERFLOW;
> +
>  	lo->lo_offset = info->lo_offset;
>  	lo->lo_sizelimit = info->lo_sizelimit;
>  
> -	/* loff_t vars have been assigned __u64 */
> -	if (lo->lo_offset < 0 || lo->lo_sizelimit < 0)
> -		return -EOVERFLOW;
> -
>  	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
>  	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
>  	lo->lo_flags = info->lo_flags;


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH-next v2] loop: loop_set_status_from_info() check before assignment
  2023-02-06  2:55 ` Hou Tao
@ 2023-02-06  3:02   ` Hou Tao
  0 siblings, 0 replies; 4+ messages in thread
From: Hou Tao @ 2023-02-06  3:02 UTC (permalink / raw)
  To: Zhong Jinghua, axboe
  Cc: linux-block, linux-kernel, yi.zhang, yukuai3, yangerkun

Please disregard my reply. I did not notice it is not an internal email between
our team members.

On 2/6/2023 10:55 AM, Hou Tao wrote:
>
> On 2/6/2023 10:07 AM, Zhong Jinghua wrote:
>> In loop_set_status_from_info(), lo->lo_offset and lo->lo_sizelimit should
>> be checked before reassignment, because if an overflow error occurs, the
>> original correct value will be changed to the wrong value, and it will not
>> be changed back.
>>
>> Modifying to the wrong value logic is always not quiet right, we hope to
>> optimize this.
>>
>> Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
> LGTM
>> ---
>>  v1->v2: Modify note: overflowing -> overflow 
>>  drivers/block/loop.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
>> index 1518a6423279..1b35cbd029c7 100644
>> --- a/drivers/block/loop.c
>> +++ b/drivers/block/loop.c
>> @@ -977,13 +977,13 @@ loop_set_status_from_info(struct loop_device *lo,
>>  		return -EINVAL;
>>  	}
>>  
>> +	/* Avoid assigning overflow values */
>> +	if (info->lo_offset > LLONG_MAX || info->lo_sizelimit > LLONG_MAX)
>> +		return -EOVERFLOW;
>> +
>>  	lo->lo_offset = info->lo_offset;
>>  	lo->lo_sizelimit = info->lo_sizelimit;
>>  
>> -	/* loff_t vars have been assigned __u64 */
>> -	if (lo->lo_offset < 0 || lo->lo_sizelimit < 0)
>> -		return -EOVERFLOW;
>> -
>>  	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
>>  	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
>>  	lo->lo_flags = info->lo_flags;


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH-next v2] loop: loop_set_status_from_info() check before assignment
  2023-02-06  2:07 [PATCH-next v2] loop: loop_set_status_from_info() check before assignment Zhong Jinghua
  2023-02-06  2:55 ` Hou Tao
@ 2023-02-06  3:19 ` Yu Kuai
  1 sibling, 0 replies; 4+ messages in thread
From: Yu Kuai @ 2023-02-06  3:19 UTC (permalink / raw)
  To: Zhong Jinghua, axboe, code, Matthew Wilcox, Christoph Hellwig
  Cc: linux-block, linux-kernel, yi.zhang, houtao1, yangerkun, yukuai (C)

Hi,

在 2023/02/06 10:07, Zhong Jinghua 写道:
> In loop_set_status_from_info(), lo->lo_offset and lo->lo_sizelimit should
> be checked before reassignment, because if an overflow error occurs, the
> original correct value will be changed to the wrong value, and it will not
> be changed back.
> 
> Modifying to the wrong value logic is always not quiet right, we hope to
> optimize this.
> 

Please add a fix tag and cc stable:

Fixes: c490a0b5a4f3 ("loop: Check for overflow while configuring loop")

This commit doesn't fix the problem it described in commit message.

Thanks,
Kuai

> Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
> ---
>   v1->v2: Modify note: overflowing -> overflow
>   drivers/block/loop.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 1518a6423279..1b35cbd029c7 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -977,13 +977,13 @@ loop_set_status_from_info(struct loop_device *lo,
>   		return -EINVAL;
>   	}
>   
> +	/* Avoid assigning overflow values */
> +	if (info->lo_offset > LLONG_MAX || info->lo_sizelimit > LLONG_MAX)
> +		return -EOVERFLOW;
> +
>   	lo->lo_offset = info->lo_offset;
>   	lo->lo_sizelimit = info->lo_sizelimit;
>   
> -	/* loff_t vars have been assigned __u64 */
> -	if (lo->lo_offset < 0 || lo->lo_sizelimit < 0)
> -		return -EOVERFLOW;
> -
>   	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
>   	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
>   	lo->lo_flags = info->lo_flags;
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-06  3:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-06  2:07 [PATCH-next v2] loop: loop_set_status_from_info() check before assignment Zhong Jinghua
2023-02-06  2:55 ` Hou Tao
2023-02-06  3:02   ` Hou Tao
2023-02-06  3:19 ` Yu Kuai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.