Navigation Menu

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

Fix compilation of AF_ALG engine #4617

Closed
wants to merge 2 commits into from
Closed

Conversation

zorun
Copy link

@zorun zorun commented Oct 30, 2017

Two small commits for AF_ALG engine, one fixing a kernel version comparison, and the other one fixing compilation on aarch64 (#1685)

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Oct 30, 2017
@zorun
Copy link
Author

zorun commented Oct 30, 2017

By the way, this should be backported to the 1.1 stable branch, but can't be cherry-picked because the files have been moved around. Should I send another pull request against OpenSSL_1_1_0-stable?

@levitte
Copy link
Member

levitte commented Oct 30, 2017

A parallell PR for 1.1.0 would be nice, thanks.

@levitte
Copy link
Member

levitte commented Oct 30, 2017

Another thing... if you think this PR is legally trivial, please edit your commit messages by adding this on a separate line:

CLA: trivial

If not, we need you to sign a CLA and send it to us, see https://www.openssl.org/policies/cla.html

Baptiste Jonglez added 2 commits October 30, 2017 14:12
The eventfd syscall is deprecated and is not available on aarch64, causing
build to fail:

    engines/e_afalg.c: In function 'eventfd':
    engines/e_afalg.c:108:20: error: '__NR_eventfd' undeclared (first use in this function)
         return syscall(__NR_eventfd, n);
                        ^

Instead, switch to the newer eventfd2 syscall, which is supposed to be
supported by all architectures.

This kind of issues would be avoided by simply using the eventfd(2)
wrapper from the libc, but there must be subtle reasons not to...

Tested on a aarch64 system running OpenSUSE Leap 42.1 (gcc118 from
https://cfarm.tetaneutral.net/machines/list/ ) and also cross-compiling
for aarch64 with LEDE (kernel 4.9).

This properly fixes openssl#1685.

CLA: trivial
Fixes: 7f458a4 ("ALG: Add AFALG engine")
Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>
The check should reject kernel versions < 4.1.0, not <= 4.1.0.

The issue was spotted on OpenSUSE 42.1 Leap, since its linux/version.h
header advertises 4.1.0.

CLA: trivial
Fixes: 7f458a4 ("ALG: Add AFALG engine")
Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>
@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Oct 30, 2017
@zorun
Copy link
Author

zorun commented Oct 30, 2017

@levitte thanks for the feedback, I have added the CLA tag, and opened #4618 for the 1.1 stable branch.

Copy link
Member

@levitte levitte left a comment

Choose a reason for hiding this comment

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

Looks good to me. @mattcaswell, you've battled with this in the past, care to give it a view?

@levitte
Copy link
Member

levitte commented Oct 31, 2017

For formality: I agree this seems legally trivial

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

I agree this is trivial

@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label Oct 31, 2017
@levitte
Copy link
Member

levitte commented Oct 31, 2017

Merged. bee9c8a and 3ba7023

@levitte levitte closed this Oct 31, 2017
levitte pushed a commit that referenced this pull request Oct 31, 2017
The eventfd syscall is deprecated and is not available on aarch64, causing
build to fail:

    engines/e_afalg.c: In function 'eventfd':
    engines/e_afalg.c:108:20: error: '__NR_eventfd' undeclared (first use in this function)
         return syscall(__NR_eventfd, n);
                        ^

Instead, switch to the newer eventfd2 syscall, which is supposed to be
supported by all architectures.

This kind of issues would be avoided by simply using the eventfd(2)
wrapper from the libc, but there must be subtle reasons not to...

Tested on a aarch64 system running OpenSUSE Leap 42.1 (gcc118 from
https://cfarm.tetaneutral.net/machines/list/ ) and also cross-compiling
for aarch64 with LEDE (kernel 4.9).

This properly fixes #1685.

CLA: trivial
Fixes: 7f458a4 ("ALG: Add AFALG engine")
Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #4617)
levitte pushed a commit that referenced this pull request Oct 31, 2017
The check should reject kernel versions < 4.1.0, not <= 4.1.0.

The issue was spotted on OpenSUSE 42.1 Leap, since its linux/version.h
header advertises 4.1.0.

CLA: trivial
Fixes: 7f458a4 ("ALG: Add AFALG engine")
Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #4617)
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants