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 server startup on NetBSD (and probably other systems) #838

Closed
wants to merge 1 commit into from

Conversation

manu0401
Copy link

@manu0401 manu0401 commented Aug 5, 2018

my_open_parent_dir_nosymlinks() calls openat() with dfd = -1, which
Linux takes for root of filesystem, but this behavior is not standard
as per OpenGroup XSH:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html

On NetBSD, dfd = -1 causes openat() to fail with EBADF, which breaks
server startup. The same result probaly happens on any non Linux
system implementing openat().

The proposed fix is to call plain open() when dfd = -1

my_open_parent_dir_nosymlinks() calls openat() with dfd = -1, which
Linux takes for root of filesystem, but this behavior is not standard
as per OpenGroup XSH:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html

On NetBSD, dfd = -1 causes openat() to fail with EBADF, which breaks
server startup. The same result probaly happens on any non Linux
system implementing openat().

The proposed fix is to call plain open() when dfd = -1
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 5, 2018
@cvicentiu cvicentiu self-assigned this Aug 14, 2018
@cvicentiu cvicentiu added this to the 10.3 milestone Aug 14, 2018
if (dfd == -1)
fd = open(s, O_NOFOLLOW | O_PATH | O_CLOEXEC);
else
#endif
Copy link

@svoj svoj Jan 17, 2019

Choose a reason for hiding this comment

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

I'd suggest to move it before the loop and instead do something like:

#ifndef linux
dfd= open("/", O_NOFOLLOW | O_PATH | O_CLOEXEC);
#endif

@vuvova
Copy link
Member

vuvova commented Jan 29, 2019

Strictly speaking, Linux does not take dfd==-1 as root. The manual page open(2) says:

If pathname is absolute, then dirfd is ignored.

The OpenGroup page you mentioned says

The openat() function shall be equivalent to the open() function except in the case where path specifies a relative path.

As far as I understand, it means that if the path is "/", dfd does not matter and could as well be -1.
If NetBSD behaves differently, it must be a NetBSD bug.

@manu0401
Copy link
Author

manu0401 commented Jan 29, 2019 via email

@vuvova
Copy link
Member

vuvova commented Jan 29, 2019

Yes, I tend to agree with that

netbsd-srcmastr pushed a commit to NetBSD/src that referenced this pull request Jan 31, 2019
Opengroup says "The openat() function shall be equivalent to the open() function except in the case where path specifies a relative path", but
says nothing about fdat usage when path is absolute;
https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html

 We used to always reslove fdat, leading to error if it was invalid (e.g.: -1). That caused portability problem with other systems that
 just ignore it. See discussion in a pull request to work around that
 problem with MariaDB: MariaDB/server#838

 We fix the problem by ignoring fdat when path is absolute.
ryo pushed a commit to IIJ-NetBSD/netbsd-src that referenced this pull request Jan 31, 2019
Opengroup says "The openat() function shall be equivalent to the open() function except in the case where path specifies a relative path", but
says nothing about fdat usage when path is absolute;
https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html

 We used to always reslove fdat, leading to error if it was invalid (e.g.: -1). That caused portability problem with other systems that
 just ignore it. See discussion in a pull request to work around that
 problem with MariaDB: MariaDB/server#838

 We fix the problem by ignoring fdat when path is absolute.
netbsd-srcmastr pushed a commit to NetBSD/src that referenced this pull request Jan 31, 2019
Opengroup says "The openat() function shall be equivalent to the open() function except in the case where path specifies a relative path", but
says nothing about fdat usage when path is absolute;
https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html

 We used to always reslove fdat, leading to error if it was invalid (e.g.: -1). That caused portability problem with other systems that
 just ignore it. See discussion in a pull request to work around that
 problem with MariaDB: MariaDB/server#838

 We fix the problem by ignoring fdat when path is absolute.
@an3l an3l self-assigned this Feb 25, 2020
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Emmanuel Dreyfus seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@grooverdan grooverdan added the need feedback Can the contributor please address the questions asked. label Mar 26, 2021
@an3l an3l assigned grooverdan and unassigned an3l Apr 7, 2021
@robertbindar robertbindar self-assigned this Nov 30, 2021
@grooverdan grooverdan removed their assignment Dec 8, 2021
@LinuxJedi LinuxJedi self-assigned this Sep 7, 2022
@LinuxJedi
Copy link
Contributor

Hello @manu0401,

I'm trying to go through older pull requests so that we can work towards getting them moving again. Is this patch still something you would like to see in MariaDB? If so I can try to assist you in getting it to a place where it can be merged.

Please let me know how you would like to proceed.

@LinuxJedi
Copy link
Contributor

Hello @manu0401,

I'm going to close this one for now as we have not heard from you. But please feel free to reopen or contact me if you wish to continue this one or discuss it further.

@LinuxJedi LinuxJedi closed this Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need feedback Can the contributor please address the questions asked.
9 participants