[PATCH] remove binfmts.h from header exports

[PATCH] remove binfmts.h from header exports

[Olaf Hering]

remove linux/binfmts.h from make headers_install

A recent patch added PAGE_SIZE to the part outside of __KERNEL__.
qemu and ia32el have their own define of MAX_ARG_PAGES.
No package uses linux/binfmts.h, so it is safe to not provide it.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 include/linux/Kbuild    |    1 -
 include/linux/binfmts.h |    2 --
 2 files changed, 3 deletions(-)

--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -173,7 +173,6 @@ unifdef-y += atm.h
 unifdef-y += atm_tcp.h
 unifdef-y += audit.h
 unifdef-y += auto_fs.h
-unifdef-y += binfmts.h
 unifdef-y += capability.h
 unifdef-y += capi.h
 unifdef-y += cciss_ioctl.h
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -17,7 +17,6 @@ struct pt_regs;
 /* sizeof(linux_binprm->buf) */
 #define BINPRM_BUF_SIZE 128
 
-#ifdef __KERNEL__
 
 #define CORENAME_MAX_SIZE 128
 
@@ -99,5 +98,4 @@ extern void compute_creds(struct linux_b
 extern int do_coredump(long signr, int exit_code, struct pt_regs * regs);
 extern int set_binfmt(struct linux_binfmt *new);
 
-#endif /* __KERNEL__ */
 #endif /* _LINUX_BINFMTS_H */

Re: [PATCH] remove binfmts.h from header exports

[Alexey Dobriyan]

On Tue, Aug 07, 2007 at 10:16:03PM +0200, Olaf Hering wrote:
> remove linux/binfmts.h from make headers_install
> 
> A recent patch added PAGE_SIZE to the part outside of __KERNEL__.
> qemu and ia32el have their own define of MAX_ARG_PAGES.

Should they use kernel header instead?

> No package uses linux/binfmts.h, so it is safe to not provide it.

And? Does it contain stuff which is userspace visible?
binfmts.h has at least CORENAME_MAX_SIZE and SUID_DUMP_*

"userspace doesn't use header" is something headers_install has never
been about.

> --- a/include/linux/Kbuild
> +++ b/include/linux/Kbuild
> @@ -173,7 +173,6 @@ unifdef-y += atm.h
>  unifdef-y += atm_tcp.h
>  unifdef-y += audit.h
>  unifdef-y += auto_fs.h
> -unifdef-y += binfmts.h
>  unifdef-y += capability.h
>  unifdef-y += capi.h
>  unifdef-y += cciss_ioctl.h
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -17,7 +17,6 @@ struct pt_regs;
>  /* sizeof(linux_binprm->buf) */
>  #define BINPRM_BUF_SIZE 128
>  
> -#ifdef __KERNEL__
>  
>  #define CORENAME_MAX_SIZE 128
>  
> @@ -99,5 +98,4 @@ extern void compute_creds(struct linux_b
>  extern int do_coredump(long signr, int exit_code, struct pt_regs * regs);
>  extern int set_binfmt(struct linux_binfmt *new);
>  
> -#endif /* __KERNEL__ */
>  #endif /* _LINUX_BINFMTS_H */

Re: [PATCH] remove binfmts.h from header exports

[Olaf Hering]

On Wed, Aug 08, Alexey Dobriyan wrote:

> On Tue, Aug 07, 2007 at 10:16:03PM +0200, Olaf Hering wrote:
> > remove linux/binfmts.h from make headers_install
> > 
> > A recent patch added PAGE_SIZE to the part outside of __KERNEL__.
> > qemu and ia32el have their own define of MAX_ARG_PAGES.
> 
> Should they use kernel header instead?

No, because the header will disappear. And from my understanding, the
args limit is now gone.

> > No package uses linux/binfmts.h, so it is safe to not provide it.
> 
> And? Does it contain stuff which is userspace visible?
> binfmts.h has at least CORENAME_MAX_SIZE and SUID_DUMP_*

Yes, its inside __KERNEL__. Have you read that header already? We are
talking about the part below.
What part is useable for an application?

....
#include <linux/capability.h>

struct pt_regs;

/*
 * These are the maximum length and maximum number of strings passed to the
 * execve() system call.  MAX_ARG_STRLEN is essentially random but serves to
 * prevent the kernel from being unduly impacted by misaddressed pointers.
 * MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer.
 */
#define MAX_ARG_STRLEN (PAGE_SIZE * 32)
#define MAX_ARG_STRINGS 0x7FFFFFFF

/* sizeof(linux_binprm->buf) */
#define BINPRM_BUF_SIZE 128
....

> "userspace doesn't use header" is something headers_install has never
> been about.

Thats true.
I remember someone even wrote something for Documentation/ a few days ago.

Re: [PATCH] remove binfmts.h from header exports

[Alexey Dobriyan]

On 8/8/07, Olaf Hering <olaf@aepfle.de> wrote:
> On Wed, Aug 08, Alexey Dobriyan wrote:
>
> > On Tue, Aug 07, 2007 at 10:16:03PM +0200, Olaf Hering wrote:
> > > remove linux/binfmts.h from make headers_install
> > >
> > > A recent patch added PAGE_SIZE to the part outside of __KERNEL__.
> > > qemu and ia32el have their own define of MAX_ARG_PAGES.
> >
> > Should they use kernel header instead?
>
> No, because the header will disappear. And from my understanding, the
> args limit is now gone.

OK.

> > > No package uses linux/binfmts.h, so it is safe to not provide it.
> >
> > And? Does it contain stuff which is userspace visible?
> > binfmts.h has at least CORENAME_MAX_SIZE and SUID_DUMP_*
>
> Yes, its inside __KERNEL__.

Can't you accept for a second that some stuff under __KERNEL__
was put there by mistake?

> We are talking about the part below.

No, we are talking about whole header since you're going
to unexport whole header.

> What part is useable for an application?

>From part you quoted nothing. Otherwise:

fd = open("/proc/sys/fs/suid_dumpable", 1);
snprintf(buf, sizeof(buf), "%d", SUID_DUMP_ROOT);
write(fd, buf, strlen(buf));

> #include <linux/capability.h>
>
> struct pt_regs;
>
> /*
>  * These are the maximum length and maximum number of strings passed to the
>  * execve() system call.  MAX_ARG_STRLEN is essentially random but serves to
>  * prevent the kernel from being unduly impacted by misaddressed pointers.
>  * MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer.
>  */
> #define MAX_ARG_STRLEN (PAGE_SIZE * 32)
> #define MAX_ARG_STRINGS 0x7FFFFFFF
>
> /* sizeof(linux_binprm->buf) */
> #define BINPRM_BUF_SIZE 128

Re: [PATCH] remove binfmts.h from header exports

[Olaf Hering]

On Wed, Aug 08, Alexey Dobriyan wrote:

> > > And? Does it contain stuff which is userspace visible?
> > > binfmts.h has at least CORENAME_MAX_SIZE and SUID_DUMP_*
> >
> > Yes, its inside __KERNEL__.
> 
> Can't you accept for a second that some stuff under __KERNEL__
> was put there by mistake?

I wonder why the defines exist at all? My grep doesnt find SUID_DUMP_*.
Looking at the commit message from d6e711448137ca3301512cec41a2c2ce852b3d0a

...
    > >         if (current->euid == current->uid && current->egid == current->gid)
    > >                 current->mm->dumpable = 1;
    >
    > Should this be SUID_DUMP_USER?

    Actually the feedback I had from last time was that the SUID_ defines
    should go because its clearer to follow the numbers. They can go
    everywhere (and there are lots of places where dumpable is tested/used
    as a bool in untouched code)
...

Exporting a random array size is not very useful. The app and the kernel
has to check the string length anyway.

So lets remove the header and the 3 unused defines.

Re: [PATCH] remove binfmts.h from header exports

[Rene Herman]

On 08/07/2007 10:16 PM, Olaf Hering wrote:

> remove linux/binfmts.h from make headers_install
> 
> A recent patch added PAGE_SIZE to the part outside of __KERNEL__.
> qemu and ia32el have their own define of MAX_ARG_PAGES.
> No package uses linux/binfmts.h, so it is safe to not provide it.

How is "all packages" defined?

Rene.

Re: [PATCH] remove binfmts.h from header exports

[Olaf Hering]

On Wed, Aug 08, Rene Herman wrote:

> >No package uses linux/binfmts.h, so it is safe to not provide it.
> 
> How is "all packages" defined?

3052 openSuSE 10.3 packages.