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

client: get caller's uid/gid on every libcephfs operation #11526

Merged
merged 1 commit into from Oct 20, 2016

Conversation

ukernel
Copy link
Contributor

@ukernel ukernel commented Oct 18, 2016

Fixes: http://tracker.ceph.com/issues/17591
Signed-off-by: Yan, Zheng zyan@redhat.com

@ukernel ukernel added bug-fix cephfs Ceph File System labels Oct 18, 2016
@jcsp
Copy link
Contributor

jcsp commented Oct 18, 2016

Looks sane to me, @gregsfortytwo please could you review

@gregsfortytwo gregsfortytwo removed their assignment Oct 19, 2016
@gregsfortytwo
Copy link
Member

I was really trying to move away from this model, but I guess until we have @jtlayton's new libcephfs and bindings done we need our tests to pass. :/ Did we run this through the samba test suite?

Copy link
Contributor

@jtlayton jtlayton left a comment

Choose a reason for hiding this comment

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

Looks reasonable for now. We should probably eventually move the samba VFS to use the ceph_ll_* calls which (with the API rework) will take a userperm arg. I'll plan to do that once I'm back from the BaT.

uid_t uid() const { return m_uid; }
gid_t gid() const { return m_gid; }
uid_t uid() const { return m_uid != (uid_t)-1 ? m_uid : ::geteuid(); }
gid_t gid() const { return m_gid != (gid_t)-1 ? m_gid : ::getegid(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

For the record, we should really be using the fsuid and fsgid here (at least on Linux) and not the euid/egid. In most cases, they are identical but there are cases where they can be different. Unfortunately, Linux has no getfsuid() syscall -- you have to issue setfsuid(-1) which will always fail, but will return the existing fsuid.

What we should probably do is roll our own getfsuid() routine that does that on Linux and just calls out to geteuid() on other platforms. That said, we should probably do that in the context of a different PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm...maybe I'm wrong there actually. The setfsuid manpage says "don't use it", so maybe geteuid is fine. I think you can disregard my comment...

@ukernel
Copy link
Contributor Author

ukernel commented Oct 20, 2016

@gregsfortytwo gregsfortytwo merged commit 71d4043 into ceph:master Oct 20, 2016
@ukernel ukernel deleted the wip-17591 branch January 12, 2017 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix cephfs Ceph File System
Projects
None yet
4 participants