On Fri, Sep 23, 2016 at 6:12 PM, Jeff Darcy <jdarcy@xxxxxxxxxx> wrote:
> Jiffin found an interesting problem in posix xlator where we have never been
> using setfsuid/gid ( http://review.gluster.org/#/c/15545/ ), what I am
> seeing regressions after this is, if the files are created using non-root
> user then the file creation fails because that user doesn't have permissions
> to create the gfid-link. So it seems like the correct way forward for this
> patch is to write wrappers around sys_<syscall> to do setfsuid/gid do the
> actual operation requested and then set it back to old uid/gid and then do
> the internal operations. I am planning to write posix_sys_<syscall>() to do
> the same, may be a macro?
Kind of an aside, but I'd prefer to see a lot fewer macros in our code.
They're not type-safe, and multi-line macros often mess up line numbers for
debugging or error messages. IMO it's better to use functions whenever
possible, and usually to let the compiler worry about how/when to inline.
> I need inputs from you guys to let me know if I am on the right path and if
> you see any issues with this approach.
I think there's a bit of an interface problem here. The sys_xxx wrappers
don't have arguments that point to the current frame, so how would they get
the correct uid/gid? We could add arguments to each function, but then
we'd have to modify every call. This includes internal calls which don't
have a frame to pass, so I guess they'd have to pass NULL. Alternatively,
we could create a parallel set of functions with frame pointers. Contrary
to what I just said above, this might be a case where macros make sense:
int
sys_writev_fp (call_frame_t *frame, int fd, void *buf, size_t len)
{
if (frame) { setfsuid(...) ... }
int ret = writev (fd, buf, len);
if (frame) { setfsuid(...) ... }
return ret;
}
#define sys_writev(fd,buf,len) sys_writev_fp (NULL, fd, buf, len)
That way existing callers don't have to change, but posix can use the
extended versions to get the right setfsuid behavior.
After trying to do these modifications to test things out, I am now under the impression to remove setfsuid/gid altogether and depend on posix-acl for permission checks. It seems too cumbersome as the operations more often than not happen on files inside .glusterfs and non-root users/groups don't have permissions at all to access files in that directory.
--
Pranith
_______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://www.gluster.org/mailman/listinfo/gluster-devel
