Ceph » CephFS

Seems reasonable to remove.

We probably want to follow the kernel. But when I read the man pages and think about security, it seems like it should check: lseek does expose information about the file state! So, I'm confused.

So does fstat, and we don't do any special permission checking there either.

I guess my view is that if you're issuing a llseek then you have a valid file descriptor and all the rights that that conveys. One of those is being allowed to reposition your own fd offset. Being able to retrieve attributes on the inode that the fd points to is another.

But...isn't that insecure by design? Why should we assume an open FD is still valid on a setuid'ed fork(), for instance?

That's just the way UNIX (and hence POSIX) works. UNIX pipelines require file descriptor inheritance.

I'm not sure what you mean by setuid'ed fork(). fork() just spawns a new process with the same creds as the old. Did you mean calling exec() on a setuid'ed binary?

Yeah, sorry. I know it's possible for a process to drop permissions somehow or other.

There are several ways, but yeah...it comes down to being careful to close out old file descriptors (or use O_CLOEXEC when doing opens).

Actually, now I'm confused. What's the failing test, if this isn't a case users should run into anyway?

  public void test_lseek() throws Exception {
    /* create a new file */
    String path = makePath();
    int size = 12345;
    int fd = createFile(path, size);
    mount.close(fd);

    /* open and check size */
    fd = mount.open(path, CephMount.O_RDWR, 0);
    long end = mount.lseek(fd, 0, CephMount.SEEK_END);
    mount.close(fd);

    mount.unlink(path);

    assertTrue(size == (int)end);
  }

makePath just generates a random uuid filename. So it looks like the create and open succeeded but then the lseek failed.

Beyond that I'm not sure. Maybe another thread changed permissions on that file?

In any case, this behavior just recently changed in commit be9e43e2da41, which added in that permission check. I'm testing a patch now that removes that check and has the _getattr call in that use f->actor_perms instead.