On Friday 2008-05-02 14:52, Chris Mason wrote:
>>
>> An alternative approach, and IMHO better suited, is to:
>>
>> make -C ${kdir} all I_HAZ_AN_APPARMOR=1
^
M=$PWD

>This is better than the current situation (oops without any clues),

If it oopses, then you probably also get a compile-time warning on
that remove_suid line. And in that case, you can catch it with at least
a compile-time typecheck so as to not make the user head straight into
an oops, is not it?

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Fri, May 2, 2008 at 5:52 AM, Chris Mason <chris.mason@oracle.com> wrote:
> On Friday 02 May 2008, Jan Engelhardt wrote:
> > On Thursday 2008-05-01 22:10, Jeff Mahoney wrote:
> > >>>> Couldn't you #ifdef based on CONFIG_SECURITY_APPARMOR ? This ought to
> > >>>> work for Hardy. However the next development kernel (Intrepid) does
> > >>>> not have the APPARMOR patches, so just knowing that its an UBUNTU
> > >>>> kernel is not specific enough.
> > >>>
> > >>> I've been assuming the apparmor patches change remove_suid even when
> > >>> they are not enabled in the config.
> > >>
> > >> Lets get Kees involved. He developed the patch set for Hardy. I would
> > >> hope that if CONFIG_SECURITY_APPARMOR=n then the source would default to
> > >> its normal state.
> > >
> > >remove_suid() isn't the only change AppArmor makes to the VFS interface.
> > >It's pretty invasive and requires that dentries are passed with a
> > >companion vfsmount in most cases. Putting #ifdefs around all that code
> > >would make the problem worse, not better.
> >
> > An alternative approach, and IMHO better suited, is to:
> >
> > make -C ${kdir} all I_HAZ_AN_APPARMOR=1
>
> This is better than the current situation (oops without any clues), but I'd
> prefer that people not have to know what apparmor is or if they have it.
> (This isn't a knock on apparmor, I'd just rather take care of it
> automagically).

Chris,

Make is not my forte, but here is a working test to see if apparmor
exists in Ubuntu 8.04.
Maybe have make apply a patch to the btrfs source if this test
succeeds? Does this work in SUSE?

http://www.digitalprognosis.com/opensource/patches/btrfs/lame_apparmor_test_for_btrfs.patch

The symlink from /lib/modules/`uname -r`/build ->
/path/to/kernel/headers/ doesn't exist on CentOS 5.
Even though it is a hack, is this or something like it usable? Or is
this the completely wrong direction?

--
Jeff Schroeder

Don't drink and derive, alcohol and analysis don't mix.
http://www.digitalprognosis.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Friday 2008-05-02 16:15, Jeff Schroeder wrote:
>
>Make is not my forte, but here is a working test to see if apparmor
>exists in Ubuntu 8.04.
>Maybe have make apply a patch to the btrfs source if this test
>succeeds? Does this work in SUSE?
>
>http://www.digitalprognosis.com/opensource/patches/btrfs/lame_apparmor_test_for_btrfs.patch

Woah, you rely on autoconf.h. Uh no thanks, let the build system handle it.

# -*- Makefile -*-

btrfs-objs := ...
obj-m += btrfs.o and-so-on.o

ifneq (${CONFIG_SECURITY_APPARMOR},)
EXTRA_CFLAGS += -DI_HAZ_AN_APPARMOR
endif

all:
${MAKE} -C ${kdir} M=$PWD all;


Best of all, you can override that from the command line too...

make CONFIG_SECURITY_APPARMOR=y

(and if you don't, it takes its value from the appropriate .config,
all without you having to care about autoconf.h)

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Friday 02 May 2008, Jeff Schroeder wrote:

[ Btrfs oops with apparmor patched in ]

> Make is not my forte, but here is a working test to see if apparmor
> exists in Ubuntu 8.04.
> Maybe have make apply a patch to the btrfs source if this test
> succeeds? Does this work in SUSE?
>
> http://www.digitalprognosis.com/opensource/patches/btrfs/lame_apparmor_test
>_for_btrfs.patch
>

Thanks, but this uses CONFIG_SECURITY_APPARMOR which isn't enough to tell if
the kernel has the patch. Lets go back to Jeff's suse patch:

/*
* Even if AppArmor isn't enabled, it still has different prototypes.
* Add more distro/version pairs here to declare which has AppArmor applied.
*/
#if defined(CONFIG_SUSE_KERNEL)
# if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,22)
# define REMOVE_SUID_PATH 1
# endif
#endif

Could someone from Ubuntu please suggest a replacement for CONFIG_SUSE_KERNEL
and KERNEL_VERSION(2,6,22) that would correspond with ubuntu kernels shipped
with apparmor? We don't need some define from the apparmor patch, just a
global flag that says it comes from ubuntu is enough.

-chris

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Fri, May 02, 2008 at 10:34:07AM -0400, Chris Mason wrote:
> Thanks, but this uses CONFIG_SECURITY_APPARMOR which isn't enough to tell if
> the kernel has the patch. Lets go back to Jeff's suse patch:

Do we really need to support kernels compiled with the apparmour patch
applied but not enabled? That would be a custom kernel build, and I get
the impression that people Just Don't Do That -- if they want a newer
kernel, they fetch one from kernel.org. Maybe I'm wrong though.

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Friday 02 May 2008, Matthew Wilcox wrote:
> On Fri, May 02, 2008 at 10:34:07AM -0400, Chris Mason wrote:
> > Thanks, but this uses CONFIG_SECURITY_APPARMOR which isn't enough to tell
> > if the kernel has the patch. Lets go back to Jeff's suse patch:
>
> Do we really need to support kernels compiled with the apparmour patch
> applied but not enabled? That would be a custom kernel build, and I get
> the impression that people Just Don't Do That -- if they want a newer
> kernel, they fetch one from kernel.org. Maybe I'm wrong though.

Yeah, if there's no better test I'll just use the one for the apparmor config
(and stop spamming l-k with this btrfs specific stuff).

-chris



--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Friday 2008-05-02 16:38, Matthew Wilcox wrote:
>On Fri, May 02, 2008 at 10:34:07AM -0400, Chris Mason wrote:
>> Thanks, but this uses CONFIG_SECURITY_APPARMOR which isn't enough to tell if
>> the kernel has the patch. Lets go back to Jeff's suse patch:
>
>Do we really need to support kernels compiled with the apparmour patch
>applied but not enabled? That would be a custom kernel build, and I get
>the impression that people Just Don't Do That -- if they want a newer
>kernel, they fetch one from kernel.org. Maybe I'm wrong though.

I think so too. Corollary: if there is a distro that has apparmor
patched into its kernel, then there is [hopefully] a suitable rpm
nearby.

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Chris Mason wrote:
> On Friday 02 May 2008, Jeff Schroeder wrote:
>
> [ Btrfs oops with apparmor patched in ]
>
>> Make is not my forte, but here is a working test to see if apparmor
>> exists in Ubuntu 8.04.
>> Maybe have make apply a patch to the btrfs source if this test
>> succeeds? Does this work in SUSE?
>>
>> http://www.digitalprognosis.com/opensource/patches/btrfs/lame_apparmor_test
>> _for_btrfs.patch
>>
>
> Thanks, but this uses CONFIG_SECURITY_APPARMOR which isn't enough to tell if
> the kernel has the patch. Lets go back to Jeff's suse patch:
>
> /*
> * Even if AppArmor isn't enabled, it still has different prototypes.
> * Add more distro/version pairs here to declare which has AppArmor applied.
> */
> #if defined(CONFIG_SUSE_KERNEL)
> # if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,22)
> # define REMOVE_SUID_PATH 1
> # endif
> #endif
>
> Could someone from Ubuntu please suggest a replacement for CONFIG_SUSE_KERNEL
> and KERNEL_VERSION(2,6,22) that would correspond with ubuntu kernels shipped
> with apparmor? We don't need some define from the apparmor patch, just a
> global flag that says it comes from ubuntu is enough.
>
> -chris
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

To the best of my knowledge, the AppArmor patches are arch and flavour
independent. If CONFIG_SECURITY_APPARMOR exists, then the AA code is
compiled. This is certainly the case for Hardy. Neither Kees or myself
are aware of any reason why it won't also hold true for Intrepid.

rtg
--
Tim Gardner timg@tpi.com www.tpi.com
OR 503-601-0234 x102 MT 406-443-5357

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Tim Gardner wrote:
> Chris Mason wrote:
>> On Friday 02 May 2008, Jeff Schroeder wrote:
>>
>> [ Btrfs oops with apparmor patched in ]
>>
>>> Make is not my forte, but here is a working test to see if apparmor
>>> exists in Ubuntu 8.04.
>>> Maybe have make apply a patch to the btrfs source if this test
>>> succeeds? Does this work in SUSE?
>>>
>>> http://www.digitalprognosis.com/opensource/patches/btrfs/lame_apparmor_test
>>> _for_btrfs.patch
>>>
>> Thanks, but this uses CONFIG_SECURITY_APPARMOR which isn't enough to tell if
>> the kernel has the patch. Lets go back to Jeff's suse patch:
>>
>> /*
>> * Even if AppArmor isn't enabled, it still has different prototypes.
>> * Add more distro/version pairs here to declare which has AppArmor applied.
>> */
>> #if defined(CONFIG_SUSE_KERNEL)
>> # if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,22)
>> # define REMOVE_SUID_PATH 1
>> # endif
>> #endif
>>
>> Could someone from Ubuntu please suggest a replacement for CONFIG_SUSE_KERNEL
>> and KERNEL_VERSION(2,6,22) that would correspond with ubuntu kernels shipped
>> with apparmor? We don't need some define from the apparmor patch, just a
>> global flag that says it comes from ubuntu is enough.
>>
>> -chris
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
> To the best of my knowledge, the AppArmor patches are arch and flavour
> independent. If CONFIG_SECURITY_APPARMOR exists, then the AA code is
> compiled. This is certainly the case for Hardy. Neither Kees or myself
> are aware of any reason why it won't also hold true for Intrepid.

Grumble. The issue isn't whether AA is enabled, it's whether it's
present in the source. Patching the source with AA modifies a bunch of
core VFS function prototypes. CONFIG_SECURITY_APPARMOR won't exist if AA
isn't enabled, but the prototypes will have changed anyway.

The SUSE kernel doesn't export information about the presence of
particular features, but it does identify itself as a SUSE kernel so
that the pair of CONFIG_SUSE_KERNEL and the version number will identify
a release. For our enterprise kernels, where the version number won't
change over the lifetime of the release, we identify release version and
service pack levels as well.

I took a look at config-2.6.24-16-generic from 8.04 and didn't see
anything comparable.

At any rate, it's probably enough to ignore that corner case and assume
that any kernel with AA patched in will have it enabled. Anyone building
a distro kernel themselves to disable AA will probably also have the
knowledge to work around it in the btrfs source.

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iEYEARECAAYFAkgbQJoACgkQLPWxlyuTD7KhZACfZeZMBNx6x/avk5a2AED1g4rV
deEAnjgTp18gxVn4d7USmdfSXOeweG52
=/yZN
-----END PGP SIGNATURE-----

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

On Friday 2008-05-02 18:26, Jeff Mahoney wrote:
>>
>> To the best of my knowledge, the AppArmor patches are arch and flavour
>> independent. If CONFIG_SECURITY_APPARMOR exists, then the AA code is
>> compiled. This is certainly the case for Hardy. Neither Kees or myself
>> are aware of any reason why it won't also hold true for Intrepid.
>
>Grumble. The issue isn't whether AA is enabled, it's whether it's
>present in the source. Patching the source with AA modifies a bunch of
>core VFS function prototypes. CONFIG_SECURITY_APPARMOR won't exist if AA
>isn't enabled, but the prototypes will have changed anyway.

So... add an invisible CONFIG_HAVE_APPARMOR, much like
CONFIG_X86_HAVE_CMPXCHG (or whatever it's called), and test for that.
As long as you are not in the mainline kernel, every hack is
forgiven.

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team