AF_ALG interface not marking the end of the scatter-gather list

DISCUSSION Tom Lendacky 2 participants 3 messages Feb 12, 2015 – Feb 13, 2015 Crypto API

Share & tools

Tom Lendacky

author

2015-02-12 23:42 UTC #0 lore raw

I was doing some testing of the CCP driver using the AF_ALG interface and encountered a BUG_ON statement during scatter-gather DMA mapping.

In algif_skcipher.c, before submitting a request to the the Crypto API the input sg list is not updated to mark the last valid sg entry of the input data. So even if there is only a single valid sg entry, sg_nents returns 127 (the initial value used when creating the sg table).

In the CCP driver, when making a call to dma_map_sg I supply the number of entries as returned by sg_nents. During this call, the sg elements that are not valid cause a BUG_ON statement to be hit.

I've worked around the issue in skcipher_recvmsg by marking the last valid sg entry (sg_mark_end(sgl->sg + sgl->cur - 1)) just before the call to ablkcipher_request_set_crypt and then unmarking the entry after the return from af_alg_wait_for_completion (using sg_unmark_end).

Is this an appropriate/valid solution for this issue? If so, I can submit a patch with the fix in algif_skcipher and algif_hash.

Thanks,
Tom

Stephan Mueller

2015-02-13 11:43 UTC #1 lore raw

Am Donnerstag, 12. Februar 2015, 17:41:59 schrieb Tom Lendacky:

Hi Tom,

Quoted 12 lines

I was doing some testing of the CCP driver using the AF_ALG interface and encountered a BUG_ON statement during scatter-gather DMA mapping.
In algif_skcipher.c, before submitting a request to the the Crypto API the input sg list is not updated to mark the last valid sg entry of the input data. So even if there is only a single valid sg entry, sg_nents returns 127 (the initial value used when creating the sg table).
In the CCP driver, when making a call to dma_map_sg I supply the number of entries as returned by sg_nents. During this call, the sg elements that are not valid cause a BUG_ON statement to be hit.

Quoted 7 lines

I've worked around the issue in skcipher_recvmsg by marking the last valid sg entry (sg_mark_end(sgl->sg + sgl->cur - 1)) just before the call to ablkcipher_request_set_crypt and then unmarking the entry after the return from af_alg_wait_for_completion (using sg_unmark_end).
Is this an appropriate/valid solution for this issue? If so, I can submit a patch with the fix in algif_skcipher and algif_hash.

There has been a patch around this issue -- see patch 0f477b655a524515ec9a263d70d51f460c05a161

Quoted 7 lines

Thanks,
Tom
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Ciao
Stephan

Tom Lendacky

author

2015-02-13 15:16 UTC #2 lore raw

Quoted 21 lines · Stephan Mueller

Am Donnerstag, 12. Februar 2015, 17:41:59 schrieb Tom Lendacky:
Hi Tom,
Quoted 17 lines
I was doing some testing of the CCP driver using the AF_ALG interface and encountered a BUG_ON statement during scatter-gather DMA mapping.
In algif_skcipher.c, before submitting a request to the the Crypto API the input sg list is not updated to mark the last valid sg entry of the input data. So even if there is only a single valid sg entry, sg_nents returns 127 (the initial value used when creating the sg table).
In the CCP driver, when making a call to dma_map_sg I supply the number of entries as returned by sg_nents. During this call, the sg elements that are not valid cause a BUG_ON statement to be hit.
I've worked around the issue in skcipher_recvmsg by marking the last valid sg entry (sg_mark_end(sgl->sg + sgl->cur - 1)) just before the call to ablkcipher_request_set_crypt and then unmarking the entry after the return from af_alg_wait_for_completion (using sg_unmark_end).

Quoted 5 lines

Quoted 2 lines
Is this an appropriate/valid solution for this issue? If so, I can submit a patch with the fix in algif_skcipher and algif_hash.
There has been a patch around this issue -- see patch 0f477b655a524515ec9a263d70d51f460c05a161

Thanks for the pointer Stephan. I had been working with the main kernel tree where this patch hasn't been merged yet.

Thanks,
Tom

Quoted 9 lines

Quoted 7 lines
Thanks,
Tom
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html