Received-SPF: pass (google.com: domain of linux-kernel+bounces-24848-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Thomas Gleixner <tglx@linutronix.de>
To: Simone Weiss <simone.weiss@elektrobit.com>
Cc: simone.weiss@elektrobit.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Fix UBSAN warning for subtracting ktime_t
In-Reply-To: <20231219124434.870613-1-simone.weiss@elektrobit.com>
References: <20231219124434.870613-1-simone.weiss@elektrobit.com>
Date: Fri, 12 Jan 2024 17:14:00 +0100
Message-ID: <878r4uil6f.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

On Tue, Dec 19 2023 at 13:44, Simone Weiss wrote:
> UBSAN: Undefined behaviour in kernel/time/hrtimer.c:612:10
> signed integer overflow:
> 9223372036854775807 - -51224496 cannot be represented in type
> 'long long int'

Some explanation about the context and why the offset is < 0 would be
helpful.

> +/*
> + * Sub two ktime values and do a safety check for overflow:
> + */
> +ktime_t ktime_sub_safe(const ktime_t lhs, const ktime_t rhs)
> +{
> +	ktime_t res = ktime_sub_unsafe(lhs, rhs);
> +
> +	if (lhs > 0 && rhs < 0 && res < 0)
> +		res = ktime_set(KTIME_SEC_MAX, 0);
> +	else if (lhs < 0 && rhs > 0 && res > 0)
> +		res = ktime_set(-KTIME_SEC_MAX, 0);

Looking at the use cases, the lhs < 0 case would be a bug because the
expiry values are strictly >= 0.

> +
> +	return res;
> +}
> +EXPORT_SYMBOL_GPL(ktime_sub_safe);

That export is needed because? The only usage is in this file so this
can be static, no?

Thanks,

        tglx