Linux-RTC Archive on lore.kernel.org
 help / Atom feed
* [PATCH] rtc: Fix UBSAN overflow warning
@ 2018-12-20  9:36 ZhangXiaoxu
  2018-12-20 11:07 ` Alexandre Belloni
  2019-01-10 21:03 ` Alexandre Belloni
  0 siblings, 2 replies; 4+ messages in thread
From: ZhangXiaoxu @ 2018-12-20  9:36 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, linux-rtc

Users may call 'ioctl' and pass a very big value on 'tm->tm_year'.
It can be overflowed in 'int' after add 1900.
In function 'rtc_month_days' and 'mktime64', also treated it as an
'unsigned' parameter.

UBSAN: Undefined behaviour in drivers/rtc/rtc-lib.c:103:59
signed integer overflow:
2147483647 + 1900 cannot be represented in type 'int'

UBSAN: Undefined behaviour in drivers/rtc/rtc-lib.c:119:30
signed integer overflow:
2147483647 + 1900 cannot be represented in type 'int'

So, covert it to 'unsigned' explicitly.

Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
---
 drivers/rtc/rtc-lib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-lib.c b/drivers/rtc/rtc-lib.c
index ef160da..9714cb3 100644
--- a/drivers/rtc/rtc-lib.c
+++ b/drivers/rtc/rtc-lib.c
@@ -100,7 +100,7 @@ int rtc_valid_tm(struct rtc_time *tm)
 	if (tm->tm_year < 70
 		|| ((unsigned)tm->tm_mon) >= 12
 		|| tm->tm_mday < 1
-		|| tm->tm_mday > rtc_month_days(tm->tm_mon, tm->tm_year + 1900)
+		|| tm->tm_mday > rtc_month_days(tm->tm_mon, ((unsigned)tm->tm_year + 1900))
 		|| ((unsigned)tm->tm_hour) >= 24
 		|| ((unsigned)tm->tm_min) >= 60
 		|| ((unsigned)tm->tm_sec) >= 60)
@@ -116,8 +116,8 @@ EXPORT_SYMBOL(rtc_valid_tm);
  */
 time64_t rtc_tm_to_time64(struct rtc_time *tm)
 {
-	return mktime64(tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
-			tm->tm_hour, tm->tm_min, tm->tm_sec);
+	return mktime64(((unsigned)tm->tm_year + 1900), tm->tm_mon + 1,
+			tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec);
 }
 EXPORT_SYMBOL(rtc_tm_to_time64);
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rtc: Fix UBSAN overflow warning
  2018-12-20  9:36 [PATCH] rtc: Fix UBSAN overflow warning ZhangXiaoxu
@ 2018-12-20 11:07 ` Alexandre Belloni
  2018-12-20 14:11   ` zhangxiaoxu (A)
  2019-01-10 21:03 ` Alexandre Belloni
  1 sibling, 1 reply; 4+ messages in thread
From: Alexandre Belloni @ 2018-12-20 11:07 UTC (permalink / raw)
  To: ZhangXiaoxu; +Cc: a.zummo, linux-rtc

Hi,

On 20/12/2018 17:36:56+0800, ZhangXiaoxu wrote:
> Users may call 'ioctl' and pass a very big value on 'tm->tm_year'.
> It can be overflowed in 'int' after add 1900.
> In function 'rtc_month_days' and 'mktime64', also treated it as an
> 'unsigned' parameter.
> 
> UBSAN: Undefined behaviour in drivers/rtc/rtc-lib.c:103:59
> signed integer overflow:
> 2147483647 + 1900 cannot be represented in type 'int'
> 
> UBSAN: Undefined behaviour in drivers/rtc/rtc-lib.c:119:30
> signed integer overflow:
> 2147483647 + 1900 cannot be represented in type 'int'
> 
> So, covert it to 'unsigned' explicitly.
> 
> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
> ---
>  drivers/rtc/rtc-lib.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-lib.c b/drivers/rtc/rtc-lib.c
> index ef160da..9714cb3 100644
> --- a/drivers/rtc/rtc-lib.c
> +++ b/drivers/rtc/rtc-lib.c
> @@ -100,7 +100,7 @@ int rtc_valid_tm(struct rtc_time *tm)
>  	if (tm->tm_year < 70
>  		|| ((unsigned)tm->tm_mon) >= 12
>  		|| tm->tm_mday < 1
> -		|| tm->tm_mday > rtc_month_days(tm->tm_mon, tm->tm_year + 1900)
> +		|| tm->tm_mday > rtc_month_days(tm->tm_mon, ((unsigned)tm->tm_year + 1900))

Isn't the cast to unsigned done by rtc_month_days enough?

>  		|| ((unsigned)tm->tm_hour) >= 24
>  		|| ((unsigned)tm->tm_min) >= 60
>  		|| ((unsigned)tm->tm_sec) >= 60)
> @@ -116,8 +116,8 @@ EXPORT_SYMBOL(rtc_valid_tm);
>   */
>  time64_t rtc_tm_to_time64(struct rtc_time *tm)
>  {
> -	return mktime64(tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
> -			tm->tm_hour, tm->tm_min, tm->tm_sec);
> +	return mktime64(((unsigned)tm->tm_year + 1900), tm->tm_mon + 1,
> +			tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec);

mktime64 will fail way before tm->tm_year + 1900 overflows an int and
also it already takes an unsigned int for year so I'm not sure this cast
is actually necessary.

>  }
>  EXPORT_SYMBOL(rtc_tm_to_time64);
>  
> -- 
> 2.7.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rtc: Fix UBSAN overflow warning
  2018-12-20 11:07 ` Alexandre Belloni
@ 2018-12-20 14:11   ` zhangxiaoxu (A)
  0 siblings, 0 replies; 4+ messages in thread
From: zhangxiaoxu (A) @ 2018-12-20 14:11 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: a.zummo, linux-rtc



On 12/20/2018 7:07 PM, Alexandre Belloni wrote:
> Hi,
> 
> On 20/12/2018 17:36:56+0800, ZhangXiaoxu wrote:
>> Users may call 'ioctl' and pass a very big value on 'tm->tm_year'.
>> It can be overflowed in 'int' after add 1900.
>> In function 'rtc_month_days' and 'mktime64', also treated it as an
>> 'unsigned' parameter.
>>
>> UBSAN: Undefined behaviour in drivers/rtc/rtc-lib.c:103:59
>> signed integer overflow:
>> 2147483647 + 1900 cannot be represented in type 'int'
>>
>> UBSAN: Undefined behaviour in drivers/rtc/rtc-lib.c:119:30
>> signed integer overflow:
>> 2147483647 + 1900 cannot be represented in type 'int'
>>
>> So, covert it to 'unsigned' explicitly.
>>
>> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
>> ---
>>   drivers/rtc/rtc-lib.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-lib.c b/drivers/rtc/rtc-lib.c
>> index ef160da..9714cb3 100644
>> --- a/drivers/rtc/rtc-lib.c
>> +++ b/drivers/rtc/rtc-lib.c
>> @@ -100,7 +100,7 @@ int rtc_valid_tm(struct rtc_time *tm)
>>   	if (tm->tm_year < 70
>>   		|| ((unsigned)tm->tm_mon) >= 12
>>   		|| tm->tm_mday < 1
>> -		|| tm->tm_mday > rtc_month_days(tm->tm_mon, (unsigned)(tm->tm_year + 1900))
>> +		|| tm->tm_mday > rtc_month_days(tm->tm_mon, ((unsigned)tm->tm_year + 1900))
> 
> Isn't the cast to unsigned done by rtc_month_days enough?
The undefined behaviour is 'tm->tm_year + 1900', rtc_month_days just convert the result to unsigned.
Also, signed integer overflow is undefined according to the C standard.
> 
>>   		|| ((unsigned)tm->tm_hour) >= 24
>>   		|| ((unsigned)tm->tm_min) >= 60
>>   		|| ((unsigned)tm->tm_sec) >= 60)
>> @@ -116,8 +116,8 @@ EXPORT_SYMBOL(rtc_valid_tm);
>>    */
>>   time64_t rtc_tm_to_time64(struct rtc_time *tm)
>>   {
>> -	return mktime64(tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
>> -			tm->tm_hour, tm->tm_min, tm->tm_sec);
>> +	return mktime64(((unsigned)tm->tm_year + 1900), tm->tm_mon + 1,
>> +			tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec);
> 
> mktime64 will fail way before tm->tm_year + 1900 overflows an int and
> also it already takes an unsigned int for year so I'm not sure this cast
> is actually necessary.
> 
>>   }
>>   EXPORT_SYMBOL(rtc_tm_to_time64);
>>   
>> -- 
>> 2.7.4
>>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rtc: Fix UBSAN overflow warning
  2018-12-20  9:36 [PATCH] rtc: Fix UBSAN overflow warning ZhangXiaoxu
  2018-12-20 11:07 ` Alexandre Belloni
@ 2019-01-10 21:03 ` Alexandre Belloni
  1 sibling, 0 replies; 4+ messages in thread
From: Alexandre Belloni @ 2019-01-10 21:03 UTC (permalink / raw)
  To: ZhangXiaoxu; +Cc: a.zummo, linux-rtc

On 20/12/2018 17:36:56+0800, ZhangXiaoxu wrote:
> Users may call 'ioctl' and pass a very big value on 'tm->tm_year'.
> It can be overflowed in 'int' after add 1900.
> In function 'rtc_month_days' and 'mktime64', also treated it as an
> 'unsigned' parameter.
> 
> UBSAN: Undefined behaviour in drivers/rtc/rtc-lib.c:103:59
> signed integer overflow:
> 2147483647 + 1900 cannot be represented in type 'int'
> 
> UBSAN: Undefined behaviour in drivers/rtc/rtc-lib.c:119:30
> signed integer overflow:
> 2147483647 + 1900 cannot be represented in type 'int'
> 
> So, covert it to 'unsigned' explicitly.
> 
> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
> ---
>  drivers/rtc/rtc-lib.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-20  9:36 [PATCH] rtc: Fix UBSAN overflow warning ZhangXiaoxu
2018-12-20 11:07 ` Alexandre Belloni
2018-12-20 14:11   ` zhangxiaoxu (A)
2019-01-10 21:03 ` Alexandre Belloni

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org linux-rtc@archiver.kernel.org
	public-inbox-index linux-rtc


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/ public-inbox