Skip to content

fix out-of-bounds access in orhr_col#1018

Merged
langou merged 1 commit intoReference-LAPACK:masterfrom
mgates3:orhr_col_fix
May 24, 2024
Merged

fix out-of-bounds access in orhr_col#1018
langou merged 1 commit intoReference-LAPACK:masterfrom
mgates3:orhr_col_fix

Conversation

@mgates3
Copy link
Copy Markdown
Collaborator

@mgates3 mgates3 commented May 23, 2024

Description
In orhr_col and unhr_col, it says "if NB > N, then N is used instead of NB" and LDT >= max( 1, min( NB, N ) ).
However, lines 423–425 have

            DO I = J-JBTEMP2, NB
               T( I, J ) = ZERO
            END DO

If N < NB and LDT = N, this will set T out-of-bounds. The LAPACK++ tester encounters an invalid pointer, typically due to memory being overwritten, and aborts:

methane lapackpp/test> ./tester  --type d --nb 384 --dim 100:500:100 --check n --ref n orhr_col
LAPACK++ version 2023.11.05, id f76dae7
input: ./tester --type d --nb 384 --dim 100:500:100 --check n --ref n orhr_col
                                                                       
type       m       n    nb     error   time (s)  ref time (s)  status  
free(): invalid pointer
Aborted (core dumped)

With NB ≤ N, it passes:

methane lapackpp/test> ./tester  --type d --nb 64 --dim 100:500:100 --check n --ref n orhr_col
LAPACK++ version 2023.11.05, id f76dae7
input: ./tester --type d --nb 64 --dim '100:500:100' --check n --ref n orhr_col
                                                                       
type       m       n    nb     error   time (s)  ref time (s)  status  
   d     100     100    64        NA   0.000475            NA  no check
   d     200     200    64        NA   0.000471            NA  no check
   d     300     300    64        NA    0.00106            NA  no check
   d     400     400    64        NA    0.00195            NA  no check
   d     500     500    64        NA    0.00305            NA  no check
All tests passed for orhr_col.

This is with reference LAPACK v3.12.0. Also occurs with intel-oneapi-mkl/2023.2.

Being Fortran, I guess the simplest solution is to use

            DO I = J-JBTEMP2, MIN( NB, N )

With this change, it passes:

methane lapackpp/test> ./tester  --type d --nb 384 --dim 100:500:100 --check n --ref n orhr_col
LAPACK++ version 2023.11.05, id f76dae7
input: ./tester --type d --nb 384 --dim '100:500:100' --check n --ref n orhr_col
                                                                       
type       m       n    nb     error   time (s)  ref time (s)  status  
   d     100     100   384        NA   0.000623            NA  no check
   d     200     200   384        NA    0.00113            NA  no check
   d     300     300   384        NA    0.00261            NA  no check
   d     400     400   384        NA    0.00525            NA  no check
   d     500     500   384        NA    0.00645            NA  no check
All tests passed for orhr_col.

(With C call-by-value semantics, I would simply set nb = min( nb, n ) near the top. With Fortran's call-by-reference semantics, that would require adding a local variable, nb_ = min( nb, n ) and changing all nb to nb_, right?)

Checklist

  • [NA] The documentation has been updated.
  • [NA] If the PR solves a specific issue, it is set to be closed on merge.

@mgates3
Copy link
Copy Markdown
Collaborator Author

mgates3 commented May 23, 2024

LAPACK++ commit c52719c works around this bug, so test with a version prior to that.

@mgates3 mgates3 mentioned this pull request May 24, 2024
Merged
@langou langou merged commit dd2e5ef into Reference-LAPACK:master May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martin-frbg martin-frbg martin-frbg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mgates3 @martin-frbg @langou