Skip to contentSkip to conversation

fat: Fix sb_rdonly() change

PATCH Applied OGAWA Hirofumi 3 participants 6 messages Nov 23, 2017 – Nov 28, 2017

Subsystems (2)
VFS VFAT/FAT/msdos filesystem
lore
Share & tools
lore

OGAWA Hirofumi

author maintainer
#0 lore raw

PATCH 1/1 fat: Fix sb_rdonly() change

commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.

Signed-off-by OGAWA Hirofumi <[email protected]>

---

 fs/fat/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/fat/inode.c renamed from fs/fat/inode.c~fat-fix-sb_rdonly-fix +1 -1
diff -puN fs/fat/inode.c~fat-fix-sb_rdonly-fix fs/fat/inode.c
@@ -779,7 +779,7 @@ static void __exit fat_destroy_inodecach
 
 static int fat_remount(struct super_block *sb, int *flags, char *data)
 {
-	int new_rdonly;
+	bool new_rdonly;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	*flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);

_

Signature · 1 line
OGAWA Hirofumi <[email protected]>
author maintainer
#1 lore raw

PATCH 1/1 fat: Fix sb_rdonly() change

Ouch forgot to add stable@

Signature · 24 lines
commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.

Cc: <[email protected]>
Signed-off-by: OGAWA Hirofumi <[email protected]>
---

 fs/fat/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN fs/fat/inode.c~fat-fix-sb_rdonly-fix fs/fat/inode.c
--- linux/fs/fat/inode.c~fat-fix-sb_rdonly-fix	2017-11-23 15:03:47.371667601 +0900
+++ linux-hirofumi/fs/fat/inode.c	2017-11-23 15:04:07.654616440 +0900
@@ -779,7 +779,7 @@ static void __exit fat_destroy_inodecach
 
 static int fat_remount(struct super_block *sb, int *flags, char *data)
 {
-	int new_rdonly;
+	bool new_rdonly;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	*flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);
 
_
-- 
OGAWA Hirofumi <[email protected]>
#2 lore raw
Quoted 4 lines · OGAWA Hirofumi

Ouch forgot to add stable@

Signature · 1 line
commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.

I think your commit message needs a bit more information.

It'd be useful to describe that the introduction of sb_rdonly converted the bitwise & to a boolean and so this conversion and comparison was made defective.

Are there any other instances of defective comparisons?

author maintainer
#3 lore raw
Quoted 6 lines · Joe Perches
Quoted 4 lines · OGAWA Hirofumi

Ouch forgot to add stable@

Signature · 1 line
commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.
Quoted 7 lines

I think your commit message needs a bit more information.

It'd be useful to describe that the introduction of sb_rdonly converted the bitwise & to a boolean and so this conversion and comparison was made defective.

Are there any other instances of defective comparisons?

Please ask to that patch author.

Signature · 1 line
OGAWA Hirofumi <[email protected]>
#4 lore raw
Quoted 10 lines · OGAWA Hirofumi
Quoted 8 lines · Joe Perches
Quoted 4 lines · OGAWA Hirofumi

Ouch forgot to add stable@

Signature · 1 line
commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.

I think your commit message needs a bit more information.

Quoted 7 lines
Quoted 5 lines

It'd be useful to describe that the introduction of sb_rdonly converted the bitwise & to a boolean and so this conversion and comparison was made defective.

Are there any other instances of defective comparisons?

Please ask to that patch author.

The patch author, David Howells, is on the cc list.

btw:

It seems all the the other uses use a (bool) cast of the (*flags & MS_RDONLY) and a comparison of sb_rdonly(sb).

It would make sense to change the argument type of the ext[24]_setup_super int read_only arg to bool to match the sb_rdonly() type.

#5 lore raw
Quoted 17 lines · On Thu, 23 Nov 2017 22:51:34 -0800 Joe Perches <[email protected]> wrote:
Quoted 16 lines · OGAWA Hirofumi
Quoted 13 lines · Joe Perches
Quoted 4 lines · OGAWA Hirofumi

Ouch forgot to add stable@

Signature · 1 line
commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.

I think your commit message needs a bit more information.

It'd be useful to describe that the introduction of sb_rdonly converted the bitwise & to a boolean and so this conversion and comparison was made defective.

Are there any other instances of defective comparisons?

Quoted 8 lines
Quoted 1 line

Please ask to that patch author.

The patch author, David Howells, is on the cc list.

btw:

It seems all the the other uses use a (bool) cast of the (*flags & MS_RDONLY) and a comparison of sb_rdonly(sb).

Sentence is hard to understand.

Quoted 3 lines

It would make sense to change the argument type of the ext[24]_setup_super int read_only arg to bool to match the sb_rdonly() type.

I did this:

: From: OGAWA Hirofumi <[email protected]>
: Subject: fat: Fix sb_rdonly() change
:
: bc98a42c1f7d0f ("VFS: Convert sb->s_flags & MS_RDONLY to sb_rdonly(sb)") : converted fat_remount():new_rdonly from a bool to an int. However : fat_remount() depends upon the compiler's conversion of a non-zero integer : into boolean `true'.
:
: Fix it by switching `new_rdonly' back into a bool.
:
: Link: http://lkml.kernel.org/r/[email protected]
: Fixes: bc98a42c1f7d0f8 ("VFS: Convert sb->s_flags & MS_RDONLY to sb_rdonly(sb)") : Signed-off-by: OGAWA Hirofumi <[email protected]>
: Cc: Joe Perches <[email protected]>
: Cc: David Howells <[email protected]>
: Cc: <[email protected]>
: Signed-off-by: Andrew Morton <[email protected]>
: ---
:
: fs/fat/inode.c | 2 +-
: 1 file changed, 1 insertion(+), 1 deletion(-)
:
: diff -puN fs/fat/inode.c~fat-fix-sb_rdonly-change fs/fat/inode.c
: --- a/fs/fat/inode.c~fat-fix-sb_rdonly-change
: +++ a/fs/fat/inode.c
: @@ -779,7 +779,7 @@ static void __exit fat_destroy_inodecach
:
: static int fat_remount(struct super_block *sb, int *flags, char *data) : {
: - int new_rdonly;
: + bool new_rdonly;
: struct msdos_sb_info *sbi = MSDOS_SB(sb);
: *flags |= SB_NODIRATIME | (sbi->options.isvfat ? 0 : SB_NOATIME);
:
: _