ext3: return FSID for statvfs
PATCH 1/1 ext3: return FSID for statvfs
This patch changes ext3_statfs() to return a FSID based on least significant 64-bits of the 128-bit filesystem UUID. This patch is a partial fix for Bugzilla Bug <http://bugzilla.kernel.org/show_bug.cgi?id=136>.
---
super.c | 2 ++ 1 file changed, 2 insertions(+)
fs/ext3/super.c +2 -0
@@ -2340,6 +2340,8 @@ static int ext3_statfs (struct super_blo buf->f_files =le32_to_cpu (es->s_inodes_count); buf->f_ffree =ext3_count_free_inodes (sb); buf->f_namelen =EXT3_NAME_LEN ; + buf->f_fsid.val[0 ] =le32_to_cpup ((void *)es->s_uuid); + buf->f_fsid.val[1 ] =le32_to_cpup ((void *)es->s_uuid +sizeof (u32));return 0 ; }
Quoted 3 lines · Pekka Enberg
This patch changes
ext3_statfs()to return a FSID based on least significant 64-bits of the 128-bit filesystem UUID. This patch is a partial fix for Bugzilla Bug <http://bugzilla.kernel.org/show_bug.cgi?id=136>.
The bug mentions some reasons why this patch is sub-optimal - namely that the beginning of the UUID has common fields in it. It may make more sense to e.g. XOR the first 2 * u32 with the last 2 * u32 to reduce the chance of an FSID collision.
Also, there is a tiny memory of a security issue with exposing the FSID to applications (something to do with NFS and guessing filehandles or similar). I have no idea if that is even relevant any longer, but thought I'd mention it.
fs/ext3/super.c static int ext3_statfs (struct super_blo · line 2347
Quoted 9 lines
fs/ext3/super.c +2
-0@@ -2340,6 +2340,8 @@ static int ext3_statfs (struct super_blo buf->f_files =le32_to_cpu (es->s_inodes_count); buf->f_ffree =ext3_count_free_inodes (sb); buf->f_namelen =EXT3_NAME_LEN ; + buf->f_fsid.val[0 ] =le32_to_cpup ((void *)es->s_uuid); + buf->f_fsid.val[1 ] =le32_to_cpup ((void *)es->s_uuid +sizeof (u32));return 0 ; }
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
Hi Andreas,
Quoted 9 lines · Andreas Dilger
The bug mentions some reasons why this patch is sub-optimal - namely that the beginning of the UUID has common fields in it. It may make more sense to e.g. XOR the first 2 * u32 with the last 2 * u32 to reduce the chance of an FSID collision.
Also, there is a tiny memory of a security issue with exposing the FSID to applications (something to do with NFS and guessing filehandles or similar). I have no idea if that is even relevant any longer, but thought I'd mention it.
Something like this?
This patch changes ext3_statfs() to return a FSID based on 64 bit XOR of the 128 bit filesystem UUID as suggested by Andreas Dilger. This patch is partial fix for Bugzilla Bug <http://bugzilla.kernel.org/show_bug.cgi?id=136>.
---
super.c | 5 +++++ 1 file changed, 5 insertions(+)
fs/ext3/super.c +5 -0
@@ -2294,6 +2294,7 @@ static int ext3_statfs (struct super_blostruct ext3_super_block *es =EXT3_SB (sb)->s_es;unsigned long overhead;int i; +u64 fsid;if (test_opt (sb,MINIX_DF )) overhead =0 ;
@@ -2340,6 +2341,10 @@ static int ext3_statfs (struct super_blo buf->f_files =le32_to_cpu (es->s_inodes_count); buf->f_ffree =ext3_count_free_inodes (sb); buf->f_namelen =EXT3_NAME_LEN ; + fsid =le64_to_cpup ((void *)es->s_uuid) ^ +le64_to_cpup ((void *)es->s_uuid +sizeof (u64)); + buf->f_fsid.val[0 ] = fsid &0xFFFFFFFFUL ; + buf->f_fsid.val[1 ] = (fsid >>32 ) &0xFFFFFFFFUL ;return 0 ; }
Quoted 9 lines · Pekka J Enberg
Quoted 4 lines · Andreas Dilger
The bug mentions some reasons why this patch is sub-optimal - namely that the beginning of the UUID has common fields in it. It may make more sense to e.g. XOR the first 2 * u32 with the last 2 * u32 to reduce the chance of an FSID collision.
This patch changes
ext3_statfs()to return a FSID based on 64 bit XOR of the 128 bit filesystem UUID as suggested by Andreas Dilger. This patch is partial fix for Bugzilla Bug <http://bugzilla.kernel.org/show_bug.cgi?id=136>.
That seems at least slightly better, as it involves all the bits.
Question - what is rule for "Signed-off-by:"? I'm hesitant to add that if I haven't actually compiled+tested a fix, as I've seen too many instances of "obvious fix contains bug" to believe visual inspection is enough. If "Signed-off-by:" simply indicates "Yes, this person approves of the change in principle" then that's OK by me. Andrew, do you use the "CC" tag for that?
Quoted 19 lines
Signed-off-by Pekka Enberg <[email protected]>---
super.c | 5 +++++ 1 file changed, 5 insertions(+)Index:
2.6/fs/ext3/super.c
===================================================================fs/ext3/super.c +1
-0@@ -2294,6 +2294,7 @@ static int ext3_statfs (struct super_blostruct ext3_super_block *es =EXT3_SB (sb)->s_es;unsigned long overhead;int i; +u64 fsid;if (test_opt (sb,MINIX_DF )) overhead =0 ;
fs/ext3/super.c static int ext3_statfs (struct super_blo · line 2350
Quoted 10 lines
fs/ext3/super.c +4
-0@@ -2340,6 +2341,10 @@ static int ext3_statfs (struct super_blo buf->f_files =le32_to_cpu (es->s_inodes_count); buf->f_ffree =ext3_count_free_inodes (sb); buf->f_namelen =EXT3_NAME_LEN ; + fsid =le64_to_cpup ((void *)es->s_uuid) ^ +le64_to_cpup ((void *)es->s_uuid +sizeof (u64)); + buf->f_fsid.val[0 ] = fsid &0xFFFFFFFFUL ; + buf->f_fsid.val[1 ] = (fsid >>32 ) &0xFFFFFFFFUL ;return 0 ; }
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
Quoted 6 lines · Andreas Dilger
Question - what is rule for "Signed-off-by:"? I'm hesitant to add that if I haven't actually compiled+tested a fix, as I've seen too many instances of "obvious fix contains bug" to believe visual inspection is enough. If "Signed-off-by:" simply indicates "Yes, this person approves of the change in principle" then that's OK by me. Andrew, do you use the "CC" tag for that?
I think signed-off is only used when you either (1) change the patch or
(2) pass it forward. I have seen people use "Acked-by:" for what you seem to want to do here.
FWIW, I have tested the patch with UML.
Pekka