All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [uPATCH] refuse plain ufs mount
@ 2004-01-28 22:24 Example
  2004-01-29  0:13 ` GOTO Masanori
  0 siblings, 1 reply; 9+ messages in thread
From: Example @ 2004-01-28 22:24 UTC (permalink / raw)
  To: GOTO Masanori, linux-kernel

Hi,

There's a semantic change introduced by this patch.
I don't know enough about UFS to call it a bug, but it
certainly looks suspicious.

 >--- fs/ufs/super.c.org	2003-10-20 12:50:24.000000000 +0900
 >+++ fs/ufs/super.c	2004-01-27 13:26:05.000000000 +0900
 >@@ -516,7 +516,7 @@
 > 		printk("wrong mount options\n");
 > 		goto failed;
 > 	}
 >-	if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
 >+	if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE) && !silent) {
 > 		printk("You didn't specify the type of your ufs filesystem\n\n"
 > 		"mount -t ufs -o ufstype="
 > 		"sun|sunx86|44bsd|old|hp|nextstep|netxstep-cd|openstep ...\n\n"
 >@@ -575,7 +575,7 @@
 > 		uspi->s_sbsize = super_block_size = 2048;
 > 		uspi->s_sbbase = 0;
 > 		flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
 >-		if (!(sb->s_flags & MS_RDONLY)) {
 >+		if (!(sb->s_flags & MS_RDONLY) && !silent) {
 > 			printk(KERN_INFO "ufstype=old is supported read-only\n");
 > 			sb->s_flags |= MS_RDONLY;

If "silent" is set, this variable assignment is skipped.  I
would have thought that the assignment should happen, and only
the printk() should be skipped.

 > 		}
 >@@ -589,7 +589,7 @@
 > 		uspi->s_sbsize = super_block_size = 2048;
 > 		uspi->s_sbbase = 0;
 > 		flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
 >-		if (!(sb->s_flags & MS_RDONLY)) {
 >+		if (!(sb->s_flags & MS_RDONLY) && !silent) {
 > 			printk(KERN_INFO "ufstype=nextstep is supported read-only\n");
 > 			sb->s_flags |= MS_RDONLY;

Same here.

 > 		}
 >@@ -603,7 +603,7 @@
 > 		uspi->s_sbsize = super_block_size = 2048;
 > 		uspi->s_sbbase = 0;
 > 		flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
 >-		if (!(sb->s_flags & MS_RDONLY)) {
 >+		if (!(sb->s_flags & MS_RDONLY) && !silent) {
 > 			printk(KERN_INFO "ufstype=nextstep-cd is supported read-only\n");
 > 			sb->s_flags |= MS_RDONLY;

And again.

 > 		}
 >@@ -617,7 +617,7 @@
 > 		uspi->s_sbsize = super_block_size = 2048;
 > 		uspi->s_sbbase = 0;
 > 		flags |= UFS_DE_44BSD | UFS_UID_44BSD | UFS_ST_44BSD | UFS_CG_44BSD;
 >-		if (!(sb->s_flags & MS_RDONLY)) {
 >+		if (!(sb->s_flags & MS_RDONLY) && !silent) {
 > 			printk(KERN_INFO "ufstype=openstep is supported read-only\n");
 > 			sb->s_flags |= MS_RDONLY;

Another one.

 > 		}
 >@@ -631,13 +631,14 @@
 > 		uspi->s_sbsize = super_block_size = 2048;
 > 		uspi->s_sbbase = 0;
 > 		flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
 >-		if (!(sb->s_flags & MS_RDONLY)) {
 >+		if (!(sb->s_flags & MS_RDONLY) && !silent) {
 > 			printk(KERN_INFO "ufstype=hp is supported read-only\n");
 > 			sb->s_flags |= MS_RDONLY;

And again.

 >  		}
 >  		break;
 > 	default:
 >-		printk("unknown ufstype\n");
 >+		if (!silent)
 >+			printk("unknown ufstype\n");
 > 		goto failed;
 > 	}
 > 	
 >@@ -687,7 +688,8 @@
 > 		uspi->s_sbbase += 8;
 > 		goto again;
 > 	}
 >-	printk("ufs_read_super: bad magic number\n");
 >+	if (!silent)
 >+		printk("ufs_read_super: bad magic number\n");
 > 	goto failed;
 >
 > magic_found:


Kind regards,

Jon

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [uPATCH] refuse plain ufs mount
@ 2004-01-27  4:07 Andries.Brouwer
  2004-01-27  5:04 ` GOTO Masanori
  2004-01-27  6:15 ` Linus Torvalds
  0 siblings, 2 replies; 9+ messages in thread
From: Andries.Brouwer @ 2004-01-27  4:07 UTC (permalink / raw)
  To: Andries.Brouwer, torvalds; +Cc: akpm, gotom, linux-kernel

    From: Linus Torvalds <torvalds@osdl.org>

    > But you see, it wasn't the user at all, and it wasn't a ufs filesystem.
    > It is kernel probing that causes error messages. That is unwanted.
    > So, your version is wrong.

    Yes. 

    However, I think the _real_ bug is that we have reiserfs near the tail of 
    filesystems to try.

    Can you test that alternate patch instead? 

Funny how we alternate - when I choose the pure, theoretical point of view
you prefer practice, when I prefer practice you become pure.

This time you prefer practice: the list of filesystems is full of garbage
and good filesystems should be near the top.
I prefer theory: the kernel should not probe at all, so everybody who
forgets rootfstype= gets what he deserves.

Be that as it may - below a patch as I suppose you had in mind.
I don't like it very much. Ordering constraints in makefiles are bad.

Have not compiled or tested.
You can apply it I suppose, but after doing so my earlier patch is still
meaningful. Maybe you should also apply that (and the Doc update).

Andries


diff -u --recursive --new-file -X /linux/dontdiff a/fs/Makefile b/fs/Makefile
--- a/fs/Makefile	2003-12-18 03:58:57.000000000 +0100
+++ b/fs/Makefile	2004-01-27 04:51:33.000000000 +0100
@@ -48,6 +48,7 @@
 obj-$(CONFIG_EXT3_FS)		+= ext3/ # Before ext2 so root fs can be ext3
 obj-$(CONFIG_JBD)		+= jbd/
 obj-$(CONFIG_EXT2_FS)		+= ext2/
+obj-$(CONFIG_REISERFS_FS)	+= reiserfs/
 obj-$(CONFIG_CRAMFS)		+= cramfs/
 obj-$(CONFIG_RAMFS)		+= ramfs/
 obj-$(CONFIG_HUGETLBFS)		+= hugetlbfs/
@@ -84,7 +85,6 @@
 obj-$(CONFIG_AUTOFS_FS)		+= autofs/
 obj-$(CONFIG_AUTOFS4_FS)	+= autofs4/
 obj-$(CONFIG_ADFS_FS)		+= adfs/
-obj-$(CONFIG_REISERFS_FS)	+= reiserfs/
 obj-$(CONFIG_UDF_FS)		+= udf/
 obj-$(CONFIG_SUN_OPENPROMFS)	+= openpromfs/
 obj-$(CONFIG_JFS_FS)		+= jfs/

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [uPATCH] refuse plain ufs mount
@ 2004-01-27  1:56 Andries.Brouwer
  2004-01-27  2:09 ` Linus Torvalds
  0 siblings, 1 reply; 9+ messages in thread
