<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 26, 2016 at 4:49 PM, Niels de Vos <span dir="ltr"><<a href="mailto:ndevos@redhat.com" target="_blank">ndevos@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Sep 23, 2016 at 08:44:14PM +0530, Pranith Kumar Karampuri wrote:<br>
> On Fri, Sep 23, 2016 at 6:12 PM, Jeff Darcy <<a href="mailto:jdarcy@redhat.com">jdarcy@redhat.com</a>> wrote:<br>
><br>
> > > Jiffin found an interesting problem in posix xlator where we have never<br>
> > been<br>
> > > using setfsuid/gid ( <a href="http://review.gluster.org/#/c/15545/" rel="noreferrer" target="_blank">http://review.gluster.org/#/c/<wbr>15545/</a> ), what I am<br>
> > > seeing regressions after this is, if the files are created using non-root<br>
> > > user then the file creation fails because that user doesn't have<br>
> > permissions<br>
> > > to create the gfid-link. So it seems like the correct way forward for<br>
> > this<br>
> > > patch is to write wrappers around sys_<syscall> to do setfsuid/gid do the<br>
> > > actual operation requested and then set it back to old uid/gid and then<br>
> > do<br>
> > > the internal operations. I am planning to write posix_sys_<syscall>() to<br>
> > do<br>
> > > the same, may be a macro?<br>
> ><br>
> > Kind of an aside, but I'd prefer to see a lot fewer macros in our code.<br>
> > They're not type-safe, and multi-line macros often mess up line numbers for<br>
> > debugging or error messages. IMO it's better to use functions whenever<br>
> > possible, and usually to let the compiler worry about how/when to inline.<br>
> ><br>
> > > I need inputs from you guys to let me know if I am on the right path and<br>
> > if<br>
> > > you see any issues with this approach.<br>
> ><br>
> > I think there's a bit of an interface problem here. The sys_xxx wrappers<br>
> > don't have arguments that point to the current frame, so how would they get<br>
> > the correct uid/gid? We could add arguments to each function, but then<br>
> > we'd have to modify every call. This includes internal calls which don't<br>
> > have a frame to pass, so I guess they'd have to pass NULL. Alternatively,<br>
> > we could create a parallel set of functions with frame pointers. Contrary<br>
> > to what I just said above, this might be a case where macros make sense:<br>
> ><br>
> > int<br>
> > sys_writev_fp (call_frame_t *frame, int fd, void *buf, size_t len)<br>
> > {<br>
> > if (frame) { setfsuid(...) ... }<br>
> > int ret = writev (fd, buf, len);<br>
> > if (frame) { setfsuid(...) ... }<br>
> > return ret;<br>
> > }<br>
> > #define sys_writev(fd,buf,len) sys_writev_fp (NULL, fd, buf, len)<br>
> ><br>
> > That way existing callers don't have to change, but posix can use the<br>
> > extended versions to get the right setfsuid behavior.<br>
> ><br>
> ><br>
> After trying to do these modifications to test things out, I am now under<br>
> the impression to remove setfsuid/gid altogether and depend on posix-acl<br>
> for permission checks. It seems too cumbersome as the operations more often<br>
> than not happen on files inside .glusterfs and non-root users/groups don't<br>
> have permissions at all to access files in that directory.<br>
<br>
</div></div>But the files under .glusterfs are hardlinks. Except for creation and<br>
removal, should the users not have access to read/write and update<br>
attributes and xattrs?<br>
<br>
I would prefer to rely on the VFS permission checking on the bricks, and<br>
not bother with the posix-acl xlator when the filesystem on the brick<br>
supports POSIX ACLs.<br></blockquote><div><br></div><div>Could you list down the pros/cons with each approach?<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
Niels<br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">Pranith<br></div></div>
</div></div>