re: crypto: ccp - CCP SHA crypto API support

re: crypto: ccp - CCP SHA crypto API support

[Dan Carpenter]

Hello Tom Lendacky,

The patch 0ab0a1d505ab: "crypto: ccp - CCP SHA crypto API support"
from Nov 12, 2013, leads to the following static checker warning:
	drivers/crypto/ccp/ccp-crypto-sha.c:182 ccp_do_sha_update()
	warn: should 'rctx->hash_cnt << 3' be a 64 bit type?

drivers/crypto/ccp/ccp-crypto-sha.c
   180                  sg_mark_end(sg);
   181  
   182          rctx->msg_bits += (rctx->hash_cnt << 3);        /* Total in bits */
                                   ^^^^^^^^^^^^^^^^^^^
This operation wraps if the msg is over 500MB.  I'm not sure if that's
possible, but ->msg_bits is declared as a u64 here and in
rctx->cmd.u.sha.msg_bits as well.

   183  
   184          memset(&rctx->cmd, 0, sizeof(rctx->cmd));


regards,
dan carpenter

Re: crypto: ccp - CCP SHA crypto API support

[Tom Lendacky]

On Wednesday, December 11, 2013 01:51:50 PM Dan Carpenter wrote:
> Hello Tom Lendacky,
> 
> The patch 0ab0a1d505ab: "crypto: ccp - CCP SHA crypto API support"
> from Nov 12, 2013, leads to the following static checker warning:
> 	drivers/crypto/ccp/ccp-crypto-sha.c:182 ccp_do_sha_update()
> 	warn: should 'rctx->hash_cnt << 3' be a 64 bit type?
> 
> drivers/crypto/ccp/ccp-crypto-sha.c
>    180                  sg_mark_end(sg);
>    181
>    182          rctx->msg_bits += (rctx->hash_cnt << 3);        /* Total in
> bits */ ^^^^^^^^^^^^^^^^^^^
> This operation wraps if the msg is over 500MB.  I'm not sure if that's
> possible, but ->msg_bits is declared as a u64 here and in
> rctx->cmd.u.sha.msg_bits as well.

I should probably cast hash_cnt to a u64 before doing the shift to be
sure I don't lose any bits.  So that I can validate my fix, which static
checker did you run?

Thanks,
Tom
 
> 
>    183
>    184          memset(&rctx->cmd, 0, sizeof(rctx->cmd));
> 
> 
> regards,
> dan carpenter

Re: crypto: ccp - CCP SHA crypto API support

[Dan Carpenter]

On Wed, Dec 11, 2013 at 08:45:47AM -0600, Tom Lendacky wrote:
> 
> On Wednesday, December 11, 2013 01:51:50 PM Dan Carpenter wrote:
> > Hello Tom Lendacky,
> > 
> > The patch 0ab0a1d505ab: "crypto: ccp - CCP SHA crypto API support"
> > from Nov 12, 2013, leads to the following static checker warning:
> > 	drivers/crypto/ccp/ccp-crypto-sha.c:182 ccp_do_sha_update()
> > 	warn: should 'rctx->hash_cnt << 3' be a 64 bit type?
> > 
> > drivers/crypto/ccp/ccp-crypto-sha.c
> >    180                  sg_mark_end(sg);
> >    181
> >    182          rctx->msg_bits += (rctx->hash_cnt << 3);        /* Total in
> > bits */ ^^^^^^^^^^^^^^^^^^^
> > This operation wraps if the msg is over 500MB.  I'm not sure if that's
> > possible, but ->msg_bits is declared as a u64 here and in
> > rctx->cmd.u.sha.msg_bits as well.
> 
> I should probably cast hash_cnt to a u64 before doing the shift to be
> sure I don't lose any bits.  So that I can validate my fix, which static
> checker did you run?
> 

I haven't released the Smatch check for this...  I need to tweak it so
it has fewer false positives.

regards,
dan carpenter