From: Andries.Brouwer @ 2004-01-27  1:56 UTC (permalink / raw)
  To: Andries.Brouwer, gotom; +Cc: akpm, linux-kernel, torvalds

	From gotom@debian.or.jp  Tue Jan 27 02:44:00 2004

	I wonder this modification is really ok because user can't find why he
	can't mount his ufs if he does not specify ufstype=.  Putting only
	one line is not acceptable for you?

But you see, it wasn't the user at all, and it wasn't a ufs filesystem.
It is kernel probing that causes error messages. That is unwanted.
So, your version is wrong.

If it is really very desirable to warn the user the condition if(!silent)
should be added.

But in my opinion such a warning is not desirable at all.
mount(8) and Documentation/filesystems/ufs.txt explain the details.

Andries

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [uPATCH] refuse plain ufs mount
@ 2004-01-25  0:17 Andries.Brouwer
  2004-01-27  1:43 ` GOTO Masanori
  0 siblings, 1 reply; 9+ messages in thread
From: Andries.Brouwer @ 2004-01-25  0:17 UTC (permalink / raw)
  To: akpm, torvalds; +Cc: linux-kernel

Installed some machine with a reiserfs rootfs.
The boot messages contain a lot of garbage spouted by Intermezzo
and ufs_read_super caused by the fact that the kernel tried lots
of other filesystems before hitting on reiserfs.

On the one hand this is solved by "rootfstype=reiserfs".

But on the other hand, the kernel should not complain about the
guessing it does itself. There is a parameter "silent" for this
purpose, but in this case I think it cleaner just to remove the
complaint and tighten the requirement.

Andries

--- /linux/2.6/linux-2.6.1/linux/fs/ufs/super.c	2003-12-18 03:57:57.000000000 +0100
+++ super.c	2004-01-25 01:05:47.000000000 +0100
@@ -516,14 +516,10 @@
 		printk("wrong mount options\n");
 		goto failed;
 	}
-	if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
-		printk("You didn't specify the type of your ufs filesystem\n\n"
-		"mount -t ufs -o ufstype="
-		"sun|sunx86|44bsd|old|hp|nextstep|netxstep-cd|openstep ...\n\n"
-		">>>WARNING<<< Wrong ufstype may corrupt your filesystem, "
-		"default is ufstype=old\n");
-		ufs_set_opt (sbi->s_mount_opt, UFSTYPE_OLD);
-	}
+
+	/* "old" used to be default - now: always specify type */
+	if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE))
+		goto failed;
 
 	sbi->s_uspi = uspi =
 		kmalloc (sizeof(struct ufs_sb_private_info), GFP_KERNEL);

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2004-01-29  0:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-01-28 22:24 [uPATCH] refuse plain ufs mount Example
2004-01-29  0:13 ` GOTO Masanori
  -- strict thread matches above, loose matches on Subject: below --
2004-01-27  4:07 Andries.Brouwer
2004-01-27  5:04 ` GOTO Masanori
2004-01-27  6:15 ` Linus Torvalds
2004-01-27  1:56 Andries.Brouwer
2004-01-27  2:09 ` Linus Torvalds
2004-01-25  0:17 Andries.Brouwer
2004-01-27  1:43 ` GOTO Masanori

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.