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

Hostname addon to fix openjdk7 buffer overflow. Issue 2651 #145

Closed
wants to merge 1 commit into from

Conversation

adamburkegh
Copy link
Contributor

@adamburkegh adamburkegh commented May 24, 2019

In both the recent
https://travis-ci.org/jythontools/jython/jobs/535576706

and original

https://travis-ci.org/jythontools/jython/jobs/313613197#L728

we see:

/usr/lib/jvm/java-7-openjdk-amd64/jre/lib/amd64/libnet.so(Java_java_net_Inet4AddressImpl_getLocalHostName+0x1a5)

which suggests
travis-ci/travis-ci#5227

... is the culprit. Also, the FTP job failure shows the following:
0.00s$ hostname
travis-job-f506d6d7-075f-46e9-a40c-7245b966dcae

... which is pretty long.

The recommended fix is to use the hosts addon
https://docs.travis-ci.com/user/hosts/

Darjus applied an early workaround to the travis config in c547fb5, but there is now a more standard one available, and maybe the old one doesn't work any more.

CI has run, with expected number of failures and no buffer overflow crash in test_builtin. Since the issue is intermittent and depends on the hostname assigned, this is partial evidence. However the job log does show the hostname change being applied.

Set hostname to jyshort
$ sudo hostname jyshort

@adamburkegh adamburkegh changed the title Hostname addon to fix openjdk7 buffer overflow. Issue 2651. (Draft) Hostname addon to fix openjdk7 buffer overflow. Issue 2651 May 24, 2019
@jeff5
Copy link
Member

jeff5 commented Jul 24, 2019

Thanks for tracking this down.

This was all greek to me, but now you identify it you can see how the implementation moves on from Java 7: http://hg.openjdk.java.net/jdk7/jdk7/jdk/file/9b8c96f96a0f/src/solaris/native/java/net/Inet4AddressImpl.c#l53 to something that looks a lot cleaner in Java 8: http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/solaris/native/java/net/Inet4AddressImpl.c#l86 .

darjus pushed a commit that referenced this pull request Jul 24, 2019
--HG--
extra : amend_source : 3a9b9f7798e5c464ebbee9e311588d37bc62fe25
@jeff5
Copy link
Member

jeff5 commented Jul 24, 2019

I was perfectly happy to try this straight away, but now the build fails because it can't find jdk_switcher.
https://travis-ci.org/jythontools/jython/jobs/563244502

It didn't start with this push, but the one before:
https://travis-ci.org/jythontools/jython/jobs/563209523

while previously it worked fine:
https://travis-ci.org/jythontools/jython/jobs/561786472

Tempted to ignore it and see what future pushes produce..

@jeff5
Copy link
Member

jeff5 commented Jul 25, 2019

A difference between failing and successful use of jdk_switcher is:

Failing:

Build dist: xenial
Build id: 563209522
Job id: 563209523
Runtime kernel version: 4.15.0-1028-gcp

Successful:

Build dist: trusty
Build id: 561786471
Job id: 561786472
Runtime kernel version: 4.4.0-101-generic

What's deciding that? What we are doing seems to be recommended, and not just for particular platforms.

@adamburkegh
Copy link
Contributor Author

This thread suggests jdks, especially older jdks, might need to be installed on xenial. We don't explicitly install the jdk, just ant, in our install step.

(I agree with the commenters that say this should be an implicit effect when using jdk_switcher, but maybe that's just how it works)

@jeff5
Copy link
Member

jeff5 commented Jul 27, 2019

I've pushed this as a patch (clearly) but we don't seem to be quite there yet. It looks like jdk_switcher is only available on trusty. But we also have #140 that shows us how to do without it by declaring the matrix properly.

I feel the need of a bit of trial-and-error testing to merge #140 and what we have, but I'll do that on my own fork. It's time learned about this stuff.

@jeff5 jeff5 closed this Jul 27, 2019
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 this pull request may close these issues.

2 participants