LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [RESEND PATCH] time: changed timespec64_to_ns to avoid underrun
@ 2021-09-10 13:50 OPENSOURCE Lukas Hannen
  2021-09-11  0:48 ` Thomas Gleixner
  0 siblings, 1 reply; 4+ messages in thread
From: OPENSOURCE Lukas Hannen @ 2021-09-10 13:50 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner, EMC: linux-kernel@vger.kernel.org

This patch fixes a small oversight in timespec64_to_ns() that has resulted
in negative seconds being erroneously clamped to KTIME_MAX due to a cast
to unsigned long long (which expands to the 2's complement of a negative
long long, even if the architecture does not implement negative numbers
using 2's complement)

This is especially relevant in the PTP context, since the ptp_clock_info struct
(from include/linux/ptp_clock_kernel.h) specifies

        int (*adjtime)(struct ptp_clock_info *ptp, s64 delta);
        int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts);

which is exactly the kind of timespec64 / nanoseconds mix in combination with
negative values ( ns adjust times ) that can easily lead to calling timespec64_to_ns
with a negative ts->tv_sec, which would in turn lead to instability of the ptp clock.

Fixes: cb47755725da ("time: Prevent undefined behaviour in timespec64_to_ns()")'
Signed-off-by: Lukas Hannen <lukas.hannen@opensource.tttech-industrial.com>

---
The Patch should apply cleanly to all the branches that the original commit
cb47755725da ("time: Prevent undefined behaviour in timespec64_to_ns()")'
was backported to.

include/linux/time64.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/time64.h b/include/linux/time64.h
index 5117cb5b56561..81b9686a20799 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -21,15 +21,17 @@ struct itimerspec64 {  };

 /* Located here for timespec[64]_valid_strict */
 #define TIME64_MAX                     ((s64)~((u64)1 << 63))
 #define TIME64_MIN                     (-TIME64_MAX - 1)

 #define KTIME_MAX                      ((s64)~((u64)1 << 63))
+#define KTIME_MIN                      (-KTIME_MAX - 1)
 #define KTIME_SEC_MAX                  (KTIME_MAX / NSEC_PER_SEC)
+#define KTIME_SEC_MIN                  (KTIME_MIN / NSEC_PER_SEC)

 /*
  * Limits for settimeofday():
  *
  * To prevent setting the time close to the wraparound point time setting
  * is limited so a reasonable uptime can be accomodated. Uptime of 30 years
  * should be really sufficient, which means the cutoff is 2232. At that
@@ -120,18 +122,21 @@ static inline bool timespec64_valid_settod(const struct timespec64 *ts)
  * @ts:                pointer to the timespec64 variable to be converted
  *
  * Returns the scalar nanosecond representation of the timespec64
  * parameter.
  */
 static inline s64 timespec64_to_ns(const struct timespec64 *ts)  {
-       /* Prevent multiplication overflow */
-       if ((unsigned long long)ts->tv_sec >= KTIME_SEC_MAX)
+       /* Prevent multiplication overflow / underflow */
+       if (ts->tv_sec >= KTIME_SEC_MAX)
                return KTIME_MAX;

+       if (ts->tv_sec <= KTIME_SEC_MIN)
+               return KTIME_MIN;
+
        return ((s64) ts->tv_sec * NSEC_PER_SEC) + ts->tv_nsec;  }

 /**
  * ns_to_timespec64 - Convert nanoseconds to timespec64
  * @nsec:      the nanoseconds value to be converted
  *
--
2.31.1

Best Regards,

Lukas Hannen


Internal

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

* Re: [RESEND PATCH] time: changed timespec64_to_ns to avoid underrun
  2021-09-10 13:50 [RESEND PATCH] time: changed timespec64_to_ns to avoid underrun OPENSOURCE Lukas Hannen
@ 2021-09-11  0:48 ` Thomas Gleixner
  2021-09-13  6:49   ` OPENSOURCE Lukas Hannen
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Gleixner @ 2021-09-11  0:48 UTC (permalink / raw)
  To: OPENSOURCE Lukas Hannen, John Stultz, EMC: linux-kernel@vger.kernel.org

Lukas,

On Fri, Sep 10 2021 at 13:50, OPENSOURCE Lukas Hannen wrote:

> This patch fixes a small oversight in timespec64_to_ns() that has resulted
...

why you are resending this patch?

1) The only change you did is adding a prefix to the subject line

   which in fact disqualifies it from the RESEND tag because RESEND
   means it's unmodified or just forward ported.

   Changed patches even if the change is just in the subject line or the
   changelog want a version number.

   But what's worse:

2) You ignored any other review comment I gave here:

   https://lore.kernel.org/r/87y2876pg0.ffs@tglx

   IOW, you stopped reading at line 12 of my reply

3) Due to that you failed to notice that I said in that reply:

   "I fixed it up for you this time."

4) Of course you also ignored the fact that I actually fixed the
   identified issues and the fact that the fixed up patch is already
   applied to my tree.

   You got notfied about that:

   https://lore.kernel.org/r/163111620295.25758.18154572095175068828.tip-bot2@tip-bot2

So what exactly are you trying to achieve by "resending" a patch which
still does not apply and still has a non-sensical subject line and
changelog?

> CONFIDENTIALITY: The contents of this e-mail are confidential and
> intended only for the above addressee(s). If you are not the intended
> recipient, or the person responsible for delivering it to the intended
> recipient, copying or delivering it to anyone else or using it in any
> unauthorized manner is prohibited and may be unlawful. If you receive
> this e-mail by mistake, please notify the sender and the systems
> administrator at straymail@tttech.com immediately.

Please get rid of this nonsensical disclaimer.

 When posting to public mailing lists the boilerplate confidentiality
 disclaimers are not only meaningless, they are absolutely wrong for
 obvious reasons.

 See https://people.kernel.org/tglx/notes-about-netiquette for further
 information.

Thanks,

        tglx

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

* RE: [RESEND PATCH] time: changed timespec64_to_ns to avoid underrun
  2021-09-11  0:48 ` Thomas Gleixner
@ 2021-09-13  6:49   ` OPENSOURCE Lukas Hannen
  2021-09-13  8:23     ` Thomas Gleixner
  0 siblings, 1 reply; 4+ messages in thread
From: OPENSOURCE Lukas Hannen @ 2021-09-13  6:49 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz, EMC: linux-kernel@vger.kernel.org

> Lukas,

> why you are resending this patch?

(also ad 2,3 and 4: ) unfortunately our company mail system never showed me
your replies to my original mail.

> 1) The only change you did is adding a prefix to the subject line
>   Changed patches even if the change is just in the subject line or the
>   changelog want a version number.

I was unsure if the Subject line was considered part of the patch, and noticed
the errors in the subject line on my own, and thus thought my previous mail
got ignored because I had messed up the subject line.

>   But what's worse:
>
> 2) You ignored any other review comment I gave here:

I greatly appreciate your help in regards to commenting style and content.
I am obviously entirely new to this community.

> So what exactly are you trying to achieve by "resending" a patch which still
> does not apply and still has a non-sensical subject line and changelog?

You were right in assuming that the format of the patch was messed up by
our company mail, I am as infuriated about that as you are and will try to
change this situation asap.

The other mishaps (like the wrong use of the RESEND tag, failing to check
if the changes were already applied to your tree etc. ) were caused by me
being a young and inexperienced contributor.

> Thanks,

Thank _You_ for not immediately ignoring me forever,

Lukas Hannen

Public

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

* RE: [RESEND PATCH] time: changed timespec64_to_ns to avoid underrun
  2021-09-13  6:49   ` OPENSOURCE Lukas Hannen
@ 2021-09-13  8:23     ` Thomas Gleixner
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Gleixner @ 2021-09-13  8:23 UTC (permalink / raw)
  To: OPENSOURCE Lukas Hannen, John Stultz, EMC: linux-kernel@vger.kernel.org

Lukas,

On Mon, Sep 13 2021 at 06:49, OPENSOURCE Lukas Hannen wrote:
> (also ad 2,3 and 4: ) unfortunately our company mail system never showed me
> your replies to my original mail.

...

> You were right in assuming that the format of the patch was messed up by
> our company mail, I am as infuriated about that as you are and will try to
> change this situation asap.
>
> The other mishaps (like the wrong use of the RESEND tag, failing to check
> if the changes were already applied to your tree etc. ) were caused by me
> being a young and inexperienced contributor.

No problem. I was certainly not expecting that the resend was caused by
the lack of incoming mail on your side.

> Thank _You_ for not immediately ignoring me forever,

There's no reason to do so.

Thanks,

        tglx

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

end of thread, other threads:[~2021-09-13  8:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-10 13:50 [RESEND PATCH] time: changed timespec64_to_ns to avoid underrun OPENSOURCE Lukas Hannen
2021-09-11  0:48 ` Thomas Gleixner
2021-09-13  6:49   ` OPENSOURCE Lukas Hannen
2021-09-13  8:23     ` Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).