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

Bug in Configure out-of-tree directory path computation #22853

Closed
b-spencer opened this issue Nov 28, 2023 · 4 comments
Closed

Bug in Configure out-of-tree directory path computation #22853

b-spencer opened this issue Nov 28, 2023 · 4 comments
Assignees
Labels
branch: master Merge to master branch branch: 3.2 Merge to openssl-3.2 triaged: bug The issue/pr is/fixes a bug

Comments

@b-spencer
Copy link
Contributor

I think there's a bug in OpenSSL 3.2.0's Configure script that breaks out-of-tree configuration generation when the source tree is "several parent directories" away (and likely in other circumstances) and the source tree's parents to that same depth are not owned by you.

For example, with a freshly checked out copy of the openssl-3.2.0 tag:

~/sandbox/parent/openssl$ git log -n1
commit cf2877791ce7508684109664f467c9e40987692f (HEAD, tag: openssl-3.2.0)

~/sandbox/one/two/three/four/five$ ../../../../../parent/openssl/Configure linux-x86_64
Configuring OpenSSL version 3.2.0 for target linux-x86_64
Using os-specific seed configuration

Failure!  Makefile wasn't produced.
Please read INSTALL.md and associated NOTES-* files.  You may also have to
look over your available compiler tool chain or change your configuration.

mkdir /home/bspencer/sandbox/parent/openssl/../../../../../parent: Permission denied at ../../../../../parent/openssl/Configure line 1916.

I think this is caused by a small mistake in Configure on line 2476 (on the openssl-3.2.0 tag).

I think the $d variable needs to be computed using the original value of $i obtained from the regex match, before $i was recomputed by passing through cleanfile(). Changing this locally seemed to work for many different configuration targets and seems to produce the intended value in $d, although I couldn't be sure (thus I haven't prepared a PR).

@b-spencer b-spencer added the issue: bug report The issue was opened to report a bug label Nov 28, 2023
@t8m t8m added branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug branch: 3.2 Merge to openssl-3.2 and removed issue: bug report The issue was opened to report a bug labels Nov 28, 2023
@zhzhzoo-autra
Copy link

I agree with bspencer. https://github.com/openssl/openssl/pull/23482/files#diff-6defafa2caa65304173956e3fbc26b6947c50bc769c08c3da32c1032fa6519d4 solved the problem for me. And I think b8e98c3 did not actually fix the root cause of the problem.

@levitte
Copy link
Member

levitte commented Feb 7, 2024

I agree, that does look pretty odd.

Also, there are actually more bugs in there. For paths that are known to be directories, cleandir should be used rather than cleanfile

levitte added a commit to levitte/openssl that referenced this issue Feb 7, 2024
Configure was recently made to process this sort of line:

    DEPEND[generated]=util/perl|OpenSSL/something.pm

Unfortunately, in processing such lines, the order in which paths
were recomputed caused some resulting paths to be faulty under some
circumstances.  This change fixes that.

Fixes openssl#22853
@levitte
Copy link
Member

levitte commented Feb 7, 2024

Please try #23500

@levitte
Copy link
Member

levitte commented Feb 7, 2024

... and apologies for the wait. For some reason, this went under my radar for a bit.

openssl-machine pushed a commit that referenced this issue Feb 8, 2024
Configure was recently made to process this sort of line:

    DEPEND[generated]=util/perl|OpenSSL/something.pm

Unfortunately, in processing such lines, the order in which paths
were recomputed caused some resulting paths to be faulty under some
circumstances.  This change fixes that.

Fixes #22853

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #23500)
Sashan pushed a commit to Sashan/openssl that referenced this issue Feb 12, 2024
Configure was recently made to process this sort of line:

    DEPEND[generated]=util/perl|OpenSSL/something.pm

Unfortunately, in processing such lines, the order in which paths
were recomputed caused some resulting paths to be faulty under some
circumstances.  This change fixes that.

Fixes openssl#22853

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#23500)

(cherry picked from commit 64cae40)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch branch: 3.2 Merge to openssl-3.2 triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

No branches or pull requests

4 participants