Project

General

Profile

Bug #4920

client: does not respect O_NOFOLLOW

Added by Greg Farnum over 6 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
Low
Assignee:
-
Category:
-
Target version:
-
Start date:
05/06/2013
Due date:
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(FS):
Client
Labels (FS):
Pull request ID:

Description

It looks like doing an open() always implicitly follows symlinks, because we call path_walk() with followsym set to the default "true". There's an O_SYMLINK flag that lets you open symlinks themselves, rather than the file it points to.

20150127_4920_fixv2_and_unittest.patch View (1.29 KB) Radoslaw Zarzynski, 01/27/2015 01:56 PM

Associated revisions

Revision 911e4c05 (diff)
Added by Radoslaw Zarzynski over 4 years ago

client: add support for O_NOFOLLOW in Client::open().

Fixes: #4920
Signed-off-by: Radoslaw Zarzynski <>

History

#1 Updated by Greg Farnum over 5 years ago

  • Priority changed from Normal to Low

uclient = low priority, for now.

#2 Updated by Radoslaw Zarzynski over 4 years ago

O_SYMLINK sounds like BSD extension to me. We really want it?
Anyway, the Client::open() lacks support for O_NOFOLLOW. I am currently working on it.

#3 Updated by Greg Farnum over 4 years ago

Hmm, I don't remember where this came from but it might have been somebody cross-compiling for OS X. As marked, it's not terribly important but I'll merge in patches which fix it! :)

(I personally have a lot of interest in Ceph on OS X, and making the client usable on that platform is likely to extend CephFS' footprint quite a lot in the future.)

#4 Updated by Radoslaw Zarzynski over 4 years ago

I have added support for O_NOFOLLOW in Client::open and extended unittest as well. It works for me. Patch in attachment.
Unfortunately, I have no access to any system implementing the O_SYMLINK. :-( Anyway, the O_NOFOLLOW should be also available on Mac OS X.

#5 Updated by Greg Farnum over 4 years ago

Hmm, just looking at the man page for open() aren't you supposed to perform an open on a symlink with O_NOFOLLOW if you also specify O_PATH?

#6 Updated by Radoslaw Zarzynski over 4 years ago

Thank you for your review. You are right, on Linux the call should not fail with ELOOP if there was O_PATH present in the flags too. I am attaching the link to branch with support for O_PATH below, but currently only in Client::open. The full implementation should add checks based on Fh::mode field in many descriptor-accepting methods (like Client::read, Client::write, ...) to just return EBADF, I think. May we split the work into two tasks?

Link: https://github.com/rzarzynski/ceph/compare/wip-4920

#7 Updated by Greg Farnum over 4 years ago

I think it's probably best to merge it all as one pull request; there's nothing urgent here and that way we'll know we don't need to change any of the open logic again to make the other stuff return EBADF appropriately. :)

#8 Updated by Radoslaw Zarzynski over 4 years ago

I added the support for O_PATH in several places as well as unit test. The commit is on my github. Feel free to comment.
Link: https://github.com/rzarzynski/ceph/compare/wip-4920

#9 Updated by Greg Farnum over 4 years ago

Looks good! Can you submit it as a PR against the main Ceph repo? :)

#10 Updated by Radoslaw Zarzynski over 4 years ago

With pleasure. :-)
I corrected the typo in comment and squashed wip-4920 branch into two commits.
PR is available here: https://github.com/ceph/ceph/pull/3613

#11 Updated by Greg Farnum over 4 years ago

  • Subject changed from client: does not respect O_SYMLINK to client: does not respect O_NOFOLLOW
  • Status changed from New to Resolved

Thanks!

#12 Updated by Greg Farnum about 3 years ago

  • Component(FS) Client added

Also available in: Atom PDF