Re: [patch] hpsa: fix an sprintf() overflow in the reset handler

[Dan Carpenter <dan.carpenter@oracle.com>]

On Fri, Jun 19, 2015 at 07:13:47AM +0000, Seymour, Shane M wrote:
> With a size of 48 while it won't overflow since you're using snprintf the string with a maximum value in %d:
> 
> echo -n "cmd 2147483647 RESET FAILED, new lockup detected" |wc -c
> 48

I actually just chose 48 because it was divisible by sizeof(long) on
64 bit.  The compiler is going to pad it out a bit anyway because of
alignment.

> 
> is 48 characters long without a null terminator on the string (and in the unlikely event that it's somehow a the largest possible negative value it would be 49 characters after including the minus sign without a null terminator). If you always want a complete message no matter what the value formatted as %d will be I believe it needs to have a length of 50. The worst that will happen now though because you're using snprintf is you'll drop the trailing "d" or "ed" in the message with very large positive or negative numbers.
> 
> My somewhat sketchy memory of the hpsa driver is that the nr_cmds member of the struct ctlr_info is never more than 1040 (?) anyway. If things are working correctly I don't think it should ever happen but I thought I should point out that msg isn't large enough to contain the complete contents of the longest possible character string.

My knowledge is sketchier than yours.

When I was reading this I was thinking that instead of 1040 the
upper limit was 32.  I got that from hpsa_get_max_perf_mode_cmds().
The only negative number was -1, I think.

But either way, we both agree that 48 is probably safe.

regards,
dan carpenter