Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing potential overflow issue in inner product trait #2397

Merged
merged 2 commits into from
Oct 25, 2024

Conversation

lucbv
Copy link
Contributor

@lucbv lucbv commented Oct 24, 2024

When result type is double and inputs are floats, one input has to be cast to double so the multiplication operator for double is used instead of the float multiplication operator that could overflow for valid double values.

@csiefer2 this should take care of the security issue you reported.

@lucbv lucbv added the bug label Oct 24, 2024
@lucbv lucbv self-assigned this Oct 24, 2024
Copy link
Contributor

@yasahi-hpc yasahi-hpc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Reasonable to avoid a potential overflow.

Oh, I have one question about complex overload.

KOKKOS_INLINE_FUNCTION void updateDot(Kokkos::complex<double>& sum, const Kokkos::complex<float> x,
                                      const Kokkos::complex<float> y) {
  const auto tmp = Kokkos::conj(x) * y;
  sum += Kokkos::complex<double>(tmp.real(), tmp.imag());
}

Do you need to apply casting for this case like

KOKKOS_INLINE_FUNCTION void updateDot(Kokkos::complex<double>& sum, const Kokkos::complex<float> x,
                                      const Kokkos::complex<float> y) {
  const auto tmp_x = Kokkos::complex<double>(x.real(), x.imag());
  const auto tmp_y = Kokkos::complex<double>(y.real(), y.imag());
  sum += Kokkos::conj(tmp_x) * tmp_y;
}

lucbv added 2 commits October 25, 2024 11:38
When result type is double and inputs are floats, one input has
to be cast to double so the multiplication operator for double
is used instead of the float multiplication operator that could
overflow for valid double values.

Handle the complex case for mixed input/output fp types

Signed-off-by: Luc Berger-Vergiat <[email protected]>
@lucbv lucbv force-pushed the convert_intermediate_fp branch from 30163ca to 07da18c Compare October 25, 2024 17:39
@lucbv lucbv merged commit 585a59f into kokkos:develop Oct 25, 2024
19 checks passed
@lucbv lucbv deleted the convert_intermediate_fp branch October 28, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants