All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ext2: Don't export ext2_mask_flags() to user space
@ 2012-03-21 21:50 Thierry Reding
  2012-03-21 22:23 ` Jan Kara
  0 siblings, 1 reply; 22+ messages in thread
From: Thierry Reding @ 2012-03-21 21:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: Al Viro, Jan Kara, linux-ext4, stable

Using the exported linux/ext2_fs.h header from user space is broken
since commit 0583fcc because the umode_t type is no longer exported.
This happens for example when compiling busybox' tune2fs applet. The
e2fsprogs package does not have this problem because it ships with
its own header and doesn't use the one exported by the Linux kernel.

However, since busybox doesn't use the ext2_mask_flags() function the
easiest fix is to just not export it and as a result the umode_t type
definition is longer needed.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Jan Kara <jack@suse.cz>
Cc: linux-ext4@vger.kernel.org
Cc: stable@vger.kernel.org
---
 include/linux/ext2_fs.h |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/linux/ext2_fs.h b/include/linux/ext2_fs.h
index 091ab48..20f8f33 100644
--- a/include/linux/ext2_fs.h
+++ b/include/linux/ext2_fs.h
@@ -209,10 +209,7 @@ struct ext2_group_desc
 #define EXT2_OTHER_FLMASK (EXT2_NODUMP_FL | EXT2_NOATIME_FL)
 
 /* Mask out flags that are inappropriate for the given type of inode. */
-#ifndef __KERNEL__
-typedef unsigned short umode_t;
-#endif
-
+#ifdef __KERNEL__
 static inline __u32 ext2_mask_flags(umode_t mode, __u32 flags)
 {
 	if (S_ISDIR(mode))
@@ -222,6 +219,7 @@ static inline __u32 ext2_mask_flags(umode_t mode, __u32 flags)
 	else
 		return flags & EXT2_OTHER_FLMASK;
 }
+#endif
 
 /*
  * ioctl commands
-- 
1.7.9.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-21 21:50 [PATCH] ext2: Don't export ext2_mask_flags() to user space Thierry Reding
@ 2012-03-21 22:23 ` Jan Kara
  2012-03-21 23:16   ` Ted Ts'o
  0 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2012-03-21 22:23 UTC (permalink / raw)
  To: Thierry Reding; +Cc: linux-kernel, Al Viro, Jan Kara, linux-ext4, stable, tytso

On Wed 21-03-12 22:50:07, Thierry Reding wrote:
> Using the exported linux/ext2_fs.h header from user space is broken
> since commit 0583fcc because the umode_t type is no longer exported.
> This happens for example when compiling busybox' tune2fs applet. The
> e2fsprogs package does not have this problem because it ships with
> its own header and doesn't use the one exported by the Linux kernel.
> 
> However, since busybox doesn't use the ext2_mask_flags() function the
> easiest fix is to just not export it and as a result the umode_t type
> definition is longer needed.
  Frankly, anybody seriously wanting to do anything with ext[2-4]
filesystems should use header files as provided by libext2fs. So I wonder
if just unexporting the whole file wouldn't be the best solution going
forward. Ted, do you have opinion?

								Honza


> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Jan Kara <jack@suse.cz>
> Cc: linux-ext4@vger.kernel.org
> Cc: stable@vger.kernel.org
> ---
>  include/linux/ext2_fs.h |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/ext2_fs.h b/include/linux/ext2_fs.h
> index 091ab48..20f8f33 100644
> --- a/include/linux/ext2_fs.h
> +++ b/include/linux/ext2_fs.h
> @@ -209,10 +209,7 @@ struct ext2_group_desc
>  #define EXT2_OTHER_FLMASK (EXT2_NODUMP_FL | EXT2_NOATIME_FL)
>  
>  /* Mask out flags that are inappropriate for the given type of inode. */
> -#ifndef __KERNEL__
> -typedef unsigned short umode_t;
> -#endif
> -
> +#ifdef __KERNEL__
>  static inline __u32 ext2_mask_flags(umode_t mode, __u32 flags)
>  {
>  	if (S_ISDIR(mode))
> @@ -222,6 +219,7 @@ static inline __u32 ext2_mask_flags(umode_t mode, __u32 flags)
>  	else
>  		return flags & EXT2_OTHER_FLMASK;
>  }
> +#endif
>  
>  /*
>   * ioctl commands
> -- 
> 1.7.9.4
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-21 22:23 ` Jan Kara
@ 2012-03-21 23:16   ` Ted Ts'o
  2012-03-22  5:53     ` Thierry Reding
  0 siblings, 1 reply; 22+ messages in thread
From: Ted Ts'o @ 2012-03-21 23:16 UTC (permalink / raw)
  To: Jan Kara; +Cc: Thierry Reding, linux-kernel, Al Viro, linux-ext4, stable

On Wed, Mar 21, 2012 at 11:23:57PM +0100, Jan Kara wrote:
>   Frankly, anybody seriously wanting to do anything with ext[2-4]
> filesystems should use header files as provided by libext2fs. So I wonder
> if just unexporting the whole file wouldn't be the best solution going
> forward. Ted, do you have opinion?

Agreed, it's been almost 8 years since e2fsprogs used the
include/linux/ext2_fs.h (the last version that needed it was e2fsprogs
1.35, released February 28, 2004).

There shouldn't be *anyone* using any of the ext2/3/4 kernel header
files.  The only program that might be cheating and using kernel
header files is ext3grep, as the author wasn't willing to fix his
applications to use libext2fs.  (As a result, it doesn't work on ext4
file systems, where as properly coded programs that do use libext2fs
often work just fine on ext4, such as e2tools, which hasn't been
modified for something like eight years but which works on ext4 just
fine.)

So yeah, I'd just unexport ext2_fs.h, and probably ext3_fs.h as well.

   	     	  	   	      	  - Ted

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-21 23:16   ` Ted Ts'o
@ 2012-03-22  5:53     ` Thierry Reding
  2012-03-22  6:00       ` Artem Bityutskiy
  0 siblings, 1 reply; 22+ messages in thread
From: Thierry Reding @ 2012-03-22  5:53 UTC (permalink / raw)
  To: Ted Ts'o, Jan Kara, linux-kernel, Al Viro, linux-ext4, stable

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

* Ted Ts'o wrote:
> On Wed, Mar 21, 2012 at 11:23:57PM +0100, Jan Kara wrote:
> >   Frankly, anybody seriously wanting to do anything with ext[2-4]
> > filesystems should use header files as provided by libext2fs. So I wonder
> > if just unexporting the whole file wouldn't be the best solution going
> > forward. Ted, do you have opinion?
> 
> Agreed, it's been almost 8 years since e2fsprogs used the
> include/linux/ext2_fs.h (the last version that needed it was e2fsprogs
> 1.35, released February 28, 2004).
> 
> There shouldn't be *anyone* using any of the ext2/3/4 kernel header
> files.  The only program that might be cheating and using kernel
> header files is ext3grep, as the author wasn't willing to fix his
> applications to use libext2fs.  (As a result, it doesn't work on ext4
> file systems, where as properly coded programs that do use libext2fs
> often work just fine on ext4, such as e2tools, which hasn't been
> modified for something like eight years but which works on ext4 just
> fine.)
> 
> So yeah, I'd just unexport ext2_fs.h, and probably ext3_fs.h as well.

Alright. Do you want me to send a patch to do so?

What's the recommended fix for packages that cannot or will not use
libext2fs, like busybox? Copy the required parts into a private header
and use that instead?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-22  5:53     ` Thierry Reding
@ 2012-03-22  6:00       ` Artem Bityutskiy
  2012-03-22 16:28         ` Ted Ts'o
  0 siblings, 1 reply; 22+ messages in thread
From: Artem Bityutskiy @ 2012-03-22  6:00 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Ted Ts'o, Jan Kara, linux-kernel, Al Viro, linux-ext4, stable

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On Thu, 2012-03-22 at 06:53 +0100, Thierry Reding wrote:
> What's the recommended fix for packages that cannot or will not use
> libext2fs, like busybox? Copy the required parts into a private header
> and use that instead?

The normal way is to just keep a private copy of the whole header file.
Because the on-disk format stays compatible, those programs do not have
to update the header very often - only rarely if they want to support
some new feature.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-22  6:00       ` Artem Bityutskiy
@ 2012-03-22 16:28         ` Ted Ts'o
  2012-03-22 16:47           ` Thierry Reding
  2012-03-22 17:31           ` [PATCH] ext2: Don't export ext2_mask_flags() " Al Viro
  0 siblings, 2 replies; 22+ messages in thread
From: Ted Ts'o @ 2012-03-22 16:28 UTC (permalink / raw)
  To: Artem Bityutskiy
  Cc: Thierry Reding, Jan Kara, linux-kernel, Al Viro, linux-ext4, stable

On Thu, Mar 22, 2012 at 08:00:41AM +0200, Artem Bityutskiy wrote:
> On Thu, 2012-03-22 at 06:53 +0100, Thierry Reding wrote:
> > What's the recommended fix for packages that cannot or will not use
> > libext2fs, like busybox? Copy the required parts into a private header
> > and use that instead?
> 
> The normal way is to just keep a private copy of the whole header file.
> Because the on-disk format stays compatible, those programs do not have
> to update the header very often - only rarely if they want to support
> some new feature.

Even if they're not iwlling to use libext2fs (for space reasons, I
would assume?  It can't be because of license compatibility issues
since they are both GPLv2), they could just simply grab the ext2_fs.h
from e2fsprogs.  That has all of the file system definitions for ext2,
ext3, and ext4.

Regards,

						- Ted

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-22 16:28         ` Ted Ts'o
@ 2012-03-22 16:47           ` Thierry Reding
  2012-03-22 18:31             ` Ted Ts'o
  2012-03-22 17:31           ` [PATCH] ext2: Don't export ext2_mask_flags() " Al Viro
  1 sibling, 1 reply; 22+ messages in thread
From: Thierry Reding @ 2012-03-22 16:47 UTC (permalink / raw)
  To: Ted Ts'o, Artem Bityutskiy, Jan Kara, linux-kernel, Al Viro,
	linux-ext4, stable

[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]

* Ted Ts'o wrote:
> On Thu, Mar 22, 2012 at 08:00:41AM +0200, Artem Bityutskiy wrote:
> > On Thu, 2012-03-22 at 06:53 +0100, Thierry Reding wrote:
> > > What's the recommended fix for packages that cannot or will not use
> > > libext2fs, like busybox? Copy the required parts into a private header
> > > and use that instead?
> > 
> > The normal way is to just keep a private copy of the whole header file.
> > Because the on-disk format stays compatible, those programs do not have
> > to update the header very often - only rarely if they want to support
> > some new feature.
> 
> Even if they're not iwlling to use libext2fs (for space reasons, I
> would assume?  It can't be because of license compatibility issues
> since they are both GPLv2), they could just simply grab the ext2_fs.h
> from e2fsprogs.  That has all of the file system definitions for ext2,
> ext3, and ext4.

In fact there is already a file, e2fs_defs.h, that seems to be based on the
contents of the ext2_fs.h from the kernel. I've posted two patches to the
busybox mailing list that fix the build without using linux/ext2_fs.h. For
reference, they can be found here:

	http://lists.busybox.net/pipermail/busybox/2012-March/077563.html
	http://lists.busybox.net/pipermail/busybox/2012-March/077562.html

Do you still want me to prepare a patch to unexport ext2_fs.h or will you
take care of it?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-22 16:28         ` Ted Ts'o
  2012-03-22 16:47           ` Thierry Reding
@ 2012-03-22 17:31           ` Al Viro
  2012-03-23  0:25             ` Andreas Dilger
  2012-03-24  6:50               ` Mike Frysinger
  1 sibling, 2 replies; 22+ messages in thread
From: Al Viro @ 2012-03-22 17:31 UTC (permalink / raw)
  To: Ted Ts'o, Artem Bityutskiy, Thierry Reding, Jan Kara,
	linux-kernel, linux-ext4, stable

On Thu, Mar 22, 2012 at 12:28:25PM -0400, Ted Ts'o wrote:
> On Thu, Mar 22, 2012 at 08:00:41AM +0200, Artem Bityutskiy wrote:
> > On Thu, 2012-03-22 at 06:53 +0100, Thierry Reding wrote:
> > > What's the recommended fix for packages that cannot or will not use
> > > libext2fs, like busybox? Copy the required parts into a private header
> > > and use that instead?
> > 
> > The normal way is to just keep a private copy of the whole header file.
> > Because the on-disk format stays compatible, those programs do not have
> > to update the header very often - only rarely if they want to support
> > some new feature.
> 
> Even if they're not iwlling to use libext2fs (for space reasons, I
> would assume?  It can't be because of license compatibility issues
> since they are both GPLv2), they could just simply grab the ext2_fs.h
> from e2fsprogs.  That has all of the file system definitions for ext2,
> ext3, and ext4.

Ho-hum...  Then we could kill a lot of lines in include/linux/ext2_fs.h.
I wonder how much of what remains has any business being outside of
fs/ext2, actually - AFAICS, there are very few places that might possibly
care:

arch/blackfin/kernel/setup.c:595:       if (*((unsigned short *)(mtd_phys + 0x438)) == EXT2_SUPER_MAGIC)
fs/nfsd/nfs3proc.c:599:         case EXT2_SUPER_MAGIC:  
fs/nfsd/nfs3proc.c:600:                 resp->p_link_max = EXT2_LINK_MAX;
fs/nfsd/nfs3proc.c:601:                 resp->p_name_max = EXT2_NAME_LEN;
init/do_mounts_rd.c:57: struct ext2_super_block *ext2sb;
init/do_mounts_rd.c:70: ext2sb = (struct ext2_super_block *) buf;
init/do_mounts_rd.c:153:        if (ext2sb->s_magic == cpu_to_le16(EXT2_SUPER_MAGIC)) {
init/do_mounts_rd.c:155:                       "RAMDISK: ext2 filesystem found at block %d\n",
init/do_mounts_rd.c:157:                nblocks = le32_to_cpu(ext2sb->s_blocks_count) <<
init/do_mounts_rd.c:158:                        le32_to_cpu(ext2sb->s_log_block_size);
security/selinux/hooks.c:2974:  case EXT2_IOC_GETFLAGS:
security/selinux/hooks.c:2976:  case EXT2_IOC_GETVERSION:
security/selinux/hooks.c:2980:  case EXT2_IOC_SETFLAGS:
security/selinux/hooks.c:2982:  case EXT2_IOC_SETVERSION:

and that's it.  blackfin and do_mounts_rd are doing the same thing (blackfin -
buggy, AFAICS).  Looks like both are  asking for something along the lines of
sector_t detect_ext2(void *image), returning 0 if it's not one and size in
kilobytes if it is...  nfsd one is just plain weird; what the hell is going
on there?  And selinux wants to know 4 ioctl numbers.

Everything else doesn't go beyond fs/ext2; there's a couple of odd macros
in ext[34]_fs.h (EXT._FEATURE_COMPAT_SUPP) using EXT2_FEATURE_COMPAT_EXT_ATTR,
but they are not used anywhere *and* EXT2_FEATURE_COMPAT_EXT_ATTR is not
available in the places that include those headers...
*and* 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-22 16:47           ` Thierry Reding
@ 2012-03-22 18:31             ` Ted Ts'o
  2012-03-23  9:27                 ` Thierry Reding
  0 siblings, 1 reply; 22+ messages in thread
From: Ted Ts'o @ 2012-03-22 18:31 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Artem Bityutskiy, Jan Kara, linux-kernel, Al Viro, linux-ext4, stable

On Thu, Mar 22, 2012 at 05:47:51PM +0100, Thierry Reding wrote:
> 
> Do you still want me to prepare a patch to unexport ext2_fs.h or will you
> take care of it?

If you could do it, that would be great.  I've been a bit overloaded
lately, so the more I can delegate to others, the better....

Thanks,

					- Ted

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-22 17:31           ` [PATCH] ext2: Don't export ext2_mask_flags() " Al Viro
@ 2012-03-23  0:25             ` Andreas Dilger
  2012-03-24  6:51               ` Mike Frysinger
  2012-03-24  6:50               ` Mike Frysinger
  1 sibling, 1 reply; 22+ messages in thread
From: Andreas Dilger @ 2012-03-23  0:25 UTC (permalink / raw)
  To: Al Viro
  Cc: Ted Ts'o, Artem Bityutskiy, ThierryReding, Jan Kara,
	linux-kernel, linux-ext4, stable

On 2012-03-22, at 11:31, Al Viro <viro@ZenIV.linux.org.uk> wrote:

> On Thu, Mar 22, 2012 at 12:28:25PM -0400, Ted Ts'o wrote:
>> 
>> Even if they're not iwlling to use libext2fs (for space reasons, I
>> would assume?  It can't be because of license compatibility issues
>> since they are both GPLv2), they could just simply grab the ext2_fs.h
>> from e2fsprogs.  That has all of the file system definitions for ext2,
>> ext3, and ext4.
> 
> Ho-hum...  Then we could kill a lot of lines in include/linux/ext2_fs.h.
> I wonder how much of what remains has any business being outside of
> fs/ext2, actually - AFAICS, there are very few places that might possibly
> care:
> 
> arch/blackfin/kernel/setup.c:595:       if (*((unsigned short *)(mtd_phys + 0x438)) == EXT2_SUPER_MAGIC)
> fs/nfsd/nfs3proc.c:599:         case EXT2_SUPER_MAGIC:  
> fs/nfsd/nfs3proc.c:600:                 resp->p_link_max = EXT2_LINK_MAX;
> fs/nfsd/nfs3proc.c:601:                 resp->p_name_max = EXT2_NAME_LEN;

These don't really make sense to be using ext2 constants. 

> init/do_mounts_rd.c:57: struct ext2_super_block *ext2sb;
> init/do_mounts_rd.c:70: ext2sb = (struct ext2_super_block *) buf;
> init/do_mounts_rd.c:153:        if (ext2sb->s_magic == cpu_to_le16(EXT2_SUPER_MAGIC)) {
> init/do_mounts_rd.c:155:                       "RAMDISK: ext2 filesystem found at block %d\n",
> init/do_mounts_rd.c:157:                nblocks = le32_to_cpu(ext2sb->s_blocks_count) <<
> init/do_mounts_rd.c:158:                        le32_to_cpu(ext2sb->s_log_block_size);
> security/selinux/hooks.c:2974:  case EXT2_IOC_GETFLAGS:
> security/selinux/hooks.c:2976:  case EXT2_IOC_GETVERSION:
> security/selinux/hooks.c:2980:  case EXT2_IOC_SETFLAGS:
> security/selinux/hooks.c:2982:  case EXT2_IOC_SETVERSION:

These ones should be using the generic FS_IOC_{GET,SET}_FLAGS.

It isn't clear that there is any legitimate use for EXT2_IOC_SETVERSION, since it isn't possible to allocate specific inode numbers, so there isn't really any use to set the inode version.  Ostensibly it was for user-space NFS, but it can't be used for this, and we are planning to deprecate it from ext2/3/4 already due to incompatibility with the metadata checksum feature. 

> and that's it.  blackfin and do_mounts_rd are doing the same thing (blackfin -
> buggy, AFAICS).  Looks like both are  asking for something along the lines of
> sector_t detect_ext2(void *image), returning 0 if it's not one and size in
> kilobytes if it is...  nfsd one is just plain weird; what the hell is going
> on there?  And selinux wants to know 4 ioctl numbers.
> 
> Everything else doesn't go beyond fs/ext2; there's a couple of odd macros
> in ext[34]_fs.h (EXT._FEATURE_COMPAT_SUPP) using EXT2_FEATURE_COMPAT_EXT_ATTR,
> but they are not used anywhere *and* EXT2_FEATURE_COMPAT_EXT_ATTR is not
> available in the places that include those headers...
> *and* 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] ext2: No longer export ext2_fs.h to user space
  2012-03-22 18:31             ` Ted Ts'o
@ 2012-03-23  9:27                 ` Thierry Reding
  0 siblings, 0 replies; 22+ messages in thread
From: Thierry Reding @ 2012-03-23  9:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Al Viro, Jan Kara, Ted Ts'o, Artem Bityutskiy,
	Andreas Dilger, linux-ext4

Since the on-disk format has been stable for quite some time, users
should either use the headers provided by libext2fs or keep a private
copy of this header. For the full discussion, see this thread:

	https://lkml.org/lkml/2012/3/21/516

While at it, this commit removes all __KERNEL__ guards, which are now
unnecessary.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Jan Kara <jack@suse.cz>
Cc: Ted Ts'o <tytso@mit.edu>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Andreas Dilger <aedilger@gmail.com>
Cc: linux-ext4@vger.kernel.org
---
 include/linux/Kbuild    |    1 -
 include/linux/ext2_fs.h |   72 +++++++----------------------------------------
 2 files changed, 10 insertions(+), 63 deletions(-)

diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index a255553..1cb0704 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -120,7 +120,6 @@ header-y += errno.h
 header-y += errqueue.h
 header-y += ethtool.h
 header-y += eventpoll.h
-header-y += ext2_fs.h
 header-y += fadvise.h
 header-y += falloc.h
 header-y += fanotify.h
diff --git a/include/linux/ext2_fs.h b/include/linux/ext2_fs.h
index ce1b719..f28dba5 100644
--- a/include/linux/ext2_fs.h
+++ b/include/linux/ext2_fs.h
@@ -20,6 +20,8 @@
 #include <linux/magic.h>
 #include <linux/fs.h>
 
+#include <linux/ext2_fs_sb.h>
+
 /*
  * The second extended filesystem constants/structures
  */
@@ -66,18 +68,10 @@
 /* First non-reserved inode for old ext2 filesystems */
 #define EXT2_GOOD_OLD_FIRST_INO	11
 
-#ifdef __KERNEL__
-#include <linux/ext2_fs_sb.h>
 static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
 {
 	return sb->s_fs_info;
 }
-#else
-/* Assume that user mode programs are passing in an ext2fs superblock, not
- * a kernel struct super_block.  This will allow us to call the feature-test
- * macros from user land. */
-#define EXT2_SB(sb)	(sb)
-#endif
 
 /*
  * Maximal count of links to a file
@@ -90,29 +84,12 @@ static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
 #define EXT2_MIN_BLOCK_SIZE		1024
 #define	EXT2_MAX_BLOCK_SIZE		4096
 #define EXT2_MIN_BLOCK_LOG_SIZE		  10
-#ifdef __KERNEL__
-# define EXT2_BLOCK_SIZE(s)		((s)->s_blocksize)
-#else
-# define EXT2_BLOCK_SIZE(s)		(EXT2_MIN_BLOCK_SIZE << (s)->s_log_block_size)
-#endif
+#define EXT2_BLOCK_SIZE(s)		((s)->s_blocksize)
 #define	EXT2_ADDR_PER_BLOCK(s)		(EXT2_BLOCK_SIZE(s) / sizeof (__u32))
-#ifdef __KERNEL__
-# define EXT2_BLOCK_SIZE_BITS(s)	((s)->s_blocksize_bits)
-#else
-# define EXT2_BLOCK_SIZE_BITS(s)	((s)->s_log_block_size + 10)
-#endif
-#ifdef __KERNEL__
+#define EXT2_BLOCK_SIZE_BITS(s)		((s)->s_blocksize_bits)
 #define	EXT2_ADDR_PER_BLOCK_BITS(s)	(EXT2_SB(s)->s_addr_per_block_bits)
 #define EXT2_INODE_SIZE(s)		(EXT2_SB(s)->s_inode_size)
 #define EXT2_FIRST_INO(s)		(EXT2_SB(s)->s_first_ino)
-#else
-#define EXT2_INODE_SIZE(s)	(((s)->s_rev_level == EXT2_GOOD_OLD_REV) ? \
-				 EXT2_GOOD_OLD_INODE_SIZE : \
-				 (s)->s_inode_size)
-#define EXT2_FIRST_INO(s)	(((s)->s_rev_level == EXT2_GOOD_OLD_REV) ? \
-				 EXT2_GOOD_OLD_FIRST_INO : \
-				 (s)->s_first_ino)
-#endif
 
 /*
  * Macro-instructions used to manage fragments
@@ -120,13 +97,8 @@ static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
 #define EXT2_MIN_FRAG_SIZE		1024
 #define	EXT2_MAX_FRAG_SIZE		4096
 #define EXT2_MIN_FRAG_LOG_SIZE		  10
-#ifdef __KERNEL__
-# define EXT2_FRAG_SIZE(s)		(EXT2_SB(s)->s_frag_size)
-# define EXT2_FRAGS_PER_BLOCK(s)	(EXT2_SB(s)->s_frags_per_block)
-#else
-# define EXT2_FRAG_SIZE(s)		(EXT2_MIN_FRAG_SIZE << (s)->s_log_frag_size)
-# define EXT2_FRAGS_PER_BLOCK(s)	(EXT2_BLOCK_SIZE(s) / EXT2_FRAG_SIZE(s))
-#endif
+#define EXT2_FRAG_SIZE(s)		(EXT2_SB(s)->s_frag_size)
+#define EXT2_FRAGS_PER_BLOCK(s)		(EXT2_SB(s)->s_frags_per_block)
 
 /*
  * Structure of a blocks group descriptor
@@ -146,16 +118,10 @@ struct ext2_group_desc
 /*
  * Macro-instructions used to manage group descriptors
  */
-#ifdef __KERNEL__
-# define EXT2_BLOCKS_PER_GROUP(s)	(EXT2_SB(s)->s_blocks_per_group)
-# define EXT2_DESC_PER_BLOCK(s)		(EXT2_SB(s)->s_desc_per_block)
-# define EXT2_INODES_PER_GROUP(s)	(EXT2_SB(s)->s_inodes_per_group)
-# define EXT2_DESC_PER_BLOCK_BITS(s)	(EXT2_SB(s)->s_desc_per_block_bits)
-#else
-# define EXT2_BLOCKS_PER_GROUP(s)	((s)->s_blocks_per_group)
-# define EXT2_DESC_PER_BLOCK(s)		(EXT2_BLOCK_SIZE(s) / sizeof (struct ext2_group_desc))
-# define EXT2_INODES_PER_GROUP(s)	((s)->s_inodes_per_group)
-#endif
+#define EXT2_BLOCKS_PER_GROUP(s)	(EXT2_SB(s)->s_blocks_per_group)
+#define EXT2_DESC_PER_BLOCK(s)		(EXT2_SB(s)->s_desc_per_block)
+#define EXT2_INODES_PER_GROUP(s)	(EXT2_SB(s)->s_inodes_per_group)
+#define EXT2_DESC_PER_BLOCK_BITS(s)	(EXT2_SB(s)->s_desc_per_block_bits)
 
 /*
  * Constants relative to the data blocks
@@ -296,7 +262,6 @@ struct ext2_inode {
 
 #define i_size_high	i_dir_acl
 
-#if defined(__KERNEL__) || defined(__linux__)
 #define i_reserved1	osd1.linux1.l_i_reserved1
 #define i_frag		osd2.linux2.l_i_frag
 #define i_fsize		osd2.linux2.l_i_fsize
@@ -305,23 +270,6 @@ struct ext2_inode {
 #define i_uid_high	osd2.linux2.l_i_uid_high
 #define i_gid_high	osd2.linux2.l_i_gid_high
 #define i_reserved2	osd2.linux2.l_i_reserved2
-#endif
-
-#ifdef	__hurd__
-#define i_translator	osd1.hurd1.h_i_translator
-#define i_frag		osd2.hurd2.h_i_frag
-#define i_fsize		osd2.hurd2.h_i_fsize
-#define i_uid_high	osd2.hurd2.h_i_uid_high
-#define i_gid_high	osd2.hurd2.h_i_gid_high
-#define i_author	osd2.hurd2.h_i_author
-#endif
-
-#ifdef	__masix__
-#define i_reserved1	osd1.masix1.m_i_reserved1
-#define i_frag		osd2.masix2.m_i_frag
-#define i_fsize		osd2.masix2.m_i_fsize
-#define i_reserved2	osd2.masix2.m_i_reserved2
-#endif
 
 /*
  * File system states
-- 
1.7.9.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH] ext2: No longer export ext2_fs.h to user space
@ 2012-03-23  9:27                 ` Thierry Reding
  0 siblings, 0 replies; 22+ messages in thread
From: Thierry Reding @ 2012-03-23  9:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Al Viro, Jan Kara, Ted Ts'o, Artem Bityutskiy,
	Andreas Dilger, linux-ext4

Since the on-disk format has been stable for quite some time, users
should either use the headers provided by libext2fs or keep a private
copy of this header. For the full discussion, see this thread:

	https://lkml.org/lkml/2012/3/21/516

While at it, this commit removes all __KERNEL__ guards, which are now
unnecessary.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Jan Kara <jack@suse.cz>
Cc: Ted Ts'o <tytso@mit.edu>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Andreas Dilger <aedilger@gmail.com>
Cc: linux-ext4@vger.kernel.org
---
 include/linux/Kbuild    |    1 -
 include/linux/ext2_fs.h |   72 +++++++----------------------------------------
 2 files changed, 10 insertions(+), 63 deletions(-)

diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index a255553..1cb0704 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -120,7 +120,6 @@ header-y += errno.h
 header-y += errqueue.h
 header-y += ethtool.h
 header-y += eventpoll.h
-header-y += ext2_fs.h
 header-y += fadvise.h
 header-y += falloc.h
 header-y += fanotify.h
diff --git a/include/linux/ext2_fs.h b/include/linux/ext2_fs.h
index ce1b719..f28dba5 100644
--- a/include/linux/ext2_fs.h
+++ b/include/linux/ext2_fs.h
@@ -20,6 +20,8 @@
 #include <linux/magic.h>
 #include <linux/fs.h>
 
+#include <linux/ext2_fs_sb.h>
+
 /*
  * The second extended filesystem constants/structures
  */
