| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 19 Jun 2012 15:48:10 +1000
From: Tony Breeds <tony@...eyournoodle.com>
To: Ted Ts'o <tytso@....edu>
Cc: linux-ext4@...r.kernel.org
Subject: Re: Minimal configuration for e2fsprogs
On Mon, Jun 18, 2012 at 01:12:57PM -0400, Ted Ts'o wrote:
> On Mon, Jun 18, 2012 at 03:58:42PM +1000, Tony Breeds wrote:
> >
> > I'm not certain we're on the same page. We're linking statically so
> > that fact we don't call the progress functions doesn't matter. The
> > code is in libext2fs.a and there must be a call path from (eg)
> > ext2fs_open() to fwrite(stderr, ...). The fact we don't add the
> > EXT2_FLAG_PRINT_PROGRESS doesn't come into it does it?
>
> Oh, I see, the problem is that ext2fs_closefs() is calling the
> progress functions; I had forgotten about it. Yeah, that's something
> I can fix so that the progress functions are only dragged in if mke2fs
> explicitly requests it, via adding function which enables the progress
> functions, instead of just setting the EXT2_FLAG_PRINT_PROGRESS.
Okay that sounds good to me. If I get the other bits implemented
before you get to it. I'm happy to take a run at it.
> So a couple of things; it's a really bad to define static functions in
> a header file such as ext2fs.h. Yes, gcc will normally optimize out
> functions which aren't used, but it will also complain with warnings
> if you enable them.
Thanks for the pointers. I had meant to make them static inline I just
forgot the inline.
> Instead, what I would do is to simply take out the calls to the
> ext2fs_mmp library functions in e2fsck and tune2fs, and remove the MMP
> feature flag from the list of supported features. That way, if
> someone tries to use the e2fsck binary compiled with --disable-mmp on
> a file system with MMP enabled, e2fsck will refuse to touch the file
> system saying that it doesn't support the feature. If you just make
> the mmp functions silently succeed (by returning 0), that's really bad
> since file system damage could result.
Okay that all make sense I've had a try but I think it's pretty brutal.
I'm sure I could finesse it a bit but I have a couple of questions:
1) The only way I could see to remove MMP from the supported features
was with something like:
#ifdef CONFIG_MMP
#define EXT2_LIB_FEATURE_INCOMPAT_SUPP (...|EXT4_FEATURE_INCOMPAT_MMP|...)
#else
#define EXT2_LIB_FEATURE_INCOMPAT_SUPP (...)
#endif
But EXT2_LIB_FEATURE_INCOMPAT_SUPP is already #defined twice depending
on the state of ENABLE_COMPRESSION. So going down the path above would
mean #defining it 4 times which I'd think is starting to indicate there
must be a better way. Firstly am I in the right ball park, secondly
Would something like
#ifdef ENABLE_COMPRESSION
...
#define _EXT4_FEATURE_INCOMPAT_COMPRESSION EXT4_FEATURE_INCOMPAT_COMPRESSION
#else
#define _EXT4_FEATURE_INCOMPAT_COMPRESSION (0)
#endif
#ifdef ENABLE_MMP
...
#define _EXT4_FEATURE_INCOMPAT_MMP EXT4_FEATURE_INCOMPAT_MMP
#else
#define _EXT4_FEATURE_INCOMPAT_MMP (0)
#endif
#define EXT2_LIB_FEATURE_INCOMPAT_SUPP (...\
_EXT2_FEATURE_INCOMPAT_COMPRESSION|\
... \
_EXT4_FEATURE_INCOMPAT_MMP|\
...)
Be acceptable?
2) in debugfs you have a command table (debug_cmds.ct or similar) there
doesn't seem to be a way to exclude commands based on the state of
CONFIG_MMP. Is it a problem the expose a debugfs command that will do
nothing when built with --disable-mmp, or should I look at extending
the command table generator to support the conditionals?
I'll add my current patch after my .signature
> I already have things set up so that if you don't actually call the
> functions to read in the bitmaps, the functions to read and write the
> bitmaps don't get dragged into a static library link. That's what I'm
> referring to.
Ahh thanks for the clarification.
> The functions to actually create in-memory bitmaps for various
> housekeeping things, do need to get dragged in, but we don't
> necessarily need to drag in all of the different bitmap back-ends. If
> we assume that yaboot doesn't need to optimize for low-memory usage
> for really, really big file systems (i.e., you're not going to try to
> allocate files or blocks while opening a multi-terrabyte file system
> using libext2fs on a system with only 512mb), then you wouldn't need
> blkmap64_rb.c.
Right, we're operating in a pretty tight memory environment (typically
128MB or 256MB) but we're also usually opening file-systems <100GB
usually closer to 200MB.
> And the bitmap statistics functions are sometihng we can use a
> configure option to disable, which should remove the last bits of
> stdio usage I believe.
Okay. If I can get the disable-mmp patch into a state you like it I'll
tackle that configure option as well.
> If you'd like to try to clean up the --disable-mmp patch, and perhaps
> try your hand at a --disable-libext2fs-stats patch which disable the
> statistics options, that would be great. Otherwise, I'll try to get
> to it in the next few weeks.
I really appreciate you help and time, and hope that continually
pointing me in the right direction isn't too taxing on you.
Yours Tony
---
configure.in | 21 +++++++++++++++++++++
debugfs/debugfs.c | 2 ++
debugfs/set_fields.c | 9 +++++++++
e2fsck/journal.c | 2 ++
e2fsck/unix.c | 4 ++++
e2fsck/util.c | 6 ++++++
lib/config.h.in | 3 +++
lib/ext2fs/Makefile.in | 4 ++--
lib/ext2fs/closefs.c | 2 ++
lib/ext2fs/ext2fs.h | 2 ++
lib/ext2fs/openfs.c | 2 ++
misc/mke2fs.c | 2 ++
misc/tune2fs.c | 6 ++++++
13 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/configure.in b/configure.in
index 7373e8e..4bacd1a 100644
--- a/configure.in
+++ b/configure.in
@@ -746,6 +746,27 @@ AC_MSG_RESULT([Building uuidd by default])
)
AC_SUBST(UUIDD_CMT)
dnl
+dnl handle --disable-mmp
+dnl
+AH_TEMPLATE([CONFIG_MMP], [Define to 1 to enable mmp support])
+AC_ARG_ENABLE([mmp],
+[ --disable-mmp disable support mmp, Multi Mount Protection],
+if test "$enableval" = "no"
+then
+ AC_MSG_RESULT([Disabling mmp support])
+ MMP_CMT="#"
+else
+ AC_MSG_RESULT([Enabling mmp support])
+ AC_DEFINE(CONFIG_MMP, 1)
+ MMP_CMT=
+fi
+,
+AC_MSG_RESULT([Enabling mmp support by default])
+AC_DEFINE(CONFIG_MMP, 1)
+MMP_CMT=
+)
+AC_SUBST(MMP_CMT)
+dnl
dnl
dnl
MAKEFILE_LIBRARY=$srcdir/lib/Makefile.library
diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index cf80bd0..5df915e 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -2194,6 +2194,7 @@ void do_punch(int argc, char *argv[])
void do_dump_mmp(int argc EXT2FS_ATTR((unused)), char *argv[])
{
+#ifdef CONFIG_MMP
struct ext2_super_block *sb;
struct mmp_struct *mmp_s;
time_t t;
@@ -2237,6 +2238,7 @@ void do_dump_mmp(int argc EXT2FS_ATTR((unused)), char *argv[])
fprintf(stdout, "node_name: %s\n", mmp_s->mmp_nodename);
fprintf(stdout, "device_name: %s\n", mmp_s->mmp_bdevname);
fprintf(stdout, "magic: 0x%x\n", mmp_s->mmp_magic);
+#endif
}
static int source_file(const char *cmd_file, int sci_idx)
diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
index 08bfd8d..77fbbc1 100644
--- a/debugfs/set_fields.c
+++ b/debugfs/set_fields.c
@@ -69,8 +69,10 @@ static errcode_t parse_hashalg(struct field_set_info *info, char *field, char *a
static errcode_t parse_time(struct field_set_info *info, char *field, char *arg);
static errcode_t parse_bmap(struct field_set_info *info, char *field, char *arg);
static errcode_t parse_gd_csum(struct field_set_info *info, char *field, char *arg);
+#ifdef CONFIG_MMP
static errcode_t parse_mmp_clear(struct field_set_info *info, char *field,
char *arg);
+#endif
static struct field_set_info super_fields[] = {
{ "inodes_count", &set_sb.s_inodes_count, NULL, 4, parse_uint },
@@ -246,6 +248,7 @@ static struct field_set_info ext4_bg_fields[] = {
};
static struct field_set_info mmp_fields[] = {
+#ifdef CONFIG_MMP
{ "clear", &set_mmp.mmp_magic, NULL, sizeof(set_mmp), parse_mmp_clear },
{ "magic", &set_mmp.mmp_magic, NULL, 4, parse_uint },
{ "seq", &set_mmp.mmp_seq, NULL, 4, parse_uint },
@@ -255,6 +258,8 @@ static struct field_set_info mmp_fields[] = {
{ "bdevname", &set_mmp.mmp_bdevname, NULL, sizeof(set_mmp.mmp_bdevname),
parse_string },
{ "check_interval", &set_mmp.mmp_check_interval, NULL, 2, parse_uint },
+#endif
+ { 0, 0, 0, 0 }
};
static int check_suffix(const char *field)
@@ -748,6 +753,7 @@ void do_set_block_group_descriptor(int argc, char *argv[])
}
}
+#ifdef CONFIG
static errcode_t parse_mmp_clear(struct field_set_info *info,
char *field EXT2FS_ATTR((unused)),
char *arg EXT2FS_ATTR((unused)))
@@ -762,9 +768,11 @@ static errcode_t parse_mmp_clear(struct field_set_info *info,
return 1; /* we don't need the MMP block written again */
}
+#endif
void do_set_mmp_value(int argc, char *argv[])
{
+#ifdef CONFIG_MMP
const char *usage = "<field> <value>\n"
"\t\"set_mmp_value -l\" will list the names of "
"MMP fields\n\twhich can be set.";
@@ -819,5 +827,6 @@ void do_set_mmp_value(int argc, char *argv[])
&set_mmp);
*mmp_s = set_mmp;
}
+#endif
}
diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index 767ea10..e5ee4ed 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -889,7 +889,9 @@ int e2fsck_run_ext3_journal(e2fsck_t ctx)
if (stats && stats->bytes_written)
kbytes_written = stats->bytes_written >> 10;
+#ifdef CONFIG_MMP
ext2fs_mmp_stop(ctx->fs);
+#endif
ext2fs_free(ctx->fs);
retval = ext2fs_open(ctx->filesystem_name, EXT2_FLAG_RW,
ctx->superblock, blocksize, io_ptr,
diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 94260bd..6cca9e0 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -1042,6 +1042,7 @@ static const char *my_ver_date = E2FSPROGS_DATE;
static int e2fsck_check_mmp(ext2_filsys fs, e2fsck_t ctx)
{
+#ifdef CONFIG_MMP
struct mmp_struct *mmp_s;
unsigned int mmp_check_interval;
errcode_t retval = 0;
@@ -1120,6 +1121,9 @@ check_error:
}
}
return retval;
+#else
+ return 0;
+#endif
}
int main (int argc, char *argv[])
diff --git a/e2fsck/util.c b/e2fsck/util.c
index 7c4caab..5b9b154 100644
--- a/e2fsck/util.c
+++ b/e2fsck/util.c
@@ -56,7 +56,9 @@ void fatal_error(e2fsck_t ctx, const char *msg)
if (!fs)
goto out;
if (fs->io) {
+#ifdef CONFIG_MMP
ext2fs_mmp_stop(ctx->fs);
+#endif
if (ctx->fs->io->magic == EXT2_ET_MAGIC_IO_CHANNEL)
io_channel_flush(ctx->fs->io);
else
@@ -780,6 +782,7 @@ void dump_mmp_msg(struct mmp_struct *mmp, const char *msg)
errcode_t e2fsck_mmp_update(ext2_filsys fs)
{
+#ifdef CONFIF_MMP
errcode_t retval;
retval = ext2fs_mmp_update(fs);
@@ -789,6 +792,9 @@ errcode_t e2fsck_mmp_update(ext2_filsys fs)
"being modified while fsck is running.\n"));
return retval;
+#else
+ return 0;
+#endif
}
void e2fsck_set_bitmap_type(ext2_filsys fs, unsigned int default_type,
diff --git a/lib/config.h.in b/lib/config.h.in
index 90e9743..52c3897 100644
--- a/lib/config.h.in
+++ b/lib/config.h.in
@@ -12,6 +12,9 @@
/* Define to 1 if debugging ext3/4 journal code */
#undef CONFIG_JBD_DEBUG
+/* Define to 1 to enable mmp support */
+#undef CONFIG_MMP
+
/* Define to 1 to enable quota support */
#undef CONFIG_QUOTA
diff --git a/lib/ext2fs/Makefile.in b/lib/ext2fs/Makefile.in
index 0d9ac21..5b73958 100644
--- a/lib/ext2fs/Makefile.in
+++ b/lib/ext2fs/Makefile.in
@@ -14,9 +14,10 @@ MK_CMDS= _SS_DIR_OVERRIDE=../ss ../ss/mk_cmds
@RESIZER_CMT@...IZE_LIB_OBJS = dupfs.o
@TEST_IO_CMT@...T_IO_LIB_OBJS = test_io.o
@IMAGER_CMT@...MAGE_LIB_OBJS = imager.o
+@..._CMT@..._OBJS = mmp.o
OBJS= $(DEBUGFS_LIB_OBJS) $(RESIZE_LIB_OBJS) $(E2IMAGE_LIB_OBJS) \
- $(TEST_IO_LIB_OBJS) \
+ $(TEST_IO_LIB_OBJS) $(MMP_OBJS) \
ext2_err.o \
alloc.o \
alloc_sb.o \
@@ -66,7 +67,6 @@ OBJS= $(DEBUGFS_LIB_OBJS) $(RESIZE_LIB_OBJS) $(E2IMAGE_LIB_OBJS) \
lookup.o \
mkdir.o \
mkjournal.o \
- mmp.o \
namei.o \
native.o \
newdir.o \
diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c
index 973c2a2..abf77cc 100644
--- a/lib/ext2fs/closefs.c
+++ b/lib/ext2fs/closefs.c
@@ -464,9 +464,11 @@ errcode_t ext2fs_close2(ext2_filsys fs, int flags)
return retval;
}
+#ifdef CONFIG_MMP
retval = ext2fs_mmp_stop(fs);
if (retval)
return retval;
+#endif
ext2fs_free(fs);
return 0;
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index ff088bb..fc83973 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -1366,6 +1366,7 @@ errcode_t ext2fs_unlink(ext2_filsys fs, ext2_ino_t dir, const char *name,
ext2_ino_t ino, int flags);
/* mmp.c */
+#ifdef CONFIG_MMP
errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf);
errcode_t ext2fs_mmp_write(ext2_filsys fs, blk64_t mmp_blk, void *buf);
errcode_t ext2fs_mmp_clear(ext2_filsys fs);
@@ -1374,6 +1375,7 @@ errcode_t ext2fs_mmp_start(ext2_filsys fs);
errcode_t ext2fs_mmp_update(ext2_filsys fs);
errcode_t ext2fs_mmp_stop(ext2_filsys fs);
unsigned ext2fs_mmp_new_seq(void);
+#endif /* CONFIG_MMP */
/* read_bb.c */
extern errcode_t ext2fs_read_bb_inode(ext2_filsys fs,
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 482e4ab..88fdd4d 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -391,6 +391,7 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
fs->flags &= ~EXT2_FLAG_NOFREE_ON_ERROR;
*ret_fs = fs;
+#ifdef CONFIG_MMP
if ((fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_MMP) &&
!(flags & EXT2_FLAG_SKIP_MMP) &&
(flags & (EXT2_FLAG_RW | EXT2_FLAG_EXCLUSIVE))) {
@@ -401,6 +402,7 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
goto cleanup;
}
}
+#endif
return 0;
cleanup:
diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 7ec8cc2..e9b4c1f 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -2557,6 +2557,7 @@ int main (int argc, char *argv[])
printf(_("done\n"));
}
no_journal:
+#ifdef CONFIG_MMP
if (!super_only &&
fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_MMP) {
retval = ext2fs_mmp_init(fs);
@@ -2570,6 +2571,7 @@ no_journal:
"with update interval %d seconds.\n"),
fs->super->s_mmp_update_interval);
}
+#endif
if (EXT2_HAS_RO_COMPAT_FEATURE(&fs_param,
EXT4_FEATURE_RO_COMPAT_BIGALLOC))
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index f1f0bcf..2c4b20b 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -430,6 +430,7 @@ static int update_feature_set(ext2_filsys fs, char *features)
return 1;
}
}
+#ifdef CONFIG_MMP
if (FEATURE_ON(E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_MMP)) {
int error;
@@ -495,6 +496,7 @@ mmp_error:
sb->s_mmp_block = 0;
sb->s_mmp_update_interval = 0;
}
+#endif
if (FEATURE_ON(E2P_FEATURE_COMPAT, EXT3_FEATURE_COMPAT_HAS_JOURNAL)) {
/*
@@ -2105,10 +2107,12 @@ retry_open:
goto closefs;
}
}
+#ifdef CONFIG_MMP
if (clear_mmp) {
rc = ext2fs_mmp_clear(fs);
goto closefs;
}
+#endif
if (journal_size || journal_device) {
rc = add_journal(fs);
if (rc)
@@ -2219,7 +2223,9 @@ retry_open:
closefs:
if (rc) {
+#ifdef CONFIG_MMP
ext2fs_mmp_stop(fs);
+#endif
exit(1);
}
--
1.7.6.4
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists