Hello Tom Zanussi,
The patch b190447e0fa3: "crypto: iaa - Add compression mode
management along with fixed mode" from Dec 5, 2023 (linux-next),
leads to the following Smatch static checker warning:
drivers/crypto/intel/iaa/iaa_crypto_main.c:532 init_device_compression_mode()
error: not allocating enough for = 'device_mode->aecs_decomp_table' 5352 vs 1600
drivers/crypto/intel/iaa/iaa_crypto_main.c
510 static int init_device_compression_mode(struct iaa_device *iaa_device,
511 struct iaa_compression_mode *mode,
512 int idx, struct idxd_wq *wq)
513 {
514 size_t size = sizeof(struct aecs_comp_table_record) + IAA_AECS_ALIGN;
515 struct device *dev = &iaa_device->idxd->pdev->dev;
516 struct iaa_device_compression_mode *device_mode;
517 int ret = -ENOMEM;
518
519 device_mode = kzalloc(sizeof(*device_mode), GFP_KERNEL);
520 if (!device_mode)
521 return -ENOMEM;
522
523 device_mode->name = kstrdup(mode->name, GFP_KERNEL);
524 if (!device_mode->name)
525 goto free;
526
527 device_mode->aecs_comp_table = dma_alloc_coherent(dev, size,
528 &device_mode->aecs_comp_table_dma_addr, GFP_KERNEL);
529 if (!device_mode->aecs_comp_table)
530 goto free;
531
--> 532 device_mode->aecs_decomp_table = dma_alloc_coherent(dev, size,
^^^^^^
comp and decomp sizes are different. So we should be allocating
aecs_decomp_table_record + IAA_AECS_ALIGN here probably.
533 &device_mode->aecs_decomp_table_dma_addr, GFP_KERNEL);
534 if (!device_mode->aecs_decomp_table)
535 goto free;
536
537 /* Add Huffman table to aecs */
538 memset(device_mode->aecs_comp_table, 0, sizeof(*device_mode->aecs_comp_table));
539 memcpy(device_mode->aecs_comp_table->ll_sym, mode->ll_table, mode->ll_table_size);
540 memcpy(device_mode->aecs_comp_table->d_sym, mode->d_table, mode->d_table_size);
541
regards,
dan carpenter
Hi Dan,
On Mon, 2024-01-08 at 12:26 +0300, Dan Carpenter wrote:
> Hello Tom Zanussi,
>
> The patch b190447e0fa3: "crypto: iaa - Add compression mode
> management along with fixed mode" from Dec 5, 2023 (linux-next),
> leads to the following Smatch static checker warning:
>
> drivers/crypto/intel/iaa/iaa_crypto_main.c:532
> init_device_compression_mode()
> error: not allocating enough for = 'device_mode-
> >aecs_decomp_table' 5352 vs 1600
>
> drivers/crypto/intel/iaa/iaa_crypto_main.c
> 510 static int init_device_compression_mode(struct iaa_device
> *iaa_device,
> 511 struct
> iaa_compression_mode *mode,
> 512 int idx, struct
> idxd_wq *wq)
> 513 {
> 514 size_t size = sizeof(struct aecs_comp_table_record) +
> IAA_AECS_ALIGN;
> 515 struct device *dev = &iaa_device->idxd->pdev->dev;
> 516 struct iaa_device_compression_mode *device_mode;
> 517 int ret = -ENOMEM;
> 518
> 519 device_mode = kzalloc(sizeof(*device_mode),
> GFP_KERNEL);
> 520 if (!device_mode)
> 521 return -ENOMEM;
> 522
> 523 device_mode->name = kstrdup(mode->name, GFP_KERNEL);
> 524 if (!device_mode->name)
> 525 goto free;
> 526
> 527 device_mode->aecs_comp_table =
> dma_alloc_coherent(dev, size,
> 528
> &device_mode->aecs_comp_table_dma_addr, GFP_KERNEL);
> 529 if (!device_mode->aecs_comp_table)
> 530 goto free;
> 531
> --> 532 device_mode->aecs_decomp_table =
> dma_alloc_coherent(dev, size,
> ^^^^^^
> comp and decomp sizes are different. So we should be allocating
> aecs_decomp_table_record + IAA_AECS_ALIGN here probably.
Thanks for pointing this out.
This is actually unused by the current code - it was part of the canned
mode that was removed during review.
I'll submit a patch to remove it completely, and will fix it if/when we
resubmit canned mode.
Thanks,
Tom
>
> 533
> &device_mode->aecs_decomp_table_dma_addr, GFP_KERNEL);
> 534 if (!device_mode->aecs_decomp_table)
> 535 goto free;
> 536
> 537 /* Add Huffman table to aecs */
> 538 memset(device_mode->aecs_comp_table, 0,
> sizeof(*device_mode->aecs_comp_table));
> 539 memcpy(device_mode->aecs_comp_table->ll_sym, mode-
> >ll_table, mode->ll_table_size);
> 540 memcpy(device_mode->aecs_comp_table->d_sym, mode-
> >d_table, mode->d_table_size);
> 541
>
> regards,
> dan carpenter