Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 512712 - Include aufs support in genpatches
Summary: Include aufs support in genpatches
Status: RESOLVED WONTFIX
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Gentoo Kernel Bug Wranglers and Kernel Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-08 07:39 UTC by Justin Lecher (RETIRED)
Modified: 2014-12-22 20:21 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Lecher (RETIRED) gentoo-dev 2014-06-08 07:39:01 UTC
Security wise it is a nightmare packing the aufs-sources seperated from gentoo-sources. I am always lacking behind the security steps of gentoo-sources although aufs-sources suffer from the same. The beast solution would be including aufs into genpatches. There are quite a number of users who are using aufs so it would be nice to have.

My idea would be to cover it under either USE=aufs or experimental if we do not want additional USE.

I would take care of preparing the patches if you like.

There are 3 patches which need always to be applied. And a couple of other patches providing special funtionality. I would install them as docs and norify the user that they are there.


What do you think?
Comment 1 Mike Pagano gentoo-dev 2014-07-02 15:40:07 UTC
Justin,

If you want to include these patches in gentoo-sources with the corresponding Kconfig modifications and have them default to not being enabled, that would be fine with me.
Comment 2 Justin Lecher (RETIRED) gentoo-dev 2014-07-06 09:08:02 UTC
(In reply to Mike Pagano from comment #1)
> Justin,
> 
> If you want to include these patches in gentoo-sources with the
> corresponding Kconfig modifications and have them default to not being
> enabled, that would be fine with me.

Of course I would leave them optional. The only thing we need to decide is whether we allow to build aufs as a module. There is a patch needed which exposes a number of symbols. If I remember correctly this was one of the main critics Greg has about aufs.
Currently I apply the patch USE depend, but probably we should simply only allow build in support.
Comment 3 Mike Pagano gentoo-dev 2014-10-09 23:07:50 UTC
Justin,

If you want to do something like mptcp does (http://multipath-tcp.org/patches/) that would work for me. One patch, Kconfig changes and all. 

We can include it into experimental. 

Mike
Comment 4 Richard Yao (RETIRED) gentoo-dev 2014-10-10 01:22:15 UTC
(In reply to Justin Lecher from comment #2)
> (In reply to Mike Pagano from comment #1)
> > Justin,
> > 
> > If you want to include these patches in gentoo-sources with the
> > corresponding Kconfig modifications and have them default to not being
> > enabled, that would be fine with me.
> 
> Of course I would leave them optional. The only thing we need to decide is
> whether we allow to build aufs as a module. There is a patch needed which
> exposes a number of symbols. If I remember correctly this was one of the
> main critics Greg has about aufs.
> Currently I apply the patch USE depend, but probably we should simply only
> allow build in support.

It would have been helpful to reference the patch, which is called aufs3-standalone.patch and is in the aufs3 standalone repository:

http://sourceforge.net/p/aufs/aufs3-standalone/ci/aufs3.16/tree/aufs3-standalone.patch

There are not *that* many new exports, although new exports should always be done with the signed-off of those that hold copyright on the symbol. If we want to start exporting things without author's signed-off, they should be exported using SYMBOL_EXPORT_GPL(), not SYMBOL_EXPORT().

I do not have time to do a full review of the requested symbol exports, but here are my thoughts on the first four:

update_time() - this function probably should not be exported as is. Someone might be tempted to set i_ops->update_time = update_time, which would cause an infinite loop. It would be better to factor out the logic after the check for i_ops->update_time() into a generic_update_time() that is exported instead.

inode_sb_list_lock - this is a lock used by code in aufs to hook into magic system request and to handle remounting in a special way. It looks reminiscent of the big kernel lock in how it is taken whenever one wants to iterate the list of inodes of any superblock. We should refactor the kernel's VFS to give each superblock its own lock to protect that list rather than having a global lock dedicated to this. Having such a lock is a bad idea and letting filesystems touch it is worse. If it turns out that we must do this, we should make a helper function that wraps a call to a function pointer with locking/unlocking this lock and export that.

__mnt_drop_write() - I do not see why aufs is prefers this over the already exported mnt_drop_write(). __mnt_want_write_file() must be matched with __mnt_drop_write() while mnt_want_write_file() must be matched with mnt_drop_write(). I see nothing in the aufs code calling __mnt_want_write_file() and the mainline kernel appears to have all instances of __mnt_want_write_file() matched with __mnt_drop_write(). Consequently, I strongly suspect that using __mnt_drop_write() instead of mnt_drop_write() here is wrong.

iterate_mounts() - this function is trivial and there is nothing really stopping us from iterating the list ourselves. Interestingly, the aufs code correctly takes the rcu read lock while the audit code permitted to use it does not. It should be noted that this is only needed for NFS export support because the fh to dentry code is translated into a dentry_open() on the underlying filesystem. I think exporting this is a sane thing to do.
Comment 5 Rick Farina (Zero_Chaos) gentoo-dev 2014-10-10 01:27:05 UTC
I have to NACK this.  Other kernel source packages (such as hardened) use gentoo-sources as a base, and the patches conflict with each other.  AUFS isn't in the upstream kernel, it's never going to be in the upstream kernel.  I want to support this better, but unless this patch is only added optionally with a use flag I am going to politely scream "Oh god, please, please no, ahhhhhh!"
Comment 6 Rick Farina (Zero_Chaos) gentoo-dev 2014-10-10 01:53:59 UTC
Complaint officially retracted.

If other *-sources want to use gentoo-patches and don't want aufs they can always unipatch_exclude it entirely.

Add aufs, add it unconditionally, disabled by default. +1
Comment 7 Justin Lecher (RETIRED) gentoo-dev 2014-10-10 07:10:00 UTC
(In reply to Mike Pagano from comment #1)
> Justin,
> 
> If you want to include these patches in gentoo-sources with the
> corresponding Kconfig modifications and have them default to not being
> enabled, that would be fine with me.

Perfect, what is the best way for me to provide the patches?
Comment 8 Justin Lecher (RETIRED) gentoo-dev 2014-10-10 07:19:10 UTC
(In reply to Richard Yao from comment #4)
> There are not *that* many new exports, although new exports should always be
> done with the signed-off of those that hold copyright on the symbol. If we
> want to start exporting things without author's signed-off, they should be
> exported using SYMBOL_EXPORT_GPL(), not SYMBOL_EXPORT().
> 


As you pointed out and I said before there are problems with the export of some of the symbols. So I would vote for no modules at this point.
Comment 9 Justin Lecher (RETIRED) gentoo-dev 2014-10-10 07:23:58 UTC
(In reply to Rick Farina (Zero_Chaos) from comment #5)
> isn't in the upstream kernel, it's never going to be in the upstream kernel.

Wrong, Aufs upstream is again working with the linux upstream to get it included. I cannot remember if it was on a public ml or a private mail where the author told me that.
Comment 10 Mike Pagano gentoo-dev 2014-10-13 17:23:23 UTC
(In reply to Justin Lecher from comment #7)
> (In reply to Mike Pagano from comment #1)
> > Justin,
> > 
> > If you want to include these patches in gentoo-sources with the
> > corresponding Kconfig modifications and have them default to not being
> > enabled, that would be fine with me.
> 
> Perfect, what is the best way for me to provide the patches?

If you want to do something like mptcp does (http://multipath-tcp.org/patches/) that would work for me. One patch, Kconfig changes and all. 

We can include it into experimental. 

Even hosted on your devspace would work, I just need somewhere to look.
Comment 11 Justin Lecher (RETIRED) gentoo-dev 2014-10-17 11:58:21 UTC
(In reply to Mike Pagano from comment #10)
> (In reply to Justin Lecher from comment #7)
> > (In reply to Mike Pagano from comment #1)
> > > Justin,
> > > 
> > > If you want to include these patches in gentoo-sources with the
> > > corresponding Kconfig modifications and have them default to not being
> > > enabled, that would be fine with me.
> > 
> > Perfect, what is the best way for me to provide the patches?
> 
> If you want to do something like mptcp does
> (http://multipath-tcp.org/patches/) that would work for me. One patch,
> Kconfig changes and all. 
> 
> We can include it into experimental. 
> 
> Even hosted on your devspace would work, I just need somewhere to look.

Is it possible to provide documentation in addition to the patch?
Comment 12 Mike Pagano gentoo-dev 2014-10-17 13:21:01 UTC
(In reply to Justin Lecher from comment #11)
> (In reply to Mike Pagano from comment #10)
> > (In reply to Justin Lecher from comment #7)
> > > (In reply to Mike Pagano from comment #1)
> > > > Justin,
> > > > 
> > > > If you want to include these patches in gentoo-sources with the
> > > > corresponding Kconfig modifications and have them default to not being
> > > > enabled, that would be fine with me.
> > > 
> > > Perfect, what is the best way for me to provide the patches?
> > 
> > If you want to do something like mptcp does
> > (http://multipath-tcp.org/patches/) that would work for me. One patch,
> > Kconfig changes and all. 
> > 
> > We can include it into experimental. 
> > 
> > Even hosted on your devspace would work, I just need somewhere to look.
> 
> Is it possible to provide documentation in addition to the patch?

Justin, if you include Documentation as part of the patch and have it install into the Documentation directory of the kernel that could work.  Is that what you're asking?
Comment 13 Justin Lecher (RETIRED) gentoo-dev 2014-10-17 13:25:41 UTC
(In reply to Mike Pagano from comment #12)
> Justin, if you include Documentation as part of the patch and have it
> install into the Documentation directory of the kernel that could work.  Is
> that what you're asking?

My question was basically two fold, is it possible to install additional documentation and if, how.
So there is no way to install into /usr/share/doc? I would like to add additional patches (like the modul patch) as documentation so that the user can apply it later.
Comment 14 Mike Pagano gentoo-dev 2014-10-17 13:27:25 UTC
I'm starting to understand why this is a separate ebuild.
Comment 15 Mike Pagano gentoo-dev 2014-12-22 20:21:58 UTC
With all respect, this patch is a bit large for our goals for gentoo-sources.