Hi,
For tag only AEAD decrypt operation(Zero length Payload). The dst sg
list pointer panic with general protection fault. I think it should be
NULL when output buffer is supposed to be empty.
Kcapi command to re-produce the issue
./kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a ""
-t "5f24c68cbe6f32c29652442bf5d483ad" -q ""
Its decrypt operation. Expected result should be EBADMSG.
Regards
Harsh Jain
Am Dienstag, 21. M?rz 2017, 07:13:53 CET schrieb Harsh Jain:
Hi Harsh,
> Hi,
>
> For tag only AEAD decrypt operation(Zero length Payload). The dst sg
> list pointer panic with general protection fault. I think it should be
> NULL when output buffer is supposed to be empty.
>
> Kcapi command to re-produce the issue
>
> ./kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
> f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a ""
> -t "5f24c68cbe6f32c29652442bf5d483ad" -q ""
>
> Its decrypt operation. Expected result should be EBADMSG.
Executing this command on a 4.9 kernel, I get:
bin/kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a "" -t
"5f24c68cbe6f32c29652442bf5d483ad" -q ""
EBADMSG
There is no GP or other error. Can you please provide some details about your
system? I.e. which kernel version and what cipher implementation resolves to
gcm(aes)?
Thanks
Ciao
Stephan
On Tue, Mar 21, 2017 at 3:34 PM, Stephan Müller <[email protected]> wrote:
> Am Dienstag, 21. März 2017, 07:13:53 CET schrieb Harsh Jain:
>
> Hi Harsh,
>
>> Hi,
>>
>> For tag only AEAD decrypt operation(Zero length Payload). The dst sg
>> list pointer panic with general protection fault. I think it should be
>> NULL when output buffer is supposed to be empty.
>>
>> Kcapi command to re-produce the issue
>>
>> ./kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
>> f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a ""
>> -t "5f24c68cbe6f32c29652442bf5d483ad" -q ""
>>
>> Its decrypt operation. Expected result should be EBADMSG.
>
> Executing this command on a 4.9 kernel, I get:
>
> bin/kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
> f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a "" -t
> "5f24c68cbe6f32c29652442bf5d483ad" -q ""
> EBADMSG
Probably because s/w implementation is not trying to access dst sg
pointer because there's nothing to copy in destination buffer. 1
question If we don't have data to copy to destination buffer what
should dst pointer contains?
>
> There is no GP or other error. Can you please provide some details about your
> system? I.e. which kernel version and what cipher implementation resolves to
> gcm(aes)?
I tried with 4.10.13. It's with gcm(aes-chcr). changes which trigger
issue is not submitted to community yet.
>
> Thanks
>
> Ciao
> Stephan
Am Dienstag, 21. M?rz 2017, 11:59:54 CET schrieb Harsh Jain:
Hi Harsh,
> > Executing this command on a 4.9 kernel, I get:
> >
> > bin/kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
> > f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a "" -t
> > "5f24c68cbe6f32c29652442bf5d483ad" -q ""
> > EBADMSG
>
> Probably because s/w implementation is not trying to access dst sg
> pointer because there's nothing to copy in destination buffer. 1
> question If we don't have data to copy to destination buffer what
> should dst pointer contains?
The dst SGL should simply be discarded by implementations in the case you
mention above.
The implementation receives the tag size and the supplied input buffer. If
that input buffer length is equal to the tag length (i.e. no AAD and no
ciphertext), why would the dst SGL be ever touched during decrytion?
Ciao
Stephan
On Tue, Mar 21, 2017 at 4:29 PM, Harsh Jain <[email protected]> wrote:
> On Tue, Mar 21, 2017 at 3:34 PM, Stephan Müller <[email protected]> wrote:
>> Am Dienstag, 21. März 2017, 07:13:53 CET schrieb Harsh Jain:
>>
>> Hi Harsh,
>>
>>> Hi,
>>>
>>> For tag only AEAD decrypt operation(Zero length Payload). The dst sg
>>> list pointer panic with general protection fault. I think it should be
>>> NULL when output buffer is supposed to be empty.
>>>
>>> Kcapi command to re-produce the issue
>>>
>>> ./kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
>>> f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a ""
>>> -t "5f24c68cbe6f32c29652442bf5d483ad" -q ""
>>>
>>> Its decrypt operation. Expected result should be EBADMSG.
>>
>> Executing this command on a 4.9 kernel, I get:
>>
>> bin/kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
>> f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a "" -t
>> "5f24c68cbe6f32c29652442bf5d483ad" -q ""
>> EBADMSG
>
> Probably because s/w implementation is not trying to access dst sg
> pointer because there's nothing to copy in destination buffer. 1
> question If we don't have data to copy to destination buffer what
> should dst pointer contains? I think either NULL or null sg entry.
>
>>
>> There is no GP or other error. Can you please provide some details about your
>> system? I.e. which kernel version and what cipher implementation resolves to
>> gcm(aes)?
>
> I tried with 4.10.13. It's with gcm(aes-chcr). changes which trigger
> issue is not submitted to community yet.
typo Its 4.10.0-rc3+
>
>>
>> Thanks
>>
>> Ciao
>> Stephan
On Tue, Mar 21, 2017 at 5:13 PM, Stephan Müller <[email protected]> wrote:
> Am Dienstag, 21. März 2017, 11:59:54 CET schrieb Harsh Jain:
>
> Hi Harsh,
>
>> > Executing this command on a 4.9 kernel, I get:
>> >
>> > bin/kcapi -x 2 -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 -k
>> > f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c -a "" -t
>> > "5f24c68cbe6f32c29652442bf5d483ad" -q ""
>> > EBADMSG
>>
>> Probably because s/w implementation is not trying to access dst sg
>> pointer because there's nothing to copy in destination buffer. 1
>> question If we don't have data to copy to destination buffer what
>> should dst pointer contains?
>
> The dst SGL should simply be discarded by implementations in the case you
> mention above.
>
> The implementation receives the tag size and the supplied input buffer. If
> that input buffer length is equal to the tag length (i.e. no AAD and no
> ciphertext), why would the dst SGL be ever touched during decrytion?
Yes, Driver can figure out when to discard dst SGL but for that Driver
has to put checks before accessing dst SGL. Isn't better if AF_ALG
sends NULL for dst SGL.
>
> Ciao
> Stephan
Am Dienstag, 21. M?rz 2017, 14:23:31 CET schrieb Harsh Jain:
Hi Harsh,
> Yes, Driver can figure out when to discard dst SGL but for that Driver
> has to put checks before accessing dst SGL. Isn't better if AF_ALG
> sends NULL for dst SGL.
With the code in [1], the first longer patch is planned to be merged after the
memory management changes are agreed upon. That patch contains:
+ /* chain the areq TX SGL holding the tag with RX SGL */
+ if (!last_rsgl) {
+ /* no RX SGL present (e.g. only authentication) */
+ sg_init_table(areq->first_rsgl.sgl.sg, 2);
+ sg_chain(areq->first_rsgl.sgl.sg, 2, areq->tsgl);
+ } else {
+ /* RX SGL present */
+ struct af_alg_sgl *sgl_prev = &last_rsgl->sgl;
+
+ sg_unmark_end(sgl_prev->sg + sgl_prev->npages - 1);
+ sg_chain(sgl_prev->sg, sgl_prev->npages + 1, areq-
>tsgl);
+ }
This code snipped would exactly do what you want: the SGL is always
initialized. Besides, the code will do an in-place cipher operation.
https://www.spinics.net/lists/linux-crypto/msg24343.html
Ciao
Stephan
On Tue, Mar 21, 2017 at 04:00:04PM +0100, Stephan M?ller wrote:
> Am Dienstag, 21. M?rz 2017, 14:23:31 CET schrieb Harsh Jain:
>
> Hi Harsh,
>
> > Yes, Driver can figure out when to discard dst SGL but for that Driver
> > has to put checks before accessing dst SGL. Isn't better if AF_ALG
> > sends NULL for dst SGL.
>
> With the code in [1], the first longer patch is planned to be merged after the
> memory management changes are agreed upon. That patch contains:
>
> + /* chain the areq TX SGL holding the tag with RX SGL */
> + if (!last_rsgl) {
> + /* no RX SGL present (e.g. only authentication) */
> + sg_init_table(areq->first_rsgl.sgl.sg, 2);
> + sg_chain(areq->first_rsgl.sgl.sg, 2, areq->tsgl);
> + } else {
> + /* RX SGL present */
> + struct af_alg_sgl *sgl_prev = &last_rsgl->sgl;
> +
> + sg_unmark_end(sgl_prev->sg + sgl_prev->npages - 1);
> + sg_chain(sgl_prev->sg, sgl_prev->npages + 1, areq-
> >tsgl);
> + }
>
>
> This code snipped would exactly do what you want: the SGL is always
> initialized. Besides, the code will do an in-place cipher operation.
>
> https://www.spinics.net/lists/linux-crypto/msg24343.html
Even if we fix this one user of the crypto API, new users could
still feed you bogus SG lists. The API does not require the user
to specify a NULL SG list so please fix this in the driver.
We should also strength testmgr so that it provides something
bogus to catch buggy drivers.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt