[dpdk-dev] Potential bug in QAT PMD code

[dpdk-dev] Potential bug in QAT PMD code

[Linfeng Li]

Hi,



We believe we found a potential bug in the QAT PMD code.



file link: https://github.com/DPDK/dpdk/blob/main/drivers/crypto/qat/qat_sym.c



The undesired behavior happens when:
*         symmetric operation
*         out-of-place operation
*         encryption
*         do cipher + do hash
*         SGL enabled on either src/dst mbuf chain
*         min_ofs is smaller than the length of the first segment of the src mbuf chain



behavior: In dst mbuf, payload is ciphered as expected, but mac-i remains plain text where it's expected to be ciphered as well.



potential cause:
*         When min_ofs is smaller than the length of the first segment of the src mbuf chain with the foregoing scenario , auth_param->auth_off is calculated by auth_ofs-min_ofs(line 512 in qat_sym.c).
*         When SGL enabled + do auth + do cipher, the remaining_off is calculated by auth_param->auth_off + auth_param->auth_len + alignment_adjustment(line 534 in qat_sym.c). so remaining_off doesn't include the offset applied on auth_param->auth_off in this scenario.
*         The auth_data_end(line 546 in qat_sym.c) found doesn't seem proper since the while loop (line 540 in qat_sym.c) iterates from the very beginning of the dst mbuf.



Proposal fix:

add min_ofs in the calculation of remaining_off(line 534 in qat_sym.c)



Please let us know what your thoughts are about this issue and feel free to contact us if there are any questions.

Linfeng

Re: [dpdk-dev] Potential bug in QAT PMD code

[Doherty, Declan]

H

On 23/02/2021 12:34 AM, Linfeng Li wrote:
> Hi,
> 
> 
> 
> We believe we found a potential bug in the QAT PMD code.
> 
> 
> 
> file link: https://github.com/DPDK/dpdk/blob/main/drivers/crypto/qat/qat_sym.c
> 
> 
> 
> The undesired behavior happens when:
> *         symmetric operation
> *         out-of-place operation
> *         encryption
> *         do cipher + do hash
> *         SGL enabled on either src/dst mbuf chain
> *         min_ofs is smaller than the length of the first segment of the src mbuf chain
> 
> 
> 
> behavior: In dst mbuf, payload is ciphered as expected, but mac-i remains plain text where it's expected to be ciphered as well.
> 
> 
> 
> potential cause:
> *         When min_ofs is smaller than the length of the first segment of the src mbuf chain with the foregoing scenario , auth_param->auth_off is calculated by auth_ofs-min_ofs(line 512 in qat_sym.c).
> *         When SGL enabled + do auth + do cipher, the remaining_off is calculated by auth_param->auth_off + auth_param->auth_len + alignment_adjustment(line 534 in qat_sym.c). so remaining_off doesn't include the offset applied on auth_param->auth_off in this scenario.
> *         The auth_data_end(line 546 in qat_sym.c) found doesn't seem proper since the while loop (line 540 in qat_sym.c) iterates from the very beginning of the dst mbuf.
> 
> 
> 
> Proposal fix:
> 
> add min_ofs in the calculation of remaining_off(line 534 in qat_sym.c)
> 
> 
> 
> Please let us know what your thoughts are about this issue and feel free to contact us if there are any questions.
> 
> Linfeng
> 

Hey Linfeng, thanks we're looking into this now, and will address in 
this release cycle.

Thanks
Declan

Re: [dpdk-dev] Potential bug in QAT PMD code

[Linfeng Li]

Hi Declan,

I'm wondering if there is any update on this issue and if so when the fix can be merged into the master branch. Thanks so much.

Regards,
Linfeng

-----Original Message-----
From: Doherty, Declan <declan.doherty@intel.com> 
Sent: Wednesday, March 3, 2021 2:01 AM
To: Linfeng Li <linfli@qti.qualcomm.com>; dev@dpdk.org
Cc: john.griffin@intel.com; fiona.trahe@intel.com; deepak.k.jain@intel.com; Steve Rizor <srizor@qti.qualcomm.com>; Emil Meng <emeng@qti.qualcomm.com>
Subject: [EXT] Re: [dpdk-dev] Potential bug in QAT PMD code

H

On 23/02/2021 12:34 AM, Linfeng Li wrote:
> Hi,
> 
> 
> 
> We believe we found a potential bug in the QAT PMD code.
> 
> 
> 
> file link: 
> https://github.com/DPDK/dpdk/blob/main/drivers/crypto/qat/qat_sym.c
> 
> 
> 
> The undesired behavior happens when:
> *         symmetric operation
> *         out-of-place operation
> *         encryption
> *         do cipher + do hash
> *         SGL enabled on either src/dst mbuf chain
> *         min_ofs is smaller than the length of the first segment of the src mbuf chain
> 
> 
> 
> behavior: In dst mbuf, payload is ciphered as expected, but mac-i remains plain text where it's expected to be ciphered as well.
> 
> 
> 
> potential cause:
> *         When min_ofs is smaller than the length of the first segment of the src mbuf chain with the foregoing scenario , auth_param->auth_off is calculated by auth_ofs-min_ofs(line 512 in qat_sym.c).
> *         When SGL enabled + do auth + do cipher, the remaining_off is calculated by auth_param->auth_off + auth_param->auth_len + alignment_adjustment(line 534 in qat_sym.c). so remaining_off doesn't include the offset applied on auth_param->auth_off in this scenario.
> *         The auth_data_end(line 546 in qat_sym.c) found doesn't seem proper since the while loop (line 540 in qat_sym.c) iterates from the very beginning of the dst mbuf.
> 
> 
> 
> Proposal fix:
> 
> add min_ofs in the calculation of remaining_off(line 534 in qat_sym.c)
> 
> 
> 
> Please let us know what your thoughts are about this issue and feel free to contact us if there are any questions.
> 
> Linfeng
> 

Hey Linfeng, thanks we're looking into this now, and will address in this release cycle.

Thanks
Declan