linux-kernel - Re: [PATCH v2 0/3] fuse: Add support for mounts from pid/user namespaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 26 Sep 2014 18:41:33 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Seth Forshee <seth.forshee@...onical.com>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	Serge Hallyn <serge.hallyn@...ntu.com>,
	fuse-devel <fuse-devel@...ts.sourceforge.net>,
	Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
	Miklos Szeredi <miklos@...redi.hu>,
	"Serge E. Hallyn" <serge@...lyn.com>
Subject: Re: [PATCH v2 0/3] fuse: Add support for mounts from pid/user namespaces

Seth Forshee <seth.forshee@...onical.com> writes:

> On Thu, Sep 25, 2014 at 12:14:01PM -0700, Eric W. Biederman wrote:

>> Sorry iattr_to_setattr look for from_kuid and from_kgid.
>> 
>> The call path is
>> fuse_setattr
>>    fuse_do_setattr
>>       iattr_to_fattr.
>
> Bah. Sorry, I misread that originally and thought you were talking about
> something outside of fuse. And I was looking at a tree with my fuse
> changes, so of course I wouldn't have found it.
>
> Actually in 3.17-rc6 I still don't see that iattr_to_fattr (I assume
> this is what you meant) checks the results of the conversion (not that
> it really needs to since it uses init_user_ns), nor any use of EOVERFLOW
> in fuse. Anyway, it's not really important.

True.  I also goofed up in that I was looking at the wrong tree.  My
tree had all of my preliminary fuse patches I worked up a while ago
applied so I did have the error handling in there.

> Well, unless you say otherwise I guess I'll leave it -EINVAL to be
> consistent with chown_common().

That sounds like a good plan.

>> I am on the fence about what to do when a uid from the filesystem server
>> or for other filesystems the on-disk data structures does not map, but
>> make_bad_inode is simpler in conception.  So make_bad_inode seems like
>> a good place to start.   For fuse especially this isn't hard because
>> the make_bad_inode calls are already there to handle a change in i_mode.
>
> I agree that if we're unsure then make_bad_inode is a more logical place
> to start, since it's easier to loosen the restrictions later than to
> tighten them. I've got an initiail implementation that I'm in the
> process of testing. If you want to take a look I've pushed it to:
>
>   git://kernel.ubuntu.com/sforshee/linux.git fuse-userns 

Thanks.  If I can scrape together enough focus I will look at it.

As a second best thing here are my prototype from when I was looking at
performing this fuse conversion last.  Given that you had missed
checking the from_kuid permission checks, it might be worth comparing
and seeing if there is something else in these patches that would be
worth including.

Eric


View attachment "0001-userns-Allow-for-fuse-filesystems-outside-the-initia.patch" of type "text/x-diff" (7489 bytes)

View attachment "0002-fuse-Teach-fuse-how-to-handle-the-pid-namespace.patch" of type "text/x-diff" (4851 bytes)

View attachment "0003-userns-fuse-unprivileged-mount-suport.patch" of type "text/x-diff" (804 bytes)

View attachment "0004-fuse-Only-allow-read-writing-user-xattrs.patch" of type "text/x-diff" (1501 bytes)