2008-07-15 12:58:33

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] chsc headers userspace cleanup

Kernel headers shouldn't expose functions to userspace.

Signed-off-by: Adrian Bunk <[email protected]>

---

BTW: For compile-testing my patches it would be nice if s390 had
more defconfigs (e.g. also a non-64bit one).

include/asm-s390/Kbuild | 2 +-
include/asm-s390/chpid.h | 6 +++---
include/asm-s390/schid.h | 3 +++
3 files changed, 7 insertions(+), 4 deletions(-)

0ee038c850fe402d0a8a548b438ae045e8e06f50
diff --git a/include/asm-s390/Kbuild b/include/asm-s390/Kbuild
index 09f3125..bb5e9ed 100644
--- a/include/asm-s390/Kbuild
+++ b/include/asm-s390/Kbuild
@@ -8,9 +8,9 @@ header-y += ucontext.h
header-y += vtoc.h
header-y += zcrypt.h
header-y += kvm.h
-header-y += schid.h
header-y += chsc.h

unifdef-y += cmb.h
unifdef-y += debug.h
unifdef-y += chpid.h
+unifdef-y += schid.h
diff --git a/include/asm-s390/chpid.h b/include/asm-s390/chpid.h
index 606844d..dfe3c7f 100644
--- a/include/asm-s390/chpid.h
+++ b/include/asm-s390/chpid.h
@@ -20,6 +20,9 @@ struct chp_id {
u8 id;
} __attribute__((packed));

+#ifdef __KERNEL__
+#include <asm/cio.h>
+
static inline void chp_id_init(struct chp_id *chpid)
{
memset(chpid, 0, sizeof(struct chp_id));
@@ -40,9 +43,6 @@ static inline void chp_id_next(struct chp_id *chpid)
}
}

-#ifdef __KERNEL__
-#include <asm/cio.h>
-
static inline int chp_id_is_valid(struct chp_id *chpid)
{
return (chpid->cssid <= __MAX_CSSID);
diff --git a/include/asm-s390/schid.h b/include/asm-s390/schid.h
index 5017ffa..7707eb9 100644
--- a/include/asm-s390/schid.h
+++ b/include/asm-s390/schid.h
@@ -10,6 +10,7 @@ struct subchannel_id {
__u32 sch_no : 16;
} __attribute__ ((packed, aligned(4)));

+#ifdef __KERNEL__

/* Helper function for sane state of pre-allocated subchannel_id. */
static inline void
@@ -25,4 +26,6 @@ schid_equal(struct subchannel_id *schid1, struct subchannel_id *schid2)
return !memcmp(schid1, schid2, sizeof(struct subchannel_id));
}

+#endif /* __KERNEL__ */
+
#endif /* ASM_SCHID_H */


2008-07-15 17:18:17

by Cornelia Huck

[permalink] [raw]
Subject: Re: [2.6 patch] chsc headers userspace cleanup

On Tue, 15 Jul 2008 15:58:18 +0300,
Adrian Bunk <[email protected]> wrote:

> Kernel headers shouldn't expose functions to userspace.

Could you please elaborate? Especially as I see e.g.
include/linux/virtio_ring.h exporting functions outside #ifdef
__KERNEL__ as well...

2008-07-15 21:02:49

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] chsc headers userspace cleanup

On Tue, Jul 15, 2008 at 07:17:48PM +0200, Cornelia Huck wrote:
> On Tue, 15 Jul 2008 15:58:18 +0300,
> Adrian Bunk <[email protected]> wrote:
>
> > Kernel headers shouldn't expose functions to userspace.
>
> Could you please elaborate? Especially as I see e.g.
> include/linux/virtio_ring.h exporting functions outside #ifdef
> __KERNEL__ as well...

Our headers are in a bad shape...

The userspace headers should contain everything that is part of the ABI
between the kernel and userspace.

Nothing more.

The kernel is heavily changing with each release while the userspace ABI
is cast in stone, and leaking more stuff to userspace than required only
increases the chances of some userspace programmer using it and some
kernel programmer then changing it.

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-07-16 08:17:00

by Heiko Carstens

[permalink] [raw]
Subject: Re: [2.6 patch] chsc headers userspace cleanup

On Tue, Jul 15, 2008 at 03:58:18PM +0300, Adrian Bunk wrote:
> Kernel headers shouldn't expose functions to userspace.
>
> Signed-off-by: Adrian Bunk <[email protected]>

Makes sense. Applied -- Thanks.

> BTW: For compile-testing my patches it would be nice if s390 had
> more defconfigs (e.g. also a non-64bit one).

Yes, will consider that ;)