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

overlay2 pageSize: Fix an off-by-one bug #41830

Merged
merged 1 commit into from Dec 22, 2020
Merged

overlay2 pageSize: Fix an off-by-one bug #41830

merged 1 commit into from Dec 22, 2020

Conversation

ob
Copy link
Contributor

@ob ob commented Dec 21, 2020

fixes docker/for-linux#1012 Overlay2 mount fails for larger Dockerfiles when length of data-root is 24

The code was not considering that C strings are NULL-terminated so we need to leave one extra byte.

Without this fix, the testcase in docker/for-linux#1012 fails with

Step 61/1001 : RUN echo 60 > 60
 ---> Running in dde85ac3b1e3
Removing intermediate container dde85ac3b1e3
 ---> 80a12a18a241
Step 62/1001 : RUN echo 61 > 61
error creating overlay mount to /23456789112345678921234/overlay2/d368abcc97d6c6ebcf23fa71225e2011d095295d5d8c9b31d6810bea748bdf07-init/merged: no such file or directory

with the output of dmesg -T as:

[Sat Dec 19 02:35:40 2020] overlayfs: failed to resolve '/23456789112345678921234/overlay2/89e435a1b24583c463abb73e8abfad8bf8a88312ef8253455390c5fa0a765517-init/wor': -2

with this fix, you get the expected:

Step 126/1001 : RUN echo 125 > 125
 ---> Running in 2f2e56da89e0
max depth exceeded

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@ob ob changed the title Fix for https://github.com/docker/for-linux/issues/1012 Fix an off-by-one bug Dec 21, 2020
This is a fix for docker/for-linux#1012.

The code was not considering that C strings are NULL-terminated so
we need to leave one extra byte.

Without this fix, the testcase in docker/for-linux#1012
fails with

```
Step 61/1001 : RUN echo 60 > 60
 ---> Running in dde85ac3b1e3
Removing intermediate container dde85ac3b1e3
 ---> 80a12a18a241
Step 62/1001 : RUN echo 61 > 61
error creating overlay mount to /23456789112345678921234/overlay2/d368abcc97d6c6ebcf23fa71225e2011d095295d5d8c9b31d6810bea748bdf07-init/merged: no such file or directory
```

with the output of `dmesg -T` as:

```
[Sat Dec 19 02:35:40 2020] overlayfs: failed to resolve '/23456789112345678921234/overlay2/89e435a1b24583c463abb73e8abfad8bf8a88312ef8253455390c5fa0a765517-init/wor': -2
```

with this fix, you get the expected:

```
Step 126/1001 : RUN echo 125 > 125
 ---> Running in 2f2e56da89e0
max depth exceeded
```

Signed-off-by: Oscar Bonilla <6f6231@gmail.com>
Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

Nice catch!

Do you think it's reasonable/possible to add an integration test, or are there "platform" fiddly bits interacting to make that difficult?

@ob
Copy link
Contributor Author

ob commented Dec 21, 2020

I'm not sure how that would work. In theory it should be possible to construct a dockerfile that exceeds the maximum number of layers (like the bug report does) but in order to hit exactly the size of a page you'd have to tweak the docker-root path carefully. For most platforms the size of a page is 4k but not all... I'm not familiar with how docker is tested to be able to foresee if this would be a flaky test or fail on some platforms.

@tianon
Copy link
Member

tianon commented Dec 21, 2020

Right, that's what I was afraid of (but wasn't 100% understanding all the variables in the issue this fixes).

Thanks for explaining! 👍

@AkihiroSuda AkihiroSuda changed the title Fix an off-by-one bug overlay2 pageSize: Fix an off-by-one bug Dec 22, 2020
@AkihiroSuda AkihiroSuda merged commit aa1ada6 into moby:master Dec 22, 2020
@thaJeztah thaJeztah added this to the 20.10.2 milestone Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overlay2 mount fails for larger Dockerfiles when length of data-root is 24
4 participants