On 10/3/2022 12:39 PM, Enzo Matsumiya wrote:
On 10/03, Tom Talpey wrote:
On 10/3/2022 11:20 AM, Enzo Matsumiya wrote:
On 10/03, Tom Talpey wrote:
On 10/3/2022 10:53 AM, Enzo Matsumiya wrote:
On 10/03, Tom Talpey wrote:
On 10/2/2022 11:54 PM, Steve French wrote:
shash was not being initialized in one place in smb3_calc_signature
Suggested-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
I don't see the issue. The shash pointer is initialized in both
arms of the "if (allocate_crypto)" block.
True, but cifs_alloc_hash() returns 0 if *sdesc is not NULL, so
crypto_shash_setkey() got stack garbage as sdesc.
Sorry, I still don't get it.
if (allocate_crypto) {
rc = cifs_alloc_hash("cmac(aes)", &hash, &sdesc);
I think you're looking at an old HEAD. We've hit this bug after my
patch 10c6c7b0f060 "cifs: secmech: use shash_desc directly, remove
sdesc"
Aha. But I'm looking at Steve's current cifs-2.6. Where should
I be looking?
It's in cifs-2.6 for-next branch, HEAD 11b1c98d0986 (already has the fix
for smb2_calc_signature).
I can't see branches from the git.samba.org web interface.
Are there any browsing alternatives to pulling a full repo? Using
git on WSL is really, really slow.
Tom.
Tom.
Cheers,
Enzo
which changes the above line to:
rc = cifs_alloc_hash("cmac(aes)", &shash);
In that patch, shash is the only struct to be initialized.
cifs_alloc_hash() is:
cifs_alloc_hash(const char *name, struct shash_desc **sdesc)
{
int rc = 0;
struct crypto_shash *alg = NULL;
if (*sdesc)
return 0;
...
Hence, sdesc having the stack garbage, cifs_alloc_hash returns 0 and
crypto_shash_setkey crashes.
if (rc)
return rc;
shash = &sdesc->shash;
} else {
hash = server->secmech.cmacaes;
shash = &server->secmech.sdesccmacaes->shash;
}
The "shash" pointer is initialized one line later.
And, "sdesc" is already initilized to NULL.
Steve's patch initializes "shash", but now you're referring to
sdesc, and it still looks correct to me. Confused...
But if you do want to add this, them smb2_calc_signature has
the same code.
True. Steve will you add it to this patch please?
FYI, smb2_calc_signature() also initializes sdesc, and not shash.
Does it not oops? Same code.
Tom.
