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

Configurations: mips64*-linux-*abin32 needs bn_ops SIXTY_FOUR_BIT #19320

Closed
wants to merge 1 commit into from
Closed

Configurations: mips64*-linux-*abin32 needs bn_ops SIXTY_FOUR_BIT #19320

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 1, 2022

The IRIX mips64-cpu, n32-abi configurations include SIXTY_FOUR_BIT in bn_ops, but it is missing from mips64*-linux-*abin32 (which OpenSSL calls "linux-mips64"). This causes heap corruption when verifying TLS certificates (which tend to be RSA-signed) with openssl 1.1.1q:

nix@oak:~$ /nix/store/4k04dh6a1zs6hxiacwcg4a4nvxvgli2j-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443free(): invalid pointer
Aborted

and a slightly different failure with current HEAD:

nix@oak:~$ /nix/store/9bqxharxajsl9fid0c8ls6fb9wxp8kdc-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443
Connecting to 142.250.180.4
CONNECTED(00000003)
Fatal glibc error: malloc assertion failure in sysmalloc: (old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)
Aborted

Applying this patch and recompiling produces the expected output instead of a crash.

Note that Gentoo (and to my knowledge all other other distributions which support mips64n32) use the linux-generic32 configuration, which uses only 32-bit arithmetic (rather than full 64-bit arithmetic) and lacks assembler implementations for the SHA hash functions:

https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/openssl/files/gentoo.config-1.0.2#n102

For support in nixpkgs we would like to use the full 64-bit integer registers and perlasm routines, so I'm submitting this upstream as well.

Fixes #19319

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Oct 1, 2022
@levitte
Copy link
Member

levitte commented Oct 1, 2022

The CLA checker complains about the lack of CLA for the email address in your commit. In this case, it would be legitimate to declare the commit to be trivial, which is done by adding this at the end of the commit message, separated from the rest of the commit message with an empty line:

CLA: trivial

Of course, you are welcome to submit a CLA if you prefer, see https://www.openssl.org/policies/cla.html

@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Oct 1, 2022
@ghost ghost requested a review from levitte October 1, 2022 04:29
The IRIX mips64-cpu, n32-abi configurations include SIXTY_FOUR_BIT in bn_ops,
but it is missing from mips64*-linux-*abin32 (which OpenSSL calls
"linux-mips64").  This causes heap corruption when verifying TLS certificates
(which tend to be RSA-signed) with openssl 1.1.1q:

```
nix@oak:~$ /nix/store/4k04dh6a1zs6hxiacwcg4a4nvxvgli2j-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443free(): invalid pointer
Aborted
```

and a slightly different failure with current HEAD:

```
nix@oak:~$ /nix/store/9bqxharxajsl9fid0c8ls6fb9wxp8kdc-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443
Connecting to 142.250.180.4
CONNECTED(00000003)
Fatal glibc error: malloc assertion failure in sysmalloc: (old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)
Aborted
```

Applying this patch and recompiling produces the expected output instead of a
crash.

Note that Gentoo (and to my knowledge all other other distributions which
support mips64n32) use the `linux-generic32` configuration, which uses only
32-bit arithmetic (rather than full 64-bit arithmetic) and lacks assembler
implementations for the SHA hash functions:

  https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/openssl/files/gentoo.config-1.0.2#n102

For support in nixpkgs we would like to use the full 64-bit integer registers
and perlasm routines, so I'm submitting this upstream as well.

Fixes #19319

CLA: trivial
@bernd-edlinger bernd-edlinger added branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch branch: 3.0 Merge to openssl-3.0 branch labels Oct 1, 2022
@bernd-edlinger
Copy link
Member

I've added the 3.0 and 1.1.1 labels,
I assume there will be a trivial merge conflict for 1.1.1, though, therefore an extra PR for 1.1.1 would be welcome.

Copy link
Member

@bernd-edlinger bernd-edlinger left a comment

Choose a reason for hiding this comment

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

LGTM for all branches.

@t8m t8m added the triaged: bug The issue/pr is/fixes a bug label Oct 1, 2022
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

OK with CLA: trivial.

@t8m t8m added the approval: done This pull request has the required number of approvals label Oct 1, 2022
@ghost
Copy link
Author

ghost commented Oct 1, 2022

therefore an extra PR for 1.1.1 would be welcome.

#19321

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@paulidale
Copy link
Contributor

Merged to all three branches. Thanks for your contribution.

@paulidale paulidale closed this Oct 3, 2022
openssl-machine pushed a commit that referenced this pull request Oct 3, 2022
The IRIX mips64-cpu, n32-abi configurations include SIXTY_FOUR_BIT in bn_ops,
but it is missing from mips64*-linux-*abin32 (which OpenSSL calls
"linux-mips64").  This causes heap corruption when verifying TLS certificates
(which tend to be RSA-signed) with openssl 1.1.1q:

```
nix@oak:~$ /nix/store/4k04dh6a1zs6hxiacwcg4a4nvxvgli2j-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443free(): invalid pointer
Aborted
```

and a slightly different failure with current HEAD:

```
nix@oak:~$ /nix/store/9bqxharxajsl9fid0c8ls6fb9wxp8kdc-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443
Connecting to 142.250.180.4
CONNECTED(00000003)
Fatal glibc error: malloc assertion failure in sysmalloc: (old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)
Aborted
```

Applying this patch and recompiling produces the expected output instead of a
crash.

Note that Gentoo (and to my knowledge all other other distributions which
support mips64n32) use the `linux-generic32` configuration, which uses only
32-bit arithmetic (rather than full 64-bit arithmetic) and lacks assembler
implementations for the SHA hash functions:

  https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/openssl/files/gentoo.config-1.0.2#n102

For support in nixpkgs we would like to use the full 64-bit integer registers
and perlasm routines, so I'm submitting this upstream as well.

Fixes #19319

CLA: trivial

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #19320)

(cherry picked from commit d250e85)
openssl-machine pushed a commit that referenced this pull request Oct 3, 2022
The IRIX mips64-cpu, n32-abi configurations include SIXTY_FOUR_BIT in bn_ops,
but it is missing from mips64*-linux-*abin32 (which OpenSSL calls
"linux-mips64").  This causes heap corruption when verifying TLS certificates
(which tend to be RSA-signed) with openssl 1.1.1q:

```
nix@oak:~$ /nix/store/4k04dh6a1zs6hxiacwcg4a4nvxvgli2j-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443free(): invalid pointer
Aborted
```

and a slightly different failure with current HEAD:

```
nix@oak:~$ /nix/store/9bqxharxajsl9fid0c8ls6fb9wxp8kdc-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443
Connecting to 142.250.180.4
CONNECTED(00000003)
Fatal glibc error: malloc assertion failure in sysmalloc: (old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)
Aborted
```

Applying this patch and recompiling produces the expected output instead of a
crash.

Note that Gentoo (and to my knowledge all other other distributions which
support mips64n32) use the `linux-generic32` configuration, which uses only
32-bit arithmetic (rather than full 64-bit arithmetic) and lacks assembler
implementations for the SHA hash functions:

  https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/openssl/files/gentoo.config-1.0.2#n102

For support in nixpkgs we would like to use the full 64-bit integer registers
and perlasm routines, so I'm submitting this upstream as well.

Fixes #19319

CLA: trivial

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #19320)
openssl-machine pushed a commit that referenced this pull request Oct 3, 2022
The IRIX mips64-cpu, n32-abi configurations include SIXTY_FOUR_BIT in bn_ops,
but it is missing from mips64*-linux-*abin32 (which OpenSSL calls
"linux-mips64").  This causes heap corruption when verifying TLS certificates
(which tend to be RSA-signed) with openssl 1.1.1q:

```
nix@oak:~$ /nix/store/4k04dh6a1zs6hxiacwcg4a4nvxvgli2j-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443free(): invalid pointer
Aborted
```

and a slightly different failure with current HEAD:

```
nix@oak:~$ /nix/store/9bqxharxajsl9fid0c8ls6fb9wxp8kdc-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443
Connecting to 142.250.180.4
CONNECTED(00000003)
Fatal glibc error: malloc assertion failure in sysmalloc: (old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)
Aborted
```

Applying this patch and recompiling produces the expected output instead of a
crash.

Note that Gentoo (and to my knowledge all other other distributions which
support mips64n32) use the `linux-generic32` configuration, which uses only
32-bit arithmetic (rather than full 64-bit arithmetic) and lacks assembler
implementations for the SHA hash functions:

  https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/openssl/files/gentoo.config-1.0.2#n102

For support in nixpkgs we would like to use the full 64-bit integer registers
and perlasm routines, so I'm submitting this upstream as well.

Fixes #19319

CLA: trivial

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #19320)

(cherry picked from commit d250e85)
@ghost
Copy link
Author

ghost commented Oct 3, 2022

Merged to all three branches. Thanks for your contribution.

01b5335

9f0867d

@mattcaswell
Copy link
Member

Merged to all three branches.

1.1.1 is in security fix only mode...probably this should only have gone to master and 3.0.

@paulidale
Copy link
Contributor

It meets the configuration addition only criteria.

Happy to roll it out of 1.1.1.

beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
The IRIX mips64-cpu, n32-abi configurations include SIXTY_FOUR_BIT in bn_ops,
but it is missing from mips64*-linux-*abin32 (which OpenSSL calls
"linux-mips64").  This causes heap corruption when verifying TLS certificates
(which tend to be RSA-signed) with openssl 1.1.1q:

```
nix@oak:~$ /nix/store/4k04dh6a1zs6hxiacwcg4a4nvxvgli2j-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443free(): invalid pointer
Aborted
```

and a slightly different failure with current HEAD:

```
nix@oak:~$ /nix/store/9bqxharxajsl9fid0c8ls6fb9wxp8kdc-openssl-mips64el-unknown-linux-gnuabin32-1.1.1q-bin/bin/openssl s_client -host www.google.com -port 443
Connecting to 142.250.180.4
CONNECTED(00000003)
Fatal glibc error: malloc assertion failure in sysmalloc: (old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)
Aborted
```

Applying this patch and recompiling produces the expected output instead of a
crash.

Note that Gentoo (and to my knowledge all other other distributions which
support mips64n32) use the `linux-generic32` configuration, which uses only
32-bit arithmetic (rather than full 64-bit arithmetic) and lacks assembler
implementations for the SHA hash functions:

  https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/openssl/files/gentoo.config-1.0.2#n102

For support in nixpkgs we would like to use the full 64-bit integer registers
and perlasm routines, so I'm submitting this upstream as well.

Fixes openssl#19319

CLA: trivial

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#19320)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch branch: 3.0 Merge to openssl-3.0 branch triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

./Configure mips-linux64 (i.e. mips64n32) causes heap corruption
6 participants