Re: [Crash-utility] [PATCH v4 2/4] Get the absolute value of SYMNAME_HASH_INDEX

Hi Lianbo, On Mon, 27 Sep 2021 11:33:31 +0800 lijiang <lijiang(a)redhat.com&gt; wrote: > > Date: Sat, 18 Sep 2021 15:59:30 +0800 > > From: Tao Liu <ltao(a)redhat.com&gt; > > To: crash-utility(a)redhat.com > > Subject: [Crash-utility] [PATCH v4 2/4] Get the absolute value of > > SYMNAME_HASH_INDEX > > Message-ID: <20210918075932.132339-3-ltao(a)redhat.com&gt; > > Content-Type: text/plain; charset="US-ASCII" > > > > SYMNAME_HASH_INDEX is used as the index of symname hash table. It will > > be out of range if SYMNAME_HASH_INDEX is negative. Let's get its absolute > > value to avoid such risk. > > > > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > > Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt; > > --- > > defs.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/defs.h b/defs.h > > index c10ebff..3129f06 100644 > > --- a/defs.h > > +++ b/defs.h > > @@ -2725,7 +2725,7 @@ struct downsized { > > > > #define SYMNAME_HASH (512) > > #define SYMNAME_HASH_INDEX(name) \ > > - ((name[0] ^ (name[strlen(name)-1] * name[strlen(name)/2])) % SYMNAME_HASH) > > + (abs((name[0] ^ (name[strlen(name)-1] * name[strlen(name)/2])) % SYMNAME_HASH)) > > > > I guess this may be caused by integer overflow, the expression uses > integer calculations by default. Does the following change work for > you? > > -#define SYMNAME_HASH (512) > +#define SYMNAME_HASH (512ULL) the above isn't sufficient. The sign of the result is defined by the sign of the dividend, i.e. assume a % b, then the result has the same sign as a. So the result of SYMNAME_HASH_INDEX will become negative whenever name[0] ^ (name[strlen(name)-1] * name[strlen(name)/2]) is negative. So the only alternative I see is to guarantee that 'name' is an unsigned char. But that would be more complicated to implement then simply adding the 'abs'.

Thanks Philipp > > Thanks. > Lianbo > > #define PATCH_KERNEL_SYMBOLS_START ((char *)(1)) > > #define PATCH_KERNEL_SYMBOLS_STOP ((char *)(2)) > > -- > > 2.29.2 > > > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://listman.redhat.com/mailman/listinfo/crash-utility >