@@ -66,18 +68,10 @@
 /* First non-reserved inode for old ext2 filesystems */
 #define EXT2_GOOD_OLD_FIRST_INO	11
 
-#ifdef __KERNEL__
-#include <linux/ext2_fs_sb.h>
 static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
 {
 	return sb->s_fs_info;
 }
-#else
-/* Assume that user mode programs are passing in an ext2fs superblock, not
- * a kernel struct super_block.  This will allow us to call the feature-test
- * macros from user land. */
-#define EXT2_SB(sb)	(sb)
-#endif
 
 /*
  * Maximal count of links to a file
@@ -90,29 +84,12 @@ static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
 #define EXT2_MIN_BLOCK_SIZE		1024
 #define	EXT2_MAX_BLOCK_SIZE		4096
 #define EXT2_MIN_BLOCK_LOG_SIZE		  10
-#ifdef __KERNEL__
-# define EXT2_BLOCK_SIZE(s)		((s)->s_blocksize)
-#else
-# define EXT2_BLOCK_SIZE(s)		(EXT2_MIN_BLOCK_SIZE << (s)->s_log_block_size)
-#endif
+#define EXT2_BLOCK_SIZE(s)		((s)->s_blocksize)
 #define	EXT2_ADDR_PER_BLOCK(s)		(EXT2_BLOCK_SIZE(s) / sizeof (__u32))
-#ifdef __KERNEL__
-# define EXT2_BLOCK_SIZE_BITS(s)	((s)->s_blocksize_bits)
-#else
-# define EXT2_BLOCK_SIZE_BITS(s)	((s)->s_log_block_size + 10)
-#endif
-#ifdef __KERNEL__
+#define EXT2_BLOCK_SIZE_BITS(s)		((s)->s_blocksize_bits)
 #define	EXT2_ADDR_PER_BLOCK_BITS(s)	(EXT2_SB(s)->s_addr_per_block_bits)
 #define EXT2_INODE_SIZE(s)		(EXT2_SB(s)->s_inode_size)
 #define EXT2_FIRST_INO(s)		(EXT2_SB(s)->s_first_ino)
-#else
-#define EXT2_INODE_SIZE(s)	(((s)->s_rev_level == EXT2_GOOD_OLD_REV) ? \
-				 EXT2_GOOD_OLD_INODE_SIZE : \
-				 (s)->s_inode_size)
-#define EXT2_FIRST_INO(s)	(((s)->s_rev_level == EXT2_GOOD_OLD_REV) ? \
-				 EXT2_GOOD_OLD_FIRST_INO : \
-				 (s)->s_first_ino)
-#endif
 
 /*
  * Macro-instructions used to manage fragments
@@ -120,13 +97,8 @@ static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
 #define EXT2_MIN_FRAG_SIZE		1024
 #define	EXT2_MAX_FRAG_SIZE		4096
 #define EXT2_MIN_FRAG_LOG_SIZE		  10
-#ifdef __KERNEL__
-# define EXT2_FRAG_SIZE(s)		(EXT2_SB(s)->s_frag_size)
-# define EXT2_FRAGS_PER_BLOCK(s)	(EXT2_SB(s)->s_frags_per_block)
-#else
-# define EXT2_FRAG_SIZE(s)		(EXT2_MIN_FRAG_SIZE << (s)->s_log_frag_size)
-# define EXT2_FRAGS_PER_BLOCK(s)	(EXT2_BLOCK_SIZE(s) / EXT2_FRAG_SIZE(s))
-#endif
+#define EXT2_FRAG_SIZE(s)		(EXT2_SB(s)->s_frag_size)
+#define EXT2_FRAGS_PER_BLOCK(s)		(EXT2_SB(s)->s_frags_per_block)
 
 /*
  * Structure of a blocks group descriptor
@@ -146,16 +118,10 @@ struct ext2_group_desc
 /*
  * Macro-instructions used to manage group descriptors
  */
-#ifdef __KERNEL__
-# define EXT2_BLOCKS_PER_GROUP(s)	(EXT2_SB(s)->s_blocks_per_group)
-# define EXT2_DESC_PER_BLOCK(s)		(EXT2_SB(s)->s_desc_per_block)
-# define EXT2_INODES_PER_GROUP(s)	(EXT2_SB(s)->s_inodes_per_group)
-# define EXT2_DESC_PER_BLOCK_BITS(s)	(EXT2_SB(s)->s_desc_per_block_bits)
-#else
-# define EXT2_BLOCKS_PER_GROUP(s)	((s)->s_blocks_per_group)
-# define EXT2_DESC_PER_BLOCK(s)		(EXT2_BLOCK_SIZE(s) / sizeof (struct ext2_group_desc))
-# define EXT2_INODES_PER_GROUP(s)	((s)->s_inodes_per_group)
-#endif
+#define EXT2_BLOCKS_PER_GROUP(s)	(EXT2_SB(s)->s_blocks_per_group)
+#define EXT2_DESC_PER_BLOCK(s)		(EXT2_SB(s)->s_desc_per_block)
+#define EXT2_INODES_PER_GROUP(s)	(EXT2_SB(s)->s_inodes_per_group)
+#define EXT2_DESC_PER_BLOCK_BITS(s)	(EXT2_SB(s)->s_desc_per_block_bits)
 
 /*
  * Constants relative to the data blocks
@@ -296,7 +262,6 @@ struct ext2_inode {
 
 #define i_size_high	i_dir_acl
 
-#if defined(__KERNEL__) || defined(__linux__)
 #define i_reserved1	osd1.linux1.l_i_reserved1
 #define i_frag		osd2.linux2.l_i_frag
 #define i_fsize		osd2.linux2.l_i_fsize
@@ -305,23 +270,6 @@ struct ext2_inode {
 #define i_uid_high	osd2.linux2.l_i_uid_high
 #define i_gid_high	osd2.linux2.l_i_gid_high
 #define i_reserved2	osd2.linux2.l_i_reserved2
-#endif
-
-#ifdef	__hurd__
-#define i_translator	osd1.hurd1.h_i_translator
-#define i_frag		osd2.hurd2.h_i_frag
-#define i_fsize		osd2.hurd2.h_i_fsize
-#define i_uid_high	osd2.hurd2.h_i_uid_high
-#define i_gid_high	osd2.hurd2.h_i_gid_high
-#define i_author	osd2.hurd2.h_i_author
-#endif

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: No longer export ext2_fs.h to user space
  2012-03-23  9:27                 ` Thierry Reding
  (?)
@ 2012-03-23 13:13                 ` Ted Ts'o
  2012-03-23 17:55                   ` Al Viro
  -1 siblings, 1 reply; 22+ messages in thread
From: Ted Ts'o @ 2012-03-23 13:13 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-kernel, Al Viro, Jan Kara, Artem Bityutskiy,
	Andreas Dilger, linux-ext4

On Fri, Mar 23, 2012 at 10:27:36AM +0100, Thierry Reding wrote:
> Since the on-disk format has been stable for quite some time, users
> should either use the headers provided by libext2fs or keep a private
> copy of this header. For the full discussion, see this thread:
> 
> 	https://lkml.org/lkml/2012/3/21/516
> 
> While at it, this commit removes all __KERNEL__ guards, which are now
> unnecessary.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Ted Ts'o <tytso@mit.edu>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: Andreas Dilger <aedilger@gmail.com>
> Cc: linux-ext4@vger.kernel.org

Acked-by: "Theodore Ts'o" <tytso@mit.edu>

Thanks!!

Al, do you want to take this in the VFS tree, or do you want me to
carry this in the ext4 tree?  Or Jan could carry it in the ext2 tree.
I don't really have strong feelings about who picks it up.  I will if
no one else wants to...

					- Ted

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: No longer export ext2_fs.h to user space
  2012-03-23 13:13                 ` Ted Ts'o
@ 2012-03-23 17:55                   ` Al Viro
  2012-03-24 21:51                     ` Ted Ts'o
  0 siblings, 1 reply; 22+ messages in thread
From: Al Viro @ 2012-03-23 17:55 UTC (permalink / raw)
  To: Ted Ts'o, Thierry Reding, linux-kernel, Jan Kara,
	Artem Bityutskiy, Andreas Dilger, linux-ext4

On Fri, Mar 23, 2012 at 09:13:31AM -0400, Ted Ts'o wrote:
> On Fri, Mar 23, 2012 at 10:27:36AM +0100, Thierry Reding wrote:
> > Since the on-disk format has been stable for quite some time, users
> > should either use the headers provided by libext2fs or keep a private
> > copy of this header. For the full discussion, see this thread:
> > 
> > 	https://lkml.org/lkml/2012/3/21/516
> > 
> > While at it, this commit removes all __KERNEL__ guards, which are now
> > unnecessary.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Ted Ts'o <tytso@mit.edu>
> > Cc: Artem Bityutskiy <dedekind1@gmail.com>
> > Cc: Andreas Dilger <aedilger@gmail.com>
> > Cc: linux-ext4@vger.kernel.org
> 
> Acked-by: "Theodore Ts'o" <tytso@mit.edu>
> 
> Thanks!!
> 
> Al, do you want to take this in the VFS tree, or do you want me to
> carry this in the ext4 tree?  Or Jan could carry it in the ext2 tree.
> I don't really have strong feelings about who picks it up.  I will if
> no one else wants to...

Applied.  I'll probably add a followup moving most of that stuff to
fs/ext2/ext2.h on top of that commit...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-22 17:31           ` [PATCH] ext2: Don't export ext2_mask_flags() " Al Viro
@ 2012-03-24  6:50               ` Mike Frysinger
  2012-03-24  6:50               ` Mike Frysinger
  1 sibling, 0 replies; 22+ messages in thread
From: Mike Frysinger @ 2012-03-24  6:50 UTC (permalink / raw)
  To: Al Viro
  Cc: Ted Ts'o, Artem Bityutskiy, Thierry Reding, Jan Kara,
	linux-kernel, linux-ext4, stable

On Thu, Mar 22, 2012 at 13:31, Al Viro wrote:
> On Thu, Mar 22, 2012 at 12:28:25PM -0400, Ted Ts'o wrote:
>> On Thu, Mar 22, 2012 at 08:00:41AM +0200, Artem Bityutskiy wrote:
>> > On Thu, 2012-03-22 at 06:53 +0100, Thierry Reding wrote:
>> > > What's the recommended fix for packages that cannot or will not use
>> > > libext2fs, like busybox? Copy the required parts into a private header
>> > > and use that instead?
>> >
>> > The normal way is to just keep a private copy of the whole header file.
>> > Because the on-disk format stays compatible, those programs do not have
>> > to update the header very often - only rarely if they want to support
>> > some new feature.
>>
>> Even if they're not iwlling to use libext2fs (for space reasons, I
>> would assume?  It can't be because of license compatibility issues
>> since they are both GPLv2), they could just simply grab the ext2_fs.h
>> from e2fsprogs.  That has all of the file system definitions for ext2,
>> ext3, and ext4.
>
> Ho-hum...  Then we could kill a lot of lines in include/linux/ext2_fs.h.
> I wonder how much of what remains has any business being outside of
> fs/ext2, actually - AFAICS, there are very few places that might possibly
> care:
>
> arch/blackfin/kernel/setup.c:595:       if (*((unsigned short *)(mtd_phys + 0x438)) == EXT2_SUPER_MAGIC)
> fs/nfsd/nfs3proc.c:599:         case EXT2_SUPER_MAGIC:
> fs/nfsd/nfs3proc.c:600:                 resp->p_link_max = EXT2_LINK_MAX;
> fs/nfsd/nfs3proc.c:601:                 resp->p_name_max = EXT2_NAME_LEN;
> init/do_mounts_rd.c:57: struct ext2_super_block *ext2sb;
> init/do_mounts_rd.c:70: ext2sb = (struct ext2_super_block *) buf;
> init/do_mounts_rd.c:153:        if (ext2sb->s_magic == cpu_to_le16(EXT2_SUPER_MAGIC)) {
> init/do_mounts_rd.c:155:                       "RAMDISK: ext2 filesystem found at block %d\n",
> init/do_mounts_rd.c:157:                nblocks = le32_to_cpu(ext2sb->s_blocks_count) <<
> init/do_mounts_rd.c:158:                        le32_to_cpu(ext2sb->s_log_block_size);
> security/selinux/hooks.c:2974:  case EXT2_IOC_GETFLAGS:
> security/selinux/hooks.c:2976:  case EXT2_IOC_GETVERSION:
> security/selinux/hooks.c:2980:  case EXT2_IOC_SETFLAGS:
> security/selinux/hooks.c:2982:  case EXT2_IOC_SETVERSION:
>
> and that's it.  blackfin and do_mounts_rd are doing the same thing (blackfin -
> buggy, AFAICS).

buggy how ?  they're not exactly the same as the Blackfin code is
setting things up for the uClinux MTD map.  it isn't parsing the
filesystem itself (ignoring the size extraction from the superblock).

> Looks like both are  asking for something along the lines of
> sector_t detect_ext2(void *image), returning 0 if it's not one and size in
> kilobytes if it is...

yes, that would be fine
-mike

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
@ 2012-03-24  6:50               ` Mike Frysinger
  0 siblings, 0 replies; 22+ messages in thread
From: Mike Frysinger @ 2012-03-24  6:50 UTC (permalink / raw)
  To: Al Viro
  Cc: Ted Ts'o, Artem Bityutskiy, Thierry Reding, Jan Kara,
	linux-kernel, linux-ext4, stable

On Thu, Mar 22, 2012 at 13:31, Al Viro wrote:
> On Thu, Mar 22, 2012 at 12:28:25PM -0400, Ted Ts'o wrote:
>> On Thu, Mar 22, 2012 at 08:00:41AM +0200, Artem Bityutskiy wrote:
>> > On Thu, 2012-03-22 at 06:53 +0100, Thierry Reding wrote:
>> > > What's the recommended fix for packages that cannot or will not use
>> > > libext2fs, like busybox? Copy the required parts into a private header
>> > > and use that instead?
>> >
>> > The normal way is to just keep a private copy of the whole header file.
>> > Because the on-disk format stays compatible, those programs do not have
>> > to update the header very often - only rarely if they want to support
>> > some new feature.
>>
>> Even if they're not iwlling to use libext2fs (for space reasons, I
>> would assume?  It can't be because of license compatibility issues
>> since they are both GPLv2), they could just simply grab the ext2_fs.h
>> from e2fsprogs.  That has all of the file system definitions for ext2,
>> ext3, and ext4.
>
> Ho-hum...  Then we could kill a lot of lines in include/linux/ext2_fs.h.
> I wonder how much of what remains has any business being outside of
> fs/ext2, actually - AFAICS, there are very few places that might possibly
> care:
>
> arch/blackfin/kernel/setup.c:595:       if (*((unsigned short *)(mtd_phys + 0x438)) == EXT2_SUPER_MAGIC)
> fs/nfsd/nfs3proc.c:599:         case EXT2_SUPER_MAGIC:
> fs/nfsd/nfs3proc.c:600:                 resp->p_link_max = EXT2_LINK_MAX;
> fs/nfsd/nfs3proc.c:601:                 resp->p_name_max = EXT2_NAME_LEN;
> init/do_mounts_rd.c:57: struct ext2_super_block *ext2sb;
> init/do_mounts_rd.c:70: ext2sb = (struct ext2_super_block *) buf;
> init/do_mounts_rd.c:153:        if (ext2sb->s_magic == cpu_to_le16(EXT2_SUPER_MAGIC)) {
> init/do_mounts_rd.c:155:                       "RAMDISK: ext2 filesystem found at block %d\n",
> init/do_mounts_rd.c:157:                nblocks = le32_to_cpu(ext2sb->s_blocks_count) <<
> init/do_mounts_rd.c:158:                        le32_to_cpu(ext2sb->s_log_block_size);
> security/selinux/hooks.c:2974:  case EXT2_IOC_GETFLAGS:
> security/selinux/hooks.c:2976:  case EXT2_IOC_GETVERSION:
> security/selinux/hooks.c:2980:  case EXT2_IOC_SETFLAGS:
> security/selinux/hooks.c:2982:  case EXT2_IOC_SETVERSION:
>
> and that's it.  blackfin and do_mounts_rd are doing the same thing (blackfin -
> buggy, AFAICS).

buggy how ?  they're not exactly the same as the Blackfin code is
setting things up for the uClinux MTD map.  it isn't parsing the
filesystem itself (ignoring the size extraction from the superblock).

> Looks like both are  asking for something along the lines of
> sector_t detect_ext2(void *image), returning 0 if it's not one and size in
> kilobytes if it is...

yes, that would be fine
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-23  0:25             ` Andreas Dilger
@ 2012-03-24  6:51               ` Mike Frysinger
  0 siblings, 0 replies; 22+ messages in thread
From: Mike Frysinger @ 2012-03-24  6:51 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: Al Viro, Ted Ts'o, Artem Bityutskiy, ThierryReding, Jan Kara,
	linux-kernel, linux-ext4, stable

On Thu, Mar 22, 2012 at 20:25, Andreas Dilger wrote:
> On 2012-03-22, at 11:31, Al Viro wrote:
>> On Thu, Mar 22, 2012 at 12:28:25PM -0400, Ted Ts'o wrote:
>>> Even if they're not iwlling to use libext2fs (for space reasons, I
>>> would assume?  It can't be because of license compatibility issues
>>> since they are both GPLv2), they could just simply grab the ext2_fs.h
>>> from e2fsprogs.  That has all of the file system definitions for ext2,
>>> ext3, and ext4.
>>
>> Ho-hum...  Then we could kill a lot of lines in include/linux/ext2_fs.h.
>> I wonder how much of what remains has any business being outside of
>> fs/ext2, actually - AFAICS, there are very few places that might possibly
>> care:
>>
>> arch/blackfin/kernel/setup.c:595:       if (*((unsigned short *)(mtd_phys + 0x438)) == EXT2_SUPER_MAGIC)
>> fs/nfsd/nfs3proc.c:599:         case EXT2_SUPER_MAGIC:
>> fs/nfsd/nfs3proc.c:600:                 resp->p_link_max = EXT2_LINK_MAX;
>> fs/nfsd/nfs3proc.c:601:                 resp->p_name_max = EXT2_NAME_LEN;
>
> These don't really make sense to be using ext2 constants.

i'm guessing your quoting is over zealous and you're not actually
talking about the Blackfin code here ... just the nfsd code
-mike

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-24  6:50               ` Mike Frysinger
  (?)
@ 2012-03-24  8:37               ` Al Viro
  2012-03-24 17:59                 ` Mike Frysinger
  -1 siblings, 1 reply; 22+ messages in thread
From: Al Viro @ 2012-03-24  8:37 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: Ted Ts'o, Artem Bityutskiy, Thierry Reding, Jan Kara,
	linux-kernel, linux-ext4, stable

On Sat, Mar 24, 2012 at 02:50:38AM -0400, Mike Frysinger wrote:

> > and that's it. ??blackfin and do_mounts_rd are doing the same thing (blackfin -
> > buggy, AFAICS).
> 
> buggy how ?  they're not exactly the same as the Blackfin code is
> setting things up for the uClinux MTD map.  it isn't parsing the
> filesystem itself (ignoring the size extraction from the superblock).

Compare and you'll see...  Block size on ext2 may be bigger than 1Kb;
->s_log_block_size gives the shift (1Kb -> 0, 2Kb -> 1, etc.)  Offset
0x18 in superblock, __le32...

Anyway, I've pushed that into vfs.git#master along with other minor stuff
right now; the last 5 commits in there are handling of ext2 and ext3 headers,
starting with "ext2: No longer export ext2_fs.h to user space" by
Thierry Reding, with the next 3 after it doing the move and trimming...
Should propagate to git.kernel.org in a few.

I've left the nfsd mess alone for now; it's too ugly to live, but I don't
want to trigger the bikeshedding from hell that happens every time somebody
brings pathconf() up.  Not worth bothering with, just to move two macros to
fs/ext2/ext2.h where they really belong...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: Don't export ext2_mask_flags() to user space
  2012-03-24  8:37               ` Al Viro
@ 2012-03-24 17:59                 ` Mike Frysinger
  0 siblings, 0 replies; 22+ messages in thread
From: Mike Frysinger @ 2012-03-24 17:59 UTC (permalink / raw)
  To: Al Viro
  Cc: Ted Ts'o, Artem Bityutskiy, Thierry Reding, Jan Kara,
	linux-kernel, linux-ext4, stable

[-- Attachment #1: Type: Text/Plain, Size: 1189 bytes --]

On Saturday 24 March 2012 04:37:56 Al Viro wrote:
> On Sat, Mar 24, 2012 at 02:50:38AM -0400, Mike Frysinger wrote:
> > > and that's it. ??blackfin and do_mounts_rd are doing the same thing
> > > (blackfin - buggy, AFAICS).
> > 
> > buggy how ?  they're not exactly the same as the Blackfin code is
> > setting things up for the uClinux MTD map.  it isn't parsing the
> > filesystem itself (ignoring the size extraction from the superblock).
> 
> Compare and you'll see...  Block size on ext2 may be bigger than 1Kb;
> ->s_log_block_size gives the shift (1Kb -> 0, 2Kb -> 1, etc.)  Offset
> 0x18 in superblock, __le32...

fair enough.  the users of the Blackfin code should generally be OK though due 
to the constrained inputs and runtime systems.

> Anyway, I've pushed that into vfs.git#master along with other minor stuff
> right now; the last 5 commits in there are handling of ext2 and ext3
> headers, starting with "ext2: No longer export ext2_fs.h to user space" by
> Thierry Reding, with the next 3 after it doing the move and trimming...
> Should propagate to git.kernel.org in a few.

looks good to me
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: No longer export ext2_fs.h to user space
  2012-03-23 17:55                   ` Al Viro
@ 2012-03-24 21:51                     ` Ted Ts'o
  2012-03-24 22:16                       ` H. Peter Anvin
  0 siblings, 1 reply; 22+ messages in thread
From: Ted Ts'o @ 2012-03-24 21:51 UTC (permalink / raw)
  To: hpa
  Cc: Al Viro, Thierry Reding, linux-kernel, Jan Kara,
	Artem Bityutskiy, Andreas Dilger, linux-ext4

Peter,

Note that we are planning on unexporting ext2_fs.h, and then utterly
removing it from the tree (with what fs/ext2 needs being moved to
fs/ext2/ext2.h).

Someone who was compiling "the latest version" of syslinux has
submitted a kernel bugzilla entry that this was breaking due to
ext2_fs.h using umode_t which was no longer being exported by the
kernel header files.  Given that ext2_fs.h is about to go ***poof***
(Al already has the changes in his vfs git tree), I thought I should
give you a heads up....

					- Ted

On Fri, Mar 23, 2012 at 05:55:34PM +0000, Al Viro wrote:
> On Fri, Mar 23, 2012 at 09:13:31AM -0400, Ted Ts'o wrote:
> > On Fri, Mar 23, 2012 at 10:27:36AM +0100, Thierry Reding wrote:
> > > Since the on-disk format has been stable for quite some time, users
> > > should either use the headers provided by libext2fs or keep a private
> > > copy of this header. For the full discussion, see this thread:
> > > 
> > > 	https://lkml.org/lkml/2012/3/21/516
> > > 
> > > While at it, this commit removes all __KERNEL__ guards, which are now
> > > unnecessary.
> > > 
> > > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > > Cc: Jan Kara <jack@suse.cz>
> > > Cc: Ted Ts'o <tytso@mit.edu>
> > > Cc: Artem Bityutskiy <dedekind1@gmail.com>
> > > Cc: Andreas Dilger <aedilger@gmail.com>
> > > Cc: linux-ext4@vger.kernel.org
> > 
> > Acked-by: "Theodore Ts'o" <tytso@mit.edu>
> > 
> > Thanks!!
> > 
> > Al, do you want to take this in the VFS tree, or do you want me to
> > carry this in the ext4 tree?  Or Jan could carry it in the ext2 tree.
> > I don't really have strong feelings about who picks it up.  I will if
> > no one else wants to...
> 
> Applied.  I'll probably add a followup moving most of that stuff to
> fs/ext2/ext2.h on top of that commit...
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: No longer export ext2_fs.h to user space
  2012-03-24 21:51                     ` Ted Ts'o
@ 2012-03-24 22:16                       ` H. Peter Anvin
  2012-03-25  1:16                         ` Ted Ts'o
  0 siblings, 1 reply; 22+ messages in thread
From: H. Peter Anvin @ 2012-03-24 22:16 UTC (permalink / raw)
  To: Ted Ts'o, Al Viro, Thierry Reding, linux-kernel, Jan Kara,
	Artem Bityutskiy, Andreas Dilger, linux-ext4

On 03/24/2012 02:51 PM, Ted Ts'o wrote:
> Peter,
> 
> Note that we are planning on unexporting ext2_fs.h, and then utterly
> removing it from the tree (with what fs/ext2 needs being moved to
> fs/ext2/ext2.h).
> 
> Someone who was compiling "the latest version" of syslinux has
> submitted a kernel bugzilla entry that this was breaking due to
> ext2_fs.h using umode_t which was no longer being exported by the
> kernel header files.  Given that ext2_fs.h is about to go ***poof***
> (Al already has the changes in his vfs git tree), I thought I should
> give you a heads up....
> 

Thanks.  Syslinux already includes a private copy of ext2_fs.h, but I
need to scrub the umode_t inclusion.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] ext2: No longer export ext2_fs.h to user space
  2012-03-24 22:16                       ` H. Peter Anvin
@ 2012-03-25  1:16                         ` Ted Ts'o
  0 siblings, 0 replies; 22+ messages in thread
From: Ted Ts'o @ 2012-03-25  1:16 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Al Viro, Thierry Reding, linux-kernel, Jan Kara,
	Artem Bityutskiy, Andreas Dilger, linux-ext4

On Sat, Mar 24, 2012 at 03:16:37PM -0700, H. Peter Anvin wrote:
> > Someone who was compiling "the latest version" of syslinux has
> > submitted a kernel bugzilla entry that this was breaking due to
> > ext2_fs.h using umode_t which was no longer being exported by the
> > kernel header files.  Given that ext2_fs.h is about to go ***poof***
> > (Al already has the changes in his vfs git tree), I thought I should
> > give you a heads up....
> > 
> 
> Thanks.  Syslinux already includes a private copy of ext2_fs.h, but I
> need to scrub the umode_t inclusion.

Ah, good.  I'll close the kernel bugzilla report in that case.

Thanks,

    	   	   	      	  	 - Ted


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2012-03-25  1:16 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-21 21:50 [PATCH] ext2: Don't export ext2_mask_flags() to user space Thierry Reding
2012-03-21 22:23 ` Jan Kara
2012-03-21 23:16   ` Ted Ts'o
2012-03-22  5:53     ` Thierry Reding
2012-03-22  6:00       ` Artem Bityutskiy
2012-03-22 16:28         ` Ted Ts'o
2012-03-22 16:47           ` Thierry Reding
2012-03-22 18:31             ` Ted Ts'o
2012-03-23  9:27               ` [PATCH] ext2: No longer export ext2_fs.h " Thierry Reding
2012-03-23  9:27                 ` Thierry Reding
2012-03-23 13:13                 ` Ted Ts'o
2012-03-23 17:55                   ` Al Viro
2012-03-24 21:51                     ` Ted Ts'o
2012-03-24 22:16                       ` H. Peter Anvin
2012-03-25  1:16                         ` Ted Ts'o
2012-03-22 17:31           ` [PATCH] ext2: Don't export ext2_mask_flags() " Al Viro
2012-03-23  0:25             ` Andreas Dilger
2012-03-24  6:51               ` Mike Frysinger
2012-03-24  6:50             ` Mike Frysinger
2012-03-24  6:50               ` Mike Frysinger
2012-03-24  8:37               ` Al Viro
2012-03-24 17:59                 ` Mike Frysinger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.