Re: [RESEND PATCH] debugfs/htree.c: In do_dx_hash() read hash_seed, hash_version directly from superblock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 24, 2023 at 06:56:34AM +0000, Srivathsa Dara wrote:
> diff --git a/debugfs/htree.c b/debugfs/htree.c
> index 7fae7f11..2d881c74 100644
> --- a/debugfs/htree.c
> +++ b/debugfs/htree.c
> @@ -316,7 +316,12 @@ void do_dx_hash(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)),
>  	int		hash_flags = 0;
>  	const struct ext2fs_nls_table *encoding = NULL;
>  
> -	hash_seed[0] = hash_seed[1] = hash_seed[2] = hash_seed[3] = 0;
> +	hash_seed[0] = current_fs->super->s_hash_seed[0];
> +	hash_seed[1] = current_fs->super->s_hash_seed[1];
> +	hash_seed[2] = current_fs->super->s_hash_seed[2];
> +	hash_seed[3] = current_fs->super->s_hash_seed[3];
> +
> +	hash_version = current_fs->super->s_def_hash_version;
>  
>  	reset_getopt();
>  	while ((c = getopt(argc, argv, "h:s:ce:")) != EOF) {

The problem with this patch is that if a file system is not opened,
then current_fs is NULL.  As a result:

% gdb -q debugfs
Reading symbols from debugfs...
(gdb) run
Starting program: /build/e2fsprogs/debugfs/debugfs 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
debugfs 1.47.0 (5-Feb-2023)
debugfs:  dx_hash test1

Program received signal SIGSEGV, Segmentation fault.
0x000055555556f73d in do_dx_hash (argc=2, argv=0x5555555d38d0, sci_idx=1, infop=0x0)
    at /usr/projects/e2fsprogs/e2fsprogs/debugfs/htree.c:343
343             hash_seed[0] = current_fs->super->s_hash_seed[0];


To address this, I've fixed up your patch slightly.  (See below for
the fix up, as well as the final patch.)

Also, in the future, please make sure that the first line of the
commit is a summary of the patch, no longer than 75 characters, and
that the text is wrapped to no more than 75 characters.  (I personally
use 72 characters, but 75 is what suggested in the Linux Kernel's
submitting patches documentation[1] in the "The Canonical Patch
Format" section.)

[1] https://docs.kernel.org/process/submitting-patches.html

					- Ted


diff --git a/debugfs/htree.c b/debugfs/htree.c
index ad493e8fb..a3e95ddb0 100644
--- a/debugfs/htree.c
+++ b/debugfs/htree.c
@@ -336,16 +336,18 @@ void do_dx_hash(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)),
 	errcode_t	err;
 	int		c;
 	int		hash_version = 0;
-	__u32		hash_seed[4];
+	__u32		hash_seed[4] = { 0, };
 	int		hash_flags = 0;
 	const struct ext2fs_nls_table *encoding = NULL;
 
-	hash_seed[0] = current_fs->super->s_hash_seed[0];
-	hash_seed[1] = current_fs->super->s_hash_seed[1];
-	hash_seed[2] = current_fs->super->s_hash_seed[2];
-	hash_seed[3] = current_fs->super->s_hash_seed[3];
+	if (current_fs) {
+		hash_seed[0] = current_fs->super->s_hash_seed[0];
+		hash_seed[1] = current_fs->super->s_hash_seed[1];
+		hash_seed[2] = current_fs->super->s_hash_seed[2];
+		hash_seed[3] = current_fs->super->s_hash_seed[3];
 
-	hash_version = current_fs->super->s_def_hash_version;
+		hash_version = current_fs->super->s_def_hash_version;
+	}
 
 	reset_getopt();
 	while ((c = getopt(argc, argv, "h:s:ce:")) != EOF) {
commit 29d83fef9e6eab139516afe433c03d975e85c25b
Author: Srivathsa Dara <srivathsa.d.dara@xxxxxxxxxx>
Date:   Thu Aug 24 06:56:34 2023 +0000

    debugfs: Use the hash_version from superblock if a file system is opened
    
    The debugfs program's dx_hash command computes the hash for the given
    filename, taking the hash_seed and hash_version (i.e hash algorithm)
    as arguments.  So the user has to refer to the superblock to get these
    values used by the filesystem.  So if debugfs has an opened file
    system, use those values from the current file system.
    
    [ Fixed patch to avoid crashing when a file system is not opened. --TYT ]
    
    Signed-off-by: Srivathsa Dara <srivathsa.d.dara@xxxxxxxxxx>
    Link: https://lore.kernel.org/r/20230824065634.2662858-1-srivathsa.d.dara@xxxxxxxxxx
    Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>

diff --git a/debugfs/htree.c b/debugfs/htree.c
index a9f9211ba..a3e95ddb0 100644
--- a/debugfs/htree.c
+++ b/debugfs/htree.c
@@ -336,11 +336,18 @@ void do_dx_hash(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)),
 	errcode_t	err;
 	int		c;
 	int		hash_version = 0;
-	__u32		hash_seed[4];
+	__u32		hash_seed[4] = { 0, };
 	int		hash_flags = 0;
 	const struct ext2fs_nls_table *encoding = NULL;
 
-	hash_seed[0] = hash_seed[1] = hash_seed[2] = hash_seed[3] = 0;
+	if (current_fs) {
+		hash_seed[0] = current_fs->super->s_hash_seed[0];
+		hash_seed[1] = current_fs->super->s_hash_seed[1];
+		hash_seed[2] = current_fs->super->s_hash_seed[2];
+		hash_seed[3] = current_fs->super->s_hash_seed[3];
+
+		hash_version = current_fs->super->s_def_hash_version;
+	}
 
 	reset_getopt();
 	while ((c = getopt(argc, argv, "h:s:ce:")) != EOF) {

[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux