On Mon, Sep 26, 2016 at 4:49 PM, Niels de Vos <nde...@redhat.com> wrote:
> On Fri, Sep 23, 2016 at 08:44:14PM +0530, Pranith Kumar Karampuri wrote: > > On Fri, Sep 23, 2016 at 6:12 PM, Jeff Darcy <jda...@redhat.com> 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. > > But the files under .glusterfs are hardlinks. Except for creation and > removal, should the users not have access to read/write and update > attributes and xattrs? > > I would prefer to rely on the VFS permission checking on the bricks, and > not bother with the posix-acl xlator when the filesystem on the brick > supports POSIX ACLs. > Could you list down the pros/cons with each approach? > > Niels > -- Pranith
_______________________________________________ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel