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

Out-of-bounds read inside norm_inf #284

Closed
jwnimmer-tri opened this issue Aug 22, 2024 · 3 comments · Fixed by #285
Closed

Out-of-bounds read inside norm_inf #284

jwnimmer-tri opened this issue Aug 22, 2024 · 3 comments · Fixed by #285

Comments

@jwnimmer-tri
Copy link

jwnimmer-tri commented Aug 22, 2024

Specifications

  • OS: Ubuntu 22.04
  • SCS Version: 3.2.6
  • Compiler: GCC 11.4

Description

Out-of-bounds read inside norm_inf. This was a regression introduced by #278.

When running the latest SCS inside Drake under AddressSanitizer, it flags an out-of-bounds read. Investigating the situation, I can confirm that it is a true problem.

Here is a patch that fixes the problem, to help illustrate:

--- src/linalg.c
+++ src/linalg.c
@@ -153,6 +153,9 @@
 */
 
 scs_float SCS(norm_inf)(const scs_float *a, scs_int len) {
+  if (len <= 0) {
+    return 0;  // Follow the semantics of BLASI(lange) for zero-size array.
+  }
   blas_int bone = 1;
   blas_int blen = (blas_int)len;
   scs_int idx = (scs_int)BLASI(amax)(&blen, a, &bone);

The change in #278 switched to amax instead of lange, but failed to match the lange behavior when len == 0. In that case lange is specified to return zero, but instead #278 accidentally returns the -1th array value, which is undefined.

How to reproduce

I can extract this from our regression suite if necessary, but hopefully the bug is clear without this.

Edit: See below.

Output

Here is the backtrace during the error:

ERROR: AddressSanitizer: heap-buffer-overflow ... READ of size 8 ...
    #0 0x560df0ec5e80 in _scs_norm_inf external/scs_internal/src/linalg.c:160:10
    #1 0x560df0eed508 in compute_ruiz_mats external/scs_internal/linsys/scs_matrix.c:174:16
    #2 0x560df0eec060 in _scs_normalize_a_p external/scs_internal/linsys/scs_matrix.c:339:5
    #3 0x560df0ed24f1 in init_work external/scs_internal/src/scs.c:884:15
    #4 0x560df0ed0a0e in scs_init external/scs_internal/src/scs.c:1230:7
    #5 0x560df0ed2e8f in scs external/scs_internal/src/scs.c:1242:16
...
@jwnimmer-tri
Copy link
Author

Here's the write_data_filename output from the program that was crashing 284.zip.

@bodono
Copy link
Member

bodono commented Aug 24, 2024

Thank you for letting me know @jwnimmer-tri ! I will fix this and cut a new release asap.

@jwnimmer-tri
Copy link
Author

Works great, thanks for the quick new release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants