RE: [PATCH] time: Avoid undefined behaviour in timespec64_to_ns
> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Tuesday, September 15, 2020 8:45 PM
> To: Zengtao (B)
> Cc: Thomas Gleixner; Vincenzo Frascino; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] time: Avoid undefined behaviour in
> timespec64_to_ns
>
> On Tue, Sep 15, 2020 at 2:20 PM Zengtao (B)
> wrote:
>
> > > > Fixes: bd40a175769d ("y2038: itimer: change implementation to
> > > timespec64")
> > >
> > > This one caused the regression, but if we add the check here, it
> > > may be best to also add it in prior kernels that may have the same
> > > bug in other callers of the same function. Maybe backport all the
> > > way to stable kernels that first added timespec64?
> > >
> >
> > I think we need to do the backport, but not sure about the start
> point
> > Thanks for your review.
>
> I would suggest
>
> Cc: # v3.17+
> Fixes: 361a3bf00582 ("time64: Add time64.h header and define
> struct timespec64")
Yes, make sense, commit 361a3bf00582 introduce a potential issue and
commit bd40a175769d trigger the issue.
Regards
Zengtao
>
>Arnd
Re: [PATCH] time: Avoid undefined behaviour in timespec64_to_ns
On Tue, Sep 15, 2020 at 2:20 PM Zengtao (B) wrote:
> > > Fixes: bd40a175769d ("y2038: itimer: change implementation to
> > timespec64")
> >
> > This one caused the regression, but if we add the check here, it
> > may be best to also add it in prior kernels that may have the same
> > bug in other callers of the same function. Maybe backport all the
> > way to stable kernels that first added timespec64?
> >
>
> I think we need to do the backport, but not sure about the start point
> Thanks for your review.
I would suggest
Cc: # v3.17+
Fixes: 361a3bf00582 ("time64: Add time64.h header and define struct timespec64")
Arnd
RE: [PATCH] time: Avoid undefined behaviour in timespec64_to_ns
> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Tuesday, September 01, 2020 8:47 PM
> To: Zengtao (B)
> Cc: Thomas Gleixner; Vincenzo Frascino; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] time: Avoid undefined behaviour in
> timespec64_to_ns
>
> On Tue, Sep 1, 2020 at 11:32 AM Zeng Tao
> wrote:
> >
> > Since commit bd40a175769d ("y2038: itimer: change
> implementation to timespec64")
> > we have break the time clamping which handles the potential
> overflow.
>
> Indeed, good catch!
>
> And I broke it despite the comment telling me about the problem.
>
> > In this patch, we fix it in the timespec64_to_ns because there is
> > possiblity to cause the same undefined behaviour on overflow
> whenever
> > the function is called.
>
> I checked the most important callers of this function, and I agree
> that truncating at the maximum would be sensible in most cases
> here.
>
> In timekeeping_init(), there is already a check for
> timespec64_valid_settod() that limits it even further, but that
> wouldn't make sense for most callers.
>
> > Fixes: bd40a175769d ("y2038: itimer: change implementation to
> timespec64")
>
> This one caused the regression, but if we add the check here, it
> may be best to also add it in prior kernels that may have the same
> bug in other callers of the same function. Maybe backport all the
> way to stable kernels that first added timespec64?
>
I think we need to do the backport, but not sure about the start point
Thanks for your review.
> Cc # v3.17+
>
> > Signed-off-by: Zeng Tao
>
> Reviewed-by: Arnd Bergmann
Re: [PATCH] time: Avoid undefined behaviour in timespec64_to_ns
On Tue, Sep 1, 2020 at 11:32 AM Zeng Tao wrote:
>
> Since commit bd40a175769d ("y2038: itimer: change implementation to
> timespec64")
> we have break the time clamping which handles the potential overflow.
Indeed, good catch!
And I broke it despite the comment telling me about the problem.
> In this patch, we fix it in the timespec64_to_ns because there is
> possiblity to cause the same undefined behaviour on overflow whenever
> the function is called.
I checked the most important callers of this function, and I agree
that truncating at the maximum would be sensible in most cases
here.
In timekeeping_init(), there is already a check for
timespec64_valid_settod() that limits it even further, but that
wouldn't make sense for most callers.
> Fixes: bd40a175769d ("y2038: itimer: change implementation to timespec64")
This one caused the regression, but if we add the check here, it
may be best to also add it in prior kernels that may have the same
bug in other callers of the same function. Maybe backport all the
way to stable kernels that first added timespec64?
Cc # v3.17+
> Signed-off-by: Zeng Tao
Reviewed-by: Arnd Bergmann
