2003-02-11 19:09:38

by Andreas Gruenbacher

[permalink] [raw]
Subject: [PATCH] Extended attribute fixes, etc.

Hi Andrew,

here are five patches against 2.5.60. Each file contains a brief
description of what it does.

The first two fix an [un]lock_kernel() bug, and an iops->listxattr() bug
that for some reason hasn't found its way into 2.5 yet. (Both bugs do
not exist in the current 2.4.19/2.4.20 patches at acl.bestbits.at).

The third to fifth are all steps towards trusted extended attributes,
which are useful for privileged processes (mostly daemons). One use for
this is Hierarchical Storage Management, in which a user space daemon
stores online/offline information for files in trusted EA's, and the
kernel communicates requests to bring files online to that daemon. This
class of EA's will also find its way into XFS and ReiserFS, and
expectedly also into JFS in this form. (Trusted EAs are included in the
2.4.19/2.4.20 patches as well.)

The intended patch order is:

kernel_lock_bug.diff
ext2_ext3_listxattr-bug.diff
xattr-flags.diff
xattr-flags-policy.diff
xattr-trusted.diff

Could you please feed the patches to Linus?

Thank you very much,
Andreas Gruenbacher.


Attachments:
ext2_ext3_listxattr-bug.diff (4.70 kB)
kernel_lock_bug.diff (5.56 kB)
xattr-trusted.diff (9.18 kB)
xattr-flags-policy.diff (4.94 kB)
xattr-flags.diff (15.83 kB)
Download all attachments

2003-02-11 20:23:45

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Extended attribute fixes, etc.

Andreas Gruenbacher <[email protected]> wrote:
>
> Hi Andrew,
>
> here are five patches against 2.5.60. Each file contains a brief
> description of what it does.
>

Minor point:

> int
> ext3_xattr_set(struct inode *inode, int name_index, const char *name,
> const void *value, size_t value_len, int flags)
> {
> handle_t *handle;
> int error;
>
> lock_kernel();
> handle = ext3_journal_start(inode, EXT3_XATTR_TRANS_BLOCKS);
> if (IS_ERR(handle))
> error = PTR_ERR(handle);
> else
> error = ext3_xattr_set_handle(handle, inode, name_index, name,
> value, value_len, flags);
> ext3_journal_stop(handle, inode);

ext3_journal_stop() can return an error code - most notable -EIO if it was a
synchronous transaction, or the filesystem has detected corruption.

> The third to fifth are all steps towards trusted extended attributes,
> which are useful for privileged processes (mostly daemons). One use for
> this is Hierarchical Storage Management, in which a user space daemon
> stores online/offline information for files in trusted EA's, and the
> kernel communicates requests to bring files online to that daemon. This
> class of EA's will also find its way into XFS and ReiserFS, and
> expectedly also into JFS in this form. (Trusted EAs are included in the
> 2.4.19/2.4.20 patches as well.)

So is this new code actually functional yet? As in: something in-kernel
using it?

If not, what is involved in completing the kernel side of trusted EA's?


2003-02-12 11:08:38

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [PATCH] Extended attribute fixes, etc.

On Tuesday 11 February 2003 21:32, Andrew Morton wrote:
> Andreas Gruenbacher <[email protected]> wrote:
> > Hi Andrew,
> >
> > here are five patches against 2.5.60. Each file contains a brief
> > description of what it does.
>
> Minor point:
> ext3_journal_stop() can return an error code - most notable -EIO if
> it was a synchronous transaction, or the filesystem has detected
> corruption.

Thanks, I have overlooked this third bug. An incremental patch on top of
my previous kernel_lock_bug.diff is attached. (I have also uploaded the
patches to <http://acl.bestbits.at/pre/v2.5/> in the meantime).

> > The third to fifth are all steps towards trusted extended
> > attributes, which are useful for privileged processes (mostly
> > daemons). One use for this is Hierarchical Storage Management, in
> > which a user space daemon stores online/offline information for
> > files in trusted EA's, and the kernel communicates requests to
> > bring files online to that daemon. This class of EA's will also
> > find its way into XFS and ReiserFS, and expectedly also into JFS in
> > this form. (Trusted EAs are included in the 2.4.19/2.4.20 patches
> > as well.)
>
> So is this new code actually functional yet? As in: something
> in-kernel using it?
>
> If not, what is involved in completing the kernel side of trusted
> EA's?

The important point for me now is to get the iops xattr-flags and
xattr-flags-policy patches into 2.5 so that the API won't change during
2.6. The xattr-trusted patch only affects file systems locally, so it's
far less critical.

The kernel side of trusted EAs is completely implemented with the
patches I sent. In the future there will very likely be modules
actually making use of the XATTR_KERNEL_CONTEXT flag, but Trusted EAs
are quite useful from user space alone.

Cheers,
Andreas.


Attachments:
kernel_lock_bug2.diff (713.00 B)

2003-02-15 10:57:41

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] Extended attribute fixes, etc.

On Tue, Feb 11, 2003 at 08:18:58PM +0100, Andreas Gruenbacher wrote:
> The third to fifth are all steps towards trusted extended attributes,
> which are useful for privileged processes (mostly daemons). One use for
> this is Hierarchical Storage Management, in which a user space daemon
> stores online/offline information for files in trusted EA's, and the
> kernel communicates requests to bring files online to that daemon. This
> class of EA's will also find its way into XFS and ReiserFS, and
> expectedly also into JFS in this form. (Trusted EAs are included in the
> 2.4.19/2.4.20 patches as well.)

Please don't do the ugly flags stuff. We have fsuids and fsgids for exactly
that reason (and because we're still lacking a credentials cache..).


2003-02-15 17:49:20

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [PATCH] Extended attribute fixes, etc.

On Saturday 15 February 2003 12:07, Christoph Hellwig wrote:
> On Tue, Feb 11, 2003 at 08:18:58PM +0100, Andreas Gruenbacher wrote:
> > The third to fifth are all steps towards trusted extended
> > attributes, which are useful for privileged processes (mostly
> > daemons). One use for this is Hierarchical Storage Management, in
> > which a user space daemon stores online/offline information for
> > files in trusted EA's, and the kernel communicates requests to
> > bring files online to that daemon. This class of EA's will also
> > find its way into XFS and ReiserFS, and expectedly also into JFS in
> > this form. (Trusted EAs are included in the 2.4.19/2.4.20 patches
> > as well.)
>
> Please don't do the ugly flags stuff. We have fsuids and fsgids for
> exactly that reason (and because we're still lacking a credentials
> cache..).

The XATTR_KERNEL_CONTEXT flag cannot be substituted by a uid/gid change;
it is unrelated to that; that's the whole point of it. It would be
possible to raise some other flag (such as a capability, etc.) instead
of passing an explicit flag, but that seems uglier and more
problematic/error prone to me.

-- Andreas.

2003-02-15 18:30:08

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] Extended attribute fixes, etc.

On Sat, Feb 15, 2003 at 06:59:11PM +0100, Andreas Gruenbacher wrote:
> > Please don't do the ugly flags stuff. We have fsuids and fsgids for
> > exactly that reason (and because we're still lacking a credentials
> > cache..).
>
> The XATTR_KERNEL_CONTEXT flag cannot be substituted by a uid/gid change;
> it is unrelated to that; that's the whole point of it. It would be
> possible to raise some other flag (such as a capability, etc.) instead
> of passing an explicit flag, but that seems uglier and more
> problematic/error prone to me.

Then raise CAP_DAC_OVERRIDE. The right thing would be to pass down a
struct cred, so we could pass down the magic sys_cred that allows all
access (look at the XFS ACL code for details on that..), but
unfortuantately we still don't have proper credentials although there
were numerous patches around in the last years and we really want it
for other reasons.

Magic flags that change the DAC checks work are ugly and will most
certainly lead to bugs in some implementations sooner or later.

2003-02-15 19:07:12

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [PATCH] Extended attribute fixes, etc.

On Saturday 15 February 2003 19:39, Christoph Hellwig wrote:
> On Sat, Feb 15, 2003 at 06:59:11PM +0100, Andreas Gruenbacher wrote:
> > > Please don't do the ugly flags stuff. We have fsuids and fsgids
> > > for exactly that reason (and because we're still lacking a
> > > credentials cache..).
> >
> > The XATTR_KERNEL_CONTEXT flag cannot be substituted by a uid/gid
> > change; it is unrelated to that; that's the whole point of it. It
> > would be possible to raise some other flag (such as a capability,
> > etc.) instead of passing an explicit flag, but that seems uglier
> > and more problematic/error prone to me.
>
> Then raise CAP_DAC_OVERRIDE. The right thing would be to pass down a
> struct cred, so we could pass down the magic sys_cred that allows all
> access (look at the XFS ACL code for details on that..), but
> unfortuantately we still don't have proper credentials although there
> were numerous patches around in the last years and we really want it
> for other reasons.

That sounds quite reasonable. I would have to raise CAP_SYS_ADMIN for
trusted EA's, though. Do you see any potential side effects while a
pretty powerful capability like CAP_SYS_ADMIN is temporarily raised?

> Magic flags that change the DAC checks work are ugly and will most
> certainly lead to bugs in some implementations sooner or later.

Maybe.


Thanks,
Andreas.

2003-02-15 20:59:30

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] Extended attribute fixes, etc.

On Sat, Feb 15, 2003 at 08:17:03PM +0100, Andreas Gruenbacher wrote:
> That sounds quite reasonable. I would have to raise CAP_SYS_ADMIN for
> trusted EA's, though. Do you see any potential side effects while a
> pretty powerful capability like CAP_SYS_ADMIN is temporarily raised?

Okay, something I missed when looking over your patches, otherwise I'd
have shutde earlier :) Do you really think you want CAP_SYS_ADMIN for
trusted EAs? Soon we'll get CAP_SYS_ADMIN as catchall like old suser()..

Let me check what XFS uses for that purpose as soon as I'm back in the
office.

2003-02-15 21:29:33

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [PATCH] Extended attribute fixes, etc.

On Saturday 15 February 2003 22:09, Christoph Hellwig wrote:
> On Sat, Feb 15, 2003 at 08:17:03PM +0100, Andreas Gruenbacher wrote:
> > That sounds quite reasonable. I would have to raise CAP_SYS_ADMIN
> > for trusted EA's, though. Do you see any potential side effects
> > while a pretty powerful capability like CAP_SYS_ADMIN is
> > temporarily raised?
>
> Okay, something I missed when looking over your patches, otherwise
> I'd have shutde earlier :) Do you really think you want
> CAP_SYS_ADMIN for trusted EAs? Soon we'll get CAP_SYS_ADMIN as
> catchall like old suser()..
>
> Let me check what XFS uses for that purpose as soon as I'm back in
> the office.

The intention of Trusted Extended Attributes is for processes that
perform tasks that are relevant for the proper functioning of the
system, to allow them to use EAs. Other, non-privileged processes shall
have no access whatsoever to those EAs. This level of protection would
otherwise only be possible by providing a kernel module.

I would be quite happy with a new CAP_TRUSTED_PROCESS or whatever, but
going that route for all sorts of applications then we might soon end
up with an large number of capabilities. Maybe I'm wrong on that,
though.


Cheers,
Andreas.