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 <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
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
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 <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Jan Kara <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> 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 <[email protected]>
SUSE Labs, CR
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
* 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
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
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
* 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
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*
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
On 2012-03-22, at 11:31, Al Viro <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Ted Ts'o <[email protected]>
Cc: Artem Bityutskiy <[email protected]>
Cc: Andreas Dilger <[email protected]>
Cc: [email protected]
---
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
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 <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Jan Kara <[email protected]>
> Cc: Ted Ts'o <[email protected]>
> Cc: Artem Bityutskiy <[email protected]>
> Cc: Andreas Dilger <[email protected]>
> Cc: [email protected]
Acked-by: "Theodore Ts'o" <[email protected]>
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
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 <[email protected]>
> > Cc: Al Viro <[email protected]>
> > Cc: Jan Kara <[email protected]>
> > Cc: Ted Ts'o <[email protected]>
> > Cc: Artem Bityutskiy <[email protected]>
> > Cc: Andreas Dilger <[email protected]>
> > Cc: [email protected]
>
> Acked-by: "Theodore Ts'o" <[email protected]>
>
> 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...
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
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
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...
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 <[email protected]>
-mike
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 <[email protected]>
> > > Cc: Al Viro <[email protected]>
> > > Cc: Jan Kara <[email protected]>
> > > Cc: Ted Ts'o <[email protected]>
> > > Cc: Artem Bityutskiy <[email protected]>
> > > Cc: Andreas Dilger <[email protected]>
> > > Cc: [email protected]
> >
> > Acked-by: "Theodore Ts'o" <[email protected]>
> >
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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