LinuxLists.cc - ixp4xx

[permalink] [raw]

Subject: Re: ixp4xx_crypto panic with fragmented packets in scatterlist

The latest thing i found is kernel BUG at lib/scatterlist.c:26!

ping -s 1800 host in a IPSec tunnel. Setup is 802.1Q VLAN -->
ixp4xx_eth --> IPSec Tunnel

trace with some debug printk's i added. printk("%s:%d
values",__func__, __LINE__);

[42949542.170000] esp_output:142 skb->len=1828 data_len=328 clen=1832 alen=0 blksize=8 skb=c7a5cb40 data=c79ba054
[42949542.180000] esp_output:168 skb=c7a5cb40 data=c79ba054 clone=0
[42949542.180000] __skb_to_sgvec:2430 skb=c7a5cb40 data=c79ba030 sg=c78a0324 off=36 len=1832
[42949542.190000] __skb_to_sgvec:2477 list=c795a0c0 start=1536 end=1868 list->len=332 offset=1536
[42949542.200000] __skb_to_sgvec:2430 skb=c795a0c0 data=c6d8f048 sg=c78a0338 off=0 len=332
[42949542.210000] __skb_to_sgvec:2484 len==0 elt=2
[42949542.210000] esp_output:218 skb=c7a5cb40 data=c79ba030
[42949542.220000] esp_output:228 skb=c7a5cb40 data=c79ba030
[42949542.230000] ablk_perform:897 ivsize=8 nbytes=1840
[42949542.230000] ablk_perform:919 dst=NULL src=c78a0278
[42949542.240000] count_sg:757 i=0 nbytes=1840 sg=c78a0278 sg_next(sg)=c78a028c
[42949542.240000] count_sg:758 len=1508 sg_is_last(sg)=0
[42949542.250000] count_sg:757 i=1 nbytes=332 sg=c78a028c sg_next(sg)=c78a02a0
[42949542.250000] count_sg:758 len=0 sg_is_last(sg)=0
[42949542.260000] kernel BUG at lib/scatterlist.c:26!
[42949542.260000] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[42949542.270000] pgd = c0004000
[42949542.280000] [00000000] *pgd=00000000
[42949542.280000] Internal error: Oops: 817 [#1]
[42949542.280000] Modules linked in: xt_MARK crc_ccitt nf_conntrack_pptp nf_conntrack_proto_gre ixp4xx_crypto ipt_MASQUERADE ipt_REDIRECT nG
[42949542.280000] CPU: 0 Not tainted (2.6.28.7 #20)
[42949542.280000] PC is at __bug+0x20/0x2c
[42949542.280000] LR is at release_console_sem+0x1b0/0x1ec
[42949542.280000] pc : [<c00271dc>] lr : [<c0035d4c>] psr: 60000013
[42949542.280000] sp : c031fbac ip : c031fae4 fp : c031fbb8
[42949542.280000] r10: 00000730 r9 : 00000000 r8 : c78a02f8
[42949542.280000] r7 : ffc0b180 r6 : 00000002 r5 : 0000014c r4 : c78a02a0
[42949542.280000] r3 : 00000000 r2 : c03238c8 r1 : 000040e5 r0 : 00000039
[42949542.280000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
[42949542.280000] Control: 000039ff Table: 07a1c000 DAC: 00000017
[42949542.280000] Process swapper (pid: 0, stack limit = 0xc031e260)
[42949542.280000] Stack: (0xc031fbac to 0xc0320000)
[42949542.280000] fba0: c031fbc8 c031fbbc c013cf60 c00271c8 c031fbf0
[42949542.280000] fbc0: c031fbcc bf0c7300 c013cf44 00000000 c78a028c c78a02a0 c7a5c62c 00000008
[42949542.280000] fbe0: c78a02d0 c031fc28 c031fbf4 bf0c7938 bf0c72e8 00000730 00000000 00000020
[42949542.280000] fc00: 00000005 c78a0244 c78a0278 00000008 0000004c c78a0324 c78a02d0 c031fc38
[42949542.280000] fc20: c031fc2c bf0c7b64 bf0c777c c031fc90 c031fc3c c0116a80 bf0c7b5c c79ba000
[42949542.280000] fc40: 87654321 c78a02c8 c7b286ec c78a0324 c79ba054 c79ba054 000f3740 c79ba054
[42949542.280000] fc60: 00000000 00000005 00000005 c7b286c0 c79ba04c c78a0214 c79ba044 00000002
[42949542.280000] fc80: c78a0208 c031fcb0 c031fc94 c0122d88 c0116790 00000005 c7a5cb60 c79ba04c
[42949542.280000] fca0: c7a5cb40 c031fd1c c031fcb4 c0226658 c0122d1c c79ba030 00000000 00000000
[42949542.280000] fcc0: 00000008 c7a5cb40 c79ba054 00000000 c7aa8a00 c7b28880 c78a0324 c78a0310
[42949542.280000] fce0: c78a0200 c78a0200 00000728 00000000 c795a0c0 00000005 c7aa8a00 c7a5cb40
[42949542.280000] fd00: c031e000 c6d87000 00000000 c035f544 c031fd5c c031fd20 c0232b8c c0226284
[42949542.280000] fd20: c031fd5c c031fd30 c01ee0e4 00000000 00000004 c7a5cb40 c7a5cb40 c6d87000
[42949542.280000] fd40: c7a5cb60 c7a5cb40 c035d6b4 00000000 c031fd6c c031fd60 c0232ca0 c02328a4
[42949542.280000] fd60: c031fd8c c031fd70 c0232d94 c0232c98 c7a5cb40 c79ba054 c6d87000 c7a5cb60
[42949542.280000] fd80: c031fd9c c031fd90 c0229c70 c0232cb0 c031fdbc c031fda0 c0229cd0 c0229c28
[42949542.280000] fda0: c7878800 c0229c1c 80000000 c7a5cb40 c031fdd0 c031fdc0 c01f6fc4 c0229c80
[42949542.280000] fdc0: c7a5cb40 c031fdfc c031fdd4 c01f7290 c01f6f8c c7878800 c01f6f80 80000000
[42949542.280000] fde0: c035d6ec c7957034 c035d4a4 c7b37800 c031fe3c c031fe00 c01f5b94 c01f6fd8
[42949542.280000] fe00: c7b37800 c031fe10 c01f585c 80000000 c035db38 c035d6ec c035d6ec c7a5cb40
[42949542.280000] fe20: c035d4a4 c7b37800 c035d704 00000000 c031fe68 c031fe40 c01f60dc c01f5868
[42949542.280000] fe40: 00000000 c01f585c 80000000 c035d6ec c7a5cb40 c7b37800 00000800 c031fe98
[42949542.280000] fe60: c031fe6c c01d968c c01f5eac c0173344 c002ccbc c035d6cc 00000002 ffffcce9
[42949542.280000] fe80: 00000040 00000000 c035d6b0 c031fec4 c031fe9c c01dbc84 c01d9318 c035d6cc
[42949542.280000] fea0: 00000040 00000002 0000012a c035d6b0 c035d6c0 ffffcce9 c031fef0 c031fec8
[42949542.280000] fec0: c01db688 c01dbc04 00000100 0000000c c031e000 00000001 0000000a c0348dc0
[42949542.280000] fee0: c0348e34 c031ff20 c031fef4 c003a140 c01db634 c002c4e4 00000003 00000000
[42949542.280000] ff00: 00000008 c033f708 0001dd18 69054041 0001dc48 c031ff30 c031ff24 c003a4bc
[42949542.280000] ff20: c003a0e4 c031ff48 c031ff34 c0023068 c003a484 ffffffff 0000001f c031ffa0
[42949542.280000] ff40: c031ff4c c0023a04 c002300c c0340094 c7aa0000 c031e000 c033f710 c0024e18
[42949542.280000] ff60: c031e000 c0024e18 c033f708 0001dd18 69054041 0001dc48 c031ffa0 c031ffa4
[42949542.280000] ff80: c031ff94 c0024ce0 c0024e28 60000013 ffffffff c031ffc0 c031ffa4 c0024ce0
[42949542.280000] ffa0: c0024e24 c034b854 c033f284 c001fdb8 c0322374 c031ffd0 c031ffc4 c0263534
[42949542.280000] ffc0: c0024cb0 c031fff4 c031ffd4 c00089d8 c02634ec c0008374 c001fdb8 000039fd
[42949542.280000] ffe0: c033f76c c00201bc 00000000 c031fff8 00008034 c00087a0 00000000 00000000
[42949542.280000] Backtrace:
[42949542.280000] [<c00271bc>] (__bug+0x0/0x2c) from [<c013cf60>] (sg_next+0x28/0x58)
[42949542.280000] [<c013cf38>] (sg_next+0x0/0x58) from [<bf0c7300>] (count_sg+0x24/0xac [ixp4xx_crypto])
[42949542.280000] [<bf0c72dc>] (count_sg+0x0/0xac [ixp4xx_crypto]) from [<bf0c7938>] (ablk_perform+0x1c8/0x36c [ixp4xx_crypto])
[42949542.280000] r6:c78a02d0 r5:00000008 r4:c7a5c62c
[42949542.280000] [<bf0c7770>] (ablk_perform+0x0/0x36c [ixp4xx_crypto]) from [<bf0c7b64>] (ablk_encrypt+0x14/0x18 [ixp4xx_crypto])
[42949542.280000] [<bf0c7b50>] (ablk_encrypt+0x0/0x18 [ixp4xx_crypto]) from [<c0116a80>] (eseqiv_givencrypt+0x2fc/0x328)
[42949542.280000] [<c0116784>] (eseqiv_givencrypt+0x0/0x328) from [<c0122d88>] (crypto_authenc_givencrypt+0x78/0x98)
[42949542.280000] [<c0122d10>] (crypto_authenc_givencrypt+0x0/0x98) from [<c0226658>] (esp_output+0x3e0/0x434)
[42949542.280000] r7:c7a5cb40 r6:c79ba04c r5:c7a5cb60 r4:00000005
[42949542.280000] [<c0226278>] (esp_output+0x0/0x434) from [<c0232b8c>] (xfrm_output_resume+0x2f4/0x3f4)
[42949542.280000] [<c0232898>] (xfrm_output_resume+0x0/0x3f4) from [<c0232ca0>] (xfrm_output2+0x14/0x18)
[42949542.280000] [<c0232c8c>] (xfrm_output2+0x0/0x18) from [<c0232d94>] (xfrm_output+0xf0/0x100)
[42949542.280000] [<c0232ca4>] (xfrm_output+0x0/0x100) from [<c0229c70>] (xfrm4_output_finish+0x54/0x58)
[42949542.280000] r7:c7a5cb60 r6:c6d87000 r5:c79ba054 r4:c7a5cb40
[42949542.280000] [<c0229c1c>] (xfrm4_output_finish+0x0/0x58) from [<c0229cd0>] (xfrm4_output+0x5c/0x68)
[42949542.280000] [<c0229c74>] (xfrm4_output+0x0/0x68) from [<c01f6fc4>] (ip_forward_finish+0x44/0x4c)
[42949542.280000] r4:c7a5cb40
[42949542.280000] [<c01f6f80>] (ip_forward_finish+0x0/0x4c) from [<c01f7290>] (ip_forward+0x2c4/0x340)
[42949542.280000] r4:c7a5cb40
[42949542.280000] [<c01f6fcc>] (ip_forward+0x0/0x340) from [<c01f5b94>] (ip_rcv_finish+0x338/0x35c)
[42949542.280000] r7:c7b37800 r6:c035d4a4 r5:c7957034 r4:c035d6ec
[42949542.280000] [<c01f585c>] (ip_rcv_finish+0x0/0x35c) from [<c01f60dc>] (ip_rcv+0x23c/0x270)
[42949542.280000] [<c01f5ea0>] (ip_rcv+0x0/0x270) from [<c01d968c>] (netif_receive_skb+0x380/0x3c0)
[42949542.280000] r7:00000800 r6:c7b37800 r5:c7a5cb40 r4:c035d6ec
[42949542.280000] [<c01d930c>] (netif_receive_skb+0x0/0x3c0) from [<c01dbc84>] (process_backlog+0x8c/0x114)
[42949542.280000] [<c01dbbf8>] (process_backlog+0x0/0x114) from [<c01db688>] (net_rx_action+0x60/0x1a4)
[42949542.280000] [<c01db628>] (net_rx_action+0x0/0x1a4) from [<c003a140>] (__do_softirq+0x68/0x104)
[42949542.280000] [<c003a0d8>] (__do_softirq+0x0/0x104) from [<c003a4bc>] (irq_exit+0x44/0x4c)
[42949542.280000] [<c003a478>] (irq_exit+0x0/0x4c) from [<c0023068>] (__exception_text_start+0x68/0x84)
[42949542.280000] [<c0023000>] (__exception_text_start+0x0/0x84) from [<c0023a04>] (__irq_svc+0x24/0x80)
[42949542.280000] Exception stack(0xc031ff4c to 0xc031ff94)
[42949542.280000] ff40: c0340094 c7aa0000 c031e000 c033f710 c0024e18
[42949542.280000] ff60: c031e000 c0024e18 c033f708 0001dd18 69054041 0001dc48 c031ffa0 c031ffa4
[42949542.280000] ff80: c031ff94 c0024ce0 c0024e28 60000013 ffffffff
[42949542.280000] r5:0000001f r4:ffffffff
[42949542.280000] [<c0024e18>] (default_idle+0x0/0x4c) from [<c0024ce0>] (cpu_idle+0x3c/0x58)
[42949542.280000] [<c0024ca4>] (cpu_idle+0x0/0x58) from [<c0263534>] (rest_init+0x54/0x68)
[42949542.280000] r7:c0322374 r6:c001fdb8 r5:c033f284 r4:c034b854
[42949542.280000] [<c02634e0>] (rest_init+0x0/0x68) from [<c00089d8>] (start_kernel+0x244/0x2a4)
[42949542.280000] [<c0008794>] (start_kernel+0x0/0x2a4) from [<00008034>] (0x8034)
[42949542.280000] r6:c00201bc r5:c033f76c r4:000039fd
[42949542.280000] Code: e1a01000 e59f000c eb003c6b e3a03000 (e5833000)
[42949542.290000] Kernel panic - not syncing: Fatal exception in interrupt
[42949542.300000] Rebooting in 10 seconds..+oxtias1

2009-02-25 09:07:39

[permalink] [raw]

Subject: Re: ixp4xx_crypto panic with fragmented packets in scatterlist

On Tue, Feb 24, 2009 at 02:19:54PM +0100, Karl Hiramoto wrote:
> The latest thing i found is kernel BUG at lib/scatterlist.c:26!
>
> ping -s 1800 host in a IPSec tunnel. Setup is 802.1Q VLAN -->
> ixp4xx_eth --> IPSec Tunnel
>
> trace with some debug printk's i added. printk("%s:%d
> values",__func__, __LINE__);
>
>
> [42949542.170000] esp_output:142 skb->len=1828 data_len=328 clen=1832 alen=0 blksize=8 skb=c7a5cb40 data=c79ba054
> [42949542.180000] esp_output:168 skb=c7a5cb40 data=c79ba054 clone=0
> [42949542.180000] __skb_to_sgvec:2430 skb=c7a5cb40 data=c79ba030 sg=c78a0324 off=36 len=1832
> [42949542.190000] __skb_to_sgvec:2477 list=c795a0c0 start=1536 end=1868 list->len=332 offset=1536
> [42949542.200000] __skb_to_sgvec:2430 skb=c795a0c0 data=c6d8f048 sg=c78a0338 off=0 len=332
> [42949542.210000] __skb_to_sgvec:2484 len==0 elt=2
> [42949542.210000] esp_output:218 skb=c7a5cb40 data=c79ba030
> [42949542.220000] esp_output:228 skb=c7a5cb40 data=c79ba030
> [42949542.230000] ablk_perform:897 ivsize=8 nbytes=1840
> [42949542.230000] ablk_perform:919 dst=NULL src=c78a0278
> [42949542.240000] count_sg:757 i=0 nbytes=1840 sg=c78a0278 sg_next(sg)=c78a028c
> [42949542.240000] count_sg:758 len=1508 sg_is_last(sg)=0
> [42949542.250000] count_sg:757 i=1 nbytes=332 sg=c78a028c sg_next(sg)=c78a02a0
> [42949542.250000] count_sg:758 len=0 sg_is_last(sg)=0
> [42949542.260000] kernel BUG at lib/scatterlist.c:26!
> [42949542.260000] Unable to handle kernel NULL pointer dereference at virtual address 00000000

This differs from the issue you mentioned first.
The first one was in "dma_cache_maint".

Are you sure, your printk()s don't have any side effect ?

Christian

--
The decline of the Roman Empire can be traced to the fact that, lacking
zero, they were unable to return from their C programs.

2009-02-25 09:36:32

[permalink] [raw]

Subject: Re: ixp4xx_crypto panic with fragmented packets in scatterlist

Christian Hohnstaedt wrote:
> On Tue, Feb 24, 2009 at 02:19:54PM +0100, Karl Hiramoto wrote:
>
>> The latest thing i found is kernel BUG at lib/scatterlist.c:26!
>>
>> ping -s 1800 host in a IPSec tunnel. Setup is 802.1Q VLAN -->
>> ixp4xx_eth --> IPSec Tunnel
>>
>> trace with some debug printk's i added. printk("%s:%d
>> values",__func__, __LINE__);
>>
>>
>> [42949542.170000] esp_output:142 skb->len=1828 data_len=328 clen=1832 alen=0 blksize=8 skb=c7a5cb40 data=c79ba054
>> [42949542.180000] esp_output:168 skb=c7a5cb40 data=c79ba054 clone=0
>> [42949542.180000] __skb_to_sgvec:2430 skb=c7a5cb40 data=c79ba030 sg=c78a0324 off=36 len=1832
>> [42949542.190000] __skb_to_sgvec:2477 list=c795a0c0 start=1536 end=1868 list->len=332 offset=1536
>> [42949542.200000] __skb_to_sgvec:2430 skb=c795a0c0 data=c6d8f048 sg=c78a0338 off=0 len=332
>> [42949542.210000] __skb_to_sgvec:2484 len==0 elt=2
>> [42949542.210000] esp_output:218 skb=c7a5cb40 data=c79ba030
>> [42949542.220000] esp_output:228 skb=c7a5cb40 data=c79ba030
>> [42949542.230000] ablk_perform:897 ivsize=8 nbytes=1840
>> [42949542.230000] ablk_perform:919 dst=NULL src=c78a0278
>> [42949542.240000] count_sg:757 i=0 nbytes=1840 sg=c78a0278 sg_next(sg)=c78a028c
>> [42949542.240000] count_sg:758 len=1508 sg_is_last(sg)=0
>> [42949542.250000] count_sg:757 i=1 nbytes=332 sg=c78a028c sg_next(sg)=c78a02a0
>> [42949542.250000] count_sg:758 len=0 sg_is_last(sg)=0
>> [42949542.260000] kernel BUG at lib/scatterlist.c:26!
>> [42949542.260000] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>>
>
> This differs from the issue you mentioned first.
> The first one was in "dma_cache_maint".
>
>
When i turned on CONFIG_DEBUG_SG i hit the BUG() call in the
scatterlist, instead of the dma_cache_maint..

One thing i've found, is with fragmented packets in crypto/eseqiv.c
eseqiv_chain() scatterwalk_sg_chain(head, 2, sg); is called and
the BUG() occurs.

> Are you sure, your printk()s don't have any side effect ?
>
>
Don't think so, i tried taking them out and had the same issue. I'm
using pr_debug so undefine debug and take them out..

I've also reproduced the same issue.. in IPSec transport and tunnel
modes, with and without vlans, using 3des-cbc or aes-cbc

A simple config like below, and a ping -s 2000 will create a IP
fragment and cause the BUG()

#!/usr/sbin/setkey -f

flush;
spdflush;

add 192.168.10.51 192.168.10.54 esp 0x101 -E aes-cbc
0x000102030405060708090a0b0c0d0e0f;
add 192.168.10.54 192.168.10.51 esp 0x201 -E aes-cbc
0x000102030405060708090a0b0c0d0e0f;

spdadd 192.168.10.51 192.168.10.54 any -P out ipsec
esp/transport//require;

Thanks,

Karl

2009-02-25 11:54:11

[permalink] [raw]

Subject: Re: ixp4xx_crypto panic with fragmented packets in scatterlist

On Wed, Feb 25, 2009 at 10:36:11AM +0100, Karl Hiramoto wrote:
> Christian Hohnstaedt wrote:
> > On Tue, Feb 24, 2009 at 02:19:54PM +0100, Karl Hiramoto wrote:
> >
> >> [42949542.250000] count_sg:758 len=0 sg_is_last(sg)=0
> >> [42949542.260000] kernel BUG at lib/scatterlist.c:26!
> >> [42949542.260000] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> >>
> >
> > This differs from the issue you mentioned first.
> > The first one was in "dma_cache_maint".
> >
> >
> When i turned on CONFIG_DEBUG_SG i hit the BUG() call in the
> scatterlist, instead of the dma_cache_maint..
>
>

looks like there are different, incompatible sg chaining implementations:

include/crypto/scatterwalk.h:scatterwalk_sg_chain() uses
sg->lenght == 0 as indicator for a chained sg

include/linux/scatterlist.h:sg_chain() uses
bit 0 of sg->page_link to indicate chaining

Maybe the matters for b2ab4a57b018aafbba35bff088218f5cc3d2142e
are obsolete now...

However the scatterlist iteration in the arm implementation of
dma_map_sg() uses neither of them, but simply sg++

Please try the attached compile-tested patch.

Christian

--
Hardware
n, "The parts of a computer system that can be kicked."
— Henri Karrenbeld

Attachments:

(No filename) (1.23 kB)
scatterwalk.patch (1.56 kB)
Download all attachments

2009-02-25 14:36:46

[permalink] [raw]

Subject: Re: ixp4xx_crypto panic with fragmented packets in scatterlist

Christian Hohnstaedt wrote:
>
>
> looks like there are different, incompatible sg chaining implementations:
>
> include/crypto/scatterwalk.h:scatterwalk_sg_chain() uses
> sg->lenght == 0 as indicator for a chained sg
>
> include/linux/scatterlist.h:sg_chain() uses
> bit 0 of sg->page_link to indicate chaining
>
> Maybe the matters for b2ab4a57b018aafbba35bff088218f5cc3d2142e
> are obsolete now...
>
> However the scatterlist iteration in the arm implementation of
> dma_map_sg() uses neither of them, but simply sg++
>
> Please try the attached compile-tested patch.
>
>
I had to adapt the change in dma_map_sg() a bit for 2.6.28.7. The
patch fixed the BUG() and kernel panic. But a ping with a
fragmented packet does not get a proper response.

Note: below, the 2nd half of the fragmented packet has the data 0x0000

What i see on the PC host 192.168.10.51 where the IXP434
(192.168.10.54) is doing "ping -s 2000 -c1 192.168.10.51"

tcpdump -i eth0 -vnXX esp
tcpdump: listening on eth0, link-type EN10MB (Ethernet), capture size 96
bytes
15:28:36.373270 IP (tos 0x0, ttl 64, id 26818, offset 0, flags [+],
proto ESP (50), length 1500) 192.168.10.54 > 192.168.10.51:
ESP(spi=0x00000101,seq=0xd), length 1480
0x0000: 0015 c509 9b4a 9a39 d3c6 3651 0800 4500 .....J.9..6Q..E.
0x0010: 05dc 68c2 2000 4032 5674 c0a8 0a36 c0a8 [email protected]..
0x0020: 0a33 0000 0101 0000 000d 0f22 c952 606e .3.........".R`n
0x0030: 3d01 2214 d019 90ae ce7d 7bef 8067 4371 =."......}{..gCq
0x0040: 6404 e937 0054 9835 a09a 080e ea31 b599 d..7.T.5.....1..
0x0050: 353a d9a2 1074 3cb3 d856 0da7 13a9 a6c6 5:...t<..V......
15:28:36.373662 IP (tos 0x0, ttl 64, id 26818, offset 1480, flags
[none], proto ESP (50), length 580) 192.168.10.54 > 192.168.10.51: esp
0x0000: 0015 c509 9b4a 9a39 d3c6 3651 0800 4500 .....J.9..6Q..E.
0x0010: 0244 68c2 00b9 4032 7953 c0a8 0a36 c0a8 [email protected]..
0x0020: 0a33 0000 0000 0000 0000 0000 0000 0000 .3..............
0x0030: 0000 0000 0000 0000 0000 0000 0000 0000 ................
0x0040: 0000 0000 0000 0000 0000 0000 0000 0000 ................
0x0050: 0000 0000 0000 0000 0000 0000 0000 0000 ................
15:28:36.373952 IP (tos 0xc0, ttl 64, id 48218, offset 0, flags [none],
proto ESP (50), length 604) 192.168.10.51 > 192.168.10.54:
ESP(spi=0x00000201,seq=0xf), length 584
0x0000: 9a39 d3c6 3651 0015 c509 9b4a 0800 45c0 .9..6Q.....J..E.
0x0010: 025c bc5a 0000 4032 259c c0a8 0a33 c0a8 .\.Z..@2%....3..
0x0020: 0a36 0000 0201 0000 000f a250 69a6 bf1f .6.........Pi...
0x0030: bdc7 9873 1f25 cc84 b3f9 f1cf 2339 36c6 ...s.%......#96.
0x0040: fd2a 5097 bcef e915 437a 7c8f bc0b 4905 .*P.....Cz|...I.
0x0050: a73b ddb9 1bf9 b54b 6ade c59c ed80 9047 .;.....Kj......G

Thanks

--

Karl

2009-02-25 15:35:54

[permalink] [raw]

Subject: Re:[PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

diff -Naurp linux-2.6.28.7.a/arch/arm/include/asm/scatterlist.h linux-2.6.28.7.b/arch/arm/include/asm/scatterlist.h
--- linux-2.6.28.7.a/arch/arm/include/asm/scatterlist.h 2009-02-20 23:41:27.000000000 +0100
+++ linux-2.6.28.7.b/arch/arm/include/asm/scatterlist.h 2009-02-25 16:19:59.000000000 +0100
@@ -24,4 +24,6 @@ struct scatterlist {
#define sg_dma_address(sg) ((sg)->dma_address)
#define sg_dma_len(sg) ((sg)->length)

+#define ARCH_HAS_SG_CHAIN
+
#endif /* _ASMARM_SCATTERLIST_H */
diff -Naurp linux-2.6.28.7.a/crypto/eseqiv.c linux-2.6.28.7.b/crypto/eseqiv.c
--- linux-2.6.28.7.a/crypto/eseqiv.c 2009-02-20 23:41:27.000000000 +0100
+++ linux-2.6.28.7.b/crypto/eseqiv.c 2009-02-25 16:17:09.000000000 +0100
@@ -17,7 +17,6 @@

#include <crypto/internal/skcipher.h>
#include <crypto/rng.h>
-#include <crypto/scatterwalk.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -67,11 +66,11 @@ static void eseqiv_chain(struct scatterl
{
if (chain) {
head->length += sg->length;
- sg = scatterwalk_sg_next(sg);
+ sg = sg_next(sg);
}

if (sg)
- scatterwalk_sg_chain(head, 2, sg);
+ sg_chain(head, 2, sg);
else
sg_mark_end(head);
}

Attachments:

sg_chain.patch (1.17 kB)

2009-02-26 06:41:18

[permalink] [raw]

Subject: Re: ixp4xx_crypto panic with fragmented packets in scatterlist

Christian Hohnstaedt <[email protected]> wrote:
>
> However the scatterlist iteration in the arm implementation of
> dma_map_sg() uses neither of them, but simply sg++
>
> Please try the attached compile-tested patch.

The chaining is only allowed within the crypto layer (because
we haven't ported the generic chaining to all platforms). So
for the time being all drivers must remove the chaining before
giving the scatter list to an external party, such as the DMA
API.

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-02-26 06:42:34

[permalink] [raw]

Subject: Re: Re:[PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

Karl Hiramoto <[email protected]> wrote:
>
> The attached patch fixes my issue, but am not sure if it is correct or
> will cause problems else where.

This is what we'll do once generic chaining is available on all
architectures. Until then it's up to the driver to fix the SG
list up before passing it to DMA.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-02-26 09:56:27

by Russell King - ARM Linux

[permalink] [raw]

Subject: Re: [PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

On Wed, Feb 25, 2009 at 04:35:40PM +0100, Karl Hiramoto wrote:
> The attached patch fixes my issue, but am not sure if it is correct or
> will cause problems else where.

> diff -Naurp linux-2.6.28.7.a/arch/arm/include/asm/scatterlist.h linux-2.6.28.7.b/arch/arm/include/asm/scatterlist.h
> --- linux-2.6.28.7.a/arch/arm/include/asm/scatterlist.h 2009-02-20 23:41:27.000000000 +0100
> +++ linux-2.6.28.7.b/arch/arm/include/asm/scatterlist.h 2009-02-25 16:19:59.000000000 +0100
> @@ -24,4 +24,6 @@ struct scatterlist {
> #define sg_dma_address(sg) ((sg)->dma_address)
> #define sg_dma_len(sg) ((sg)->length)
>
> +#define ARCH_HAS_SG_CHAIN
> +

We can't merge this until _all_ of ARM has been fixed for walking
scatterlist chains.

2009-02-26 12:10:56

[permalink] [raw]

Subject: Re: [PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

Russell King - ARM Linux <[email protected]> wrote:
>
> We can't merge this until _all_ of ARM has been fixed for walking
> scatterlist chains.

Right, this is definitely not the way to fix this bug. Because
even if ARM completely supported chaining, you still have to fix
all the other architectures as well.

So please just fix the driver to remove the chaining before doing
DMA.

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-02-26 20:27:11

[permalink] [raw]

Subject: Re: [PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

Herbert Xu wrote:
> Russell King - ARM Linux <[email protected]> wrote:
>
>> We can't merge this until _all_ of ARM has been fixed for walking
>> scatterlist chains.
>>
>
> Right, this is definitely not the way to fix this bug. Because
> even if ARM completely supported chaining, you still have to fix
> all the other architectures as well.
>
> So please just fix the driver to remove the chaining before doing
> DMA.
>
>
Is there an example of how to do this somewhere?

Thanks

Karl

2009-02-26 23:20:50

by Russell King - ARM Linux

[permalink] [raw]

Subject: Re: [PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

On Thu, Feb 26, 2009 at 08:10:43PM +0800, Herbert Xu wrote:
> Russell King - ARM Linux <[email protected]> wrote:
> >
> > We can't merge this until _all_ of ARM has been fixed for walking
> > scatterlist chains.
>
> Right, this is definitely not the way to fix this bug. Because
> even if ARM completely supported chaining, you still have to fix
> all the other architectures as well.

I don't think you can use chained scatterlists unless the architecture
supports it. It's not a case that you have to flatten the chaining
before passing it over to the arch - it seems you're not allowed to
create a chained scatterlist in the first place:

static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
struct scatterlist *sgl)
{
#ifndef ARCH_HAS_SG_CHAIN
BUG();
#endif

2009-02-27 00:50:48

[permalink] [raw]

Subject: Re: [PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

Russell King - ARM Linux <[email protected]> wrote:
>
> I don't think you can use chained scatterlists unless the architecture
> supports it. It's not a case that you have to flatten the chaining
> before passing it over to the arch - it seems you're not allowed to
> create a chained scatterlist in the first place:
>
> static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
> struct scatterlist *sgl)
> {
> #ifndef ARCH_HAS_SG_CHAIN
> BUG();
> #endif

The crypto layer does its own chaining. However, this chaining
is not meant to be exposed to any external entities until generic
chaining has been ported everywhere.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-02-27 10:26:15

[permalink] [raw]

Subject: Re: [PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

On Thu, Feb 26, 2009 at 09:27:09PM +0100, Karl Hiramoto wrote:
> Herbert Xu wrote:
>> Russell King - ARM Linux <[email protected]> wrote:
>>
>>> We can't merge this until _all_ of ARM has been fixed for walking
>>> scatterlist chains.
>>>
>>
>> Right, this is definitely not the way to fix this bug. Because
>> even if ARM completely supported chaining, you still have to fix
>> all the other architectures as well.
>>
>> So please just fix the driver to remove the chaining before doing
>> DMA.
>>
>>
> Is there an example of how to do this somewhere?
>

Not needed.

Now that I know the details about the different ways of chaining
and their scope, I know how to fix my driver properly.

Patch follows, soon...

best regards
Christian

--
The iMac is just evidence of how dangerous vi is. Obviously Steve came up with
the name by accident after forgetting he was *already* in insert mode.
— dagbrown on #emacs

2009-03-02 11:45:17

[permalink] [raw]

Subject: [PATCH] crypto: fix handling of sg buffers in ixp4xx driver

- keep dma functions away from chained scatterlists.
Use the existing scatterlist iteration inside the driver
to call dma_map_single() for each chunk and avoid dma_map_sg().

Signed-off-by: Christian Hohnstaedt <[email protected]>
Tested-By: Karl Hiramoto <[email protected]>

---
drivers/crypto/ixp4xx_crypto.c | 182 ++++++++++++++--------------------------
1 files changed, 63 insertions(+), 119 deletions(-)

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index 2d637e0..fdcd0ab 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -101,6 +101,7 @@ struct buffer_desc {
u32 phys_addr;
u32 __reserved[4];
struct buffer_desc *next;
+ enum dma_data_direction dir;
};

struct crypt_ctl {
@@ -132,14 +133,10 @@ struct crypt_ctl {
struct ablk_ctx {
struct buffer_desc *src;
struct buffer_desc *dst;
- unsigned src_nents;
- unsigned dst_nents;
};

struct aead_ctx {
struct buffer_desc *buffer;
- unsigned short assoc_nents;
- unsigned short src_nents;
struct scatterlist ivlist;
/* used when the hmac is not on one sg entry */
u8 *hmac_virt;
@@ -312,7 +309,7 @@ static struct crypt_ctl *get_crypt_desc_emerg(void)
}
}

-static void free_buf_chain(struct buffer_desc *buf, u32 phys)
+static void free_buf_chain(struct device *dev, struct buffer_desc *buf,u32 phys)
{
while (buf) {
struct buffer_desc *buf1;
@@ -320,6 +317,7 @@ static void free_buf_chain(struct buffer_desc *buf, u32 phys)

buf1 = buf->next;
phys1 = buf->phys_next;
+ dma_unmap_single(dev, buf->phys_next, buf->buf_len, buf->dir);
dma_pool_free(buffer_pool, buf, phys);
buf = buf1;
phys = phys1;
@@ -348,7 +346,6 @@ static void one_packet(dma_addr_t phys)
struct crypt_ctl *crypt;
struct ixp_ctx *ctx;
int failed;
- enum dma_data_direction src_direction = DMA_BIDIRECTIONAL;

failed = phys & 0x1 ? -EBADMSG : 0;
phys &= ~0x3;
@@ -358,13 +355,8 @@ static void one_packet(dma_addr_t phys)
case CTL_FLAG_PERFORM_AEAD: {
struct aead_request *req = crypt->data.aead_req;
struct aead_ctx *req_ctx = aead_request_ctx(req);
- dma_unmap_sg(dev, req->assoc, req_ctx->assoc_nents,
- DMA_TO_DEVICE);
- dma_unmap_sg(dev, &req_ctx->ivlist, 1, DMA_BIDIRECTIONAL);
- dma_unmap_sg(dev, req->src, req_ctx->src_nents,
- DMA_BIDIRECTIONAL);

- free_buf_chain(req_ctx->buffer, crypt->src_buf);
+ free_buf_chain(dev, req_ctx->buffer, crypt->src_buf);
if (req_ctx->hmac_virt) {
finish_scattered_hmac(crypt);
}
@@ -374,16 +366,11 @@ static void one_packet(dma_addr_t phys)
case CTL_FLAG_PERFORM_ABLK: {
struct ablkcipher_request *req = crypt->data.ablk_req;
struct ablk_ctx *req_ctx = ablkcipher_request_ctx(req);
- int nents;
+
if (req_ctx->dst) {
- nents = req_ctx->dst_nents;
- dma_unmap_sg(dev, req->dst, nents, DMA_FROM_DEVICE);
- free_buf_chain(req_ctx->dst, crypt->dst_buf);
- src_direction = DMA_TO_DEVICE;
+ free_buf_chain(dev, req_ctx->dst, crypt->dst_buf);
}
- nents = req_ctx->src_nents;
- dma_unmap_sg(dev, req->src, nents, src_direction);
- free_buf_chain(req_ctx->src, crypt->src_buf);
+ free_buf_chain(dev, req_ctx->src, crypt->src_buf);
req->base.complete(&req->base, failed);
break;
}
@@ -748,56 +735,35 @@ static int setup_cipher(struct crypto_tfm *tfm, int encrypt,
return 0;
}

-static int count_sg(struct scatterlist *sg, int nbytes)
+static struct buffer_desc *chainup_buffers(struct device *dev,
+ struct scatterlist *sg, unsigned nbytes,
+ struct buffer_desc *buf, gfp_t flags,
+ enum dma_data_direction dir)
{
- int i;
- for (i = 0; nbytes > 0; i++, sg = sg_next(sg))
- nbytes -= sg->length;
- return i;
-}
-
-static struct buffer_desc *chainup_buffers(struct scatterlist *sg,
- unsigned nbytes, struct buffer_desc *buf, gfp_t flags)
-{
- int nents = 0;
-
- while (nbytes > 0) {
+ for (;nbytes > 0; sg = scatterwalk_sg_next(sg)) {
+ unsigned len = min(nbytes, sg->length);
struct buffer_desc *next_buf;
u32 next_buf_phys;
- unsigned len = min(nbytes, sg_dma_len(sg));
+ void *ptr;

- nents++;
nbytes -= len;
- if (!buf->phys_addr) {
- buf->phys_addr = sg_dma_address(sg);
- buf->buf_len = len;
- buf->next = NULL;
- buf->phys_next = 0;
- goto next;
- }
- /* Two consecutive chunks on one page may be handled by the old
- * buffer descriptor, increased by the length of the new one
- */
- if (sg_dma_address(sg) == buf->phys_addr + buf->buf_len) {
- buf->buf_len += len;
- goto next;
- }
+ ptr = page_address(sg_page(sg)) + sg->offset;
next_buf = dma_pool_alloc(buffer_pool, flags, &next_buf_phys);
- if (!next_buf)
- return NULL;
+ if (!next_buf) {
+ buf = NULL;
+ break;
+ }
+ sg_dma_address(sg) = dma_map_single(dev, ptr, len, dir);
buf->next = next_buf;
buf->phys_next = next_buf_phys;
-
buf = next_buf;
- buf->next = NULL;
- buf->phys_next = 0;
+
buf->phys_addr = sg_dma_address(sg);
buf->buf_len = len;
-next:
- if (nbytes > 0) {
- sg = sg_next(sg);
- }
+ buf->dir = dir;
}
+ buf->next = NULL;
+ buf->phys_next = 0;
return buf;
}

@@ -858,12 +824,12 @@ static int ablk_perform(struct ablkcipher_request *req, int encrypt)
struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
struct ixp_ctx *ctx = crypto_ablkcipher_ctx(tfm);
unsigned ivsize = crypto_ablkcipher_ivsize(tfm);
- int ret = -ENOMEM;
struct ix_sa_dir *dir;
struct crypt_ctl *crypt;
- unsigned int nbytes = req->nbytes, nents;
+ unsigned int nbytes = req->nbytes;
enum dma_data_direction src_direction = DMA_BIDIRECTIONAL;
struct ablk_ctx *req_ctx = ablkcipher_request_ctx(req);
+ struct buffer_desc src_hook;
gfp_t flags = req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP ?
GFP_KERNEL : GFP_ATOMIC;

@@ -876,7 +842,7 @@ static int ablk_perform(struct ablkcipher_request *req, int encrypt)

crypt = get_crypt_desc();
if (!crypt)
- return ret;
+ return -ENOMEM;

crypt->data.ablk_req = req;
crypt->crypto_ctx = dir->npe_ctx_phys;
@@ -889,53 +855,41 @@ static int ablk_perform(struct ablkcipher_request *req, int encrypt)
BUG_ON(ivsize && !req->info);
memcpy(crypt->iv, req->info, ivsize);
if (req->src != req->dst) {
+ struct buffer_desc dst_hook;
crypt->mode |= NPE_OP_NOT_IN_PLACE;
- nents = count_sg(req->dst, nbytes);
/* This was never tested by Intel
* for more than one dst buffer, I think. */
- BUG_ON(nents != 1);
- req_ctx->dst_nents = nents;
- dma_map_sg(dev, req->dst, nents, DMA_FROM_DEVICE);
- req_ctx->dst = dma_pool_alloc(buffer_pool, flags,&crypt->dst_buf);
- if (!req_ctx->dst)
- goto unmap_sg_dest;
- req_ctx->dst->phys_addr = 0;
- if (!chainup_buffers(req->dst, nbytes, req_ctx->dst, flags))
+ BUG_ON(req->dst->length < nbytes);
+ req_ctx->dst = NULL;
+ if (!chainup_buffers(dev, req->dst, nbytes, &dst_hook,
+ flags, DMA_FROM_DEVICE))
goto free_buf_dest;
src_direction = DMA_TO_DEVICE;
+ req_ctx->dst = dst_hook.next;
+ crypt->dst_buf = dst_hook.phys_next;
} else {
req_ctx->dst = NULL;
- req_ctx->dst_nents = 0;
}
- nents = count_sg(req->src, nbytes);
- req_ctx->src_nents = nents;
- dma_map_sg(dev, req->src, nents, src_direction);
-
- req_ctx->src = dma_pool_alloc(buffer_pool, flags, &crypt->src_buf);
- if (!req_ctx->src)
- goto unmap_sg_src;
- req_ctx->src->phys_addr = 0;
- if (!chainup_buffers(req->src, nbytes, req_ctx->src, flags))
+ req_ctx->src = NULL;
+ if (!chainup_buffers(dev, req->src, nbytes, &src_hook,
+ flags, src_direction))
goto free_buf_src;

+ req_ctx->src = src_hook.next;
+ crypt->src_buf = src_hook.phys_next;
crypt->ctl_flags |= CTL_FLAG_PERFORM_ABLK;
qmgr_put_entry(SEND_QID, crypt_virt2phys(crypt));
BUG_ON(qmgr_stat_overflow(SEND_QID));
return -EINPROGRESS;

free_buf_src:
- free_buf_chain(req_ctx->src, crypt->src_buf);
-unmap_sg_src:
- dma_unmap_sg(dev, req->src, req_ctx->src_nents, src_direction);
+ free_buf_chain(dev, req_ctx->src, crypt->src_buf);
free_buf_dest:
if (req->src != req->dst) {
- free_buf_chain(req_ctx->dst, crypt->dst_buf);
-unmap_sg_dest:
- dma_unmap_sg(dev, req->src, req_ctx->dst_nents,
- DMA_FROM_DEVICE);
+ free_buf_chain(dev, req_ctx->dst, crypt->dst_buf);
}
crypt->ctl_flags = CTL_FLAG_UNUSED;
- return ret;
+ return -ENOMEM;
}

static int ablk_encrypt(struct ablkcipher_request *req)
@@ -983,7 +937,7 @@ static int hmac_inconsistent(struct scatterlist *sg, unsigned start,
break;

offset += sg->length;
- sg = sg_next(sg);
+ sg = scatterwalk_sg_next(sg);
}
return (start + nbytes > offset + sg->length);
}
@@ -995,11 +949,10 @@ static int aead_perform(struct aead_request *req, int encrypt,
struct ixp_ctx *ctx = crypto_aead_ctx(tfm);
unsigned ivsize = crypto_aead_ivsize(tfm);
unsigned authsize = crypto_aead_authsize(tfm);
- int ret = -ENOMEM;
struct ix_sa_dir *dir;
struct crypt_ctl *crypt;
- unsigned int cryptlen, nents;
- struct buffer_desc *buf;
+ unsigned int cryptlen;
+ struct buffer_desc *buf, src_hook;
struct aead_ctx *req_ctx = aead_request_ctx(req);
gfp_t flags = req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP ?
GFP_KERNEL : GFP_ATOMIC;
@@ -1020,7 +973,7 @@ static int aead_perform(struct aead_request *req, int encrypt,
}
crypt = get_crypt_desc();
if (!crypt)
- return ret;
+ return -ENOMEM;

crypt->data.aead_req = req;
crypt->crypto_ctx = dir->npe_ctx_phys;
@@ -1039,31 +992,27 @@ static int aead_perform(struct aead_request *req, int encrypt,
BUG(); /* -ENOTSUP because of my lazyness */
}

- req_ctx->buffer = dma_pool_alloc(buffer_pool, flags, &crypt->src_buf);
- if (!req_ctx->buffer)
- goto out;
- req_ctx->buffer->phys_addr = 0;
/* ASSOC data */
- nents = count_sg(req->assoc, req->assoclen);
- req_ctx->assoc_nents = nents;
- dma_map_sg(dev, req->assoc, nents, DMA_TO_DEVICE);
- buf = chainup_buffers(req->assoc, req->assoclen, req_ctx->buffer,flags);
+ buf = chainup_buffers(dev, req->assoc, req->assoclen, &src_hook,
+ flags, DMA_TO_DEVICE);
+ req_ctx->buffer = src_hook.next;
+ crypt->src_buf = src_hook.phys_next;
if (!buf)
- goto unmap_sg_assoc;
+ goto out;
/* IV */
sg_init_table(&req_ctx->ivlist, 1);
sg_set_buf(&req_ctx->ivlist, iv, ivsize);
- dma_map_sg(dev, &req_ctx->ivlist, 1, DMA_BIDIRECTIONAL);
- buf = chainup_buffers(&req_ctx->ivlist, ivsize, buf, flags);
+ buf = chainup_buffers(dev, &req_ctx->ivlist, ivsize, buf, flags,
+ DMA_BIDIRECTIONAL);
if (!buf)
- goto unmap_sg_iv;
+ goto free_chain;
if (unlikely(hmac_inconsistent(req->src, cryptlen, authsize))) {
/* The 12 hmac bytes are scattered,
* we need to copy them into a safe buffer */
req_ctx->hmac_virt = dma_pool_alloc(buffer_pool, flags,
&crypt->icv_rev_aes);
if (unlikely(!req_ctx->hmac_virt))
- goto unmap_sg_iv;
+ goto free_chain;
if (!encrypt) {
scatterwalk_map_and_copy(req_ctx->hmac_virt,
req->src, cryptlen, authsize, 0);
@@ -1073,33 +1022,28 @@ static int aead_perform(struct aead_request *req, int encrypt,
req_ctx->hmac_virt = NULL;
}
/* Crypt */
- nents = count_sg(req->src, cryptlen + authsize);
- req_ctx->src_nents = nents;
- dma_map_sg(dev, req->src, nents, DMA_BIDIRECTIONAL);
- buf = chainup_buffers(req->src, cryptlen + authsize, buf, flags);
+ buf = chainup_buffers(dev, req->src, cryptlen + authsize, buf, flags,
+ DMA_BIDIRECTIONAL);
if (!buf)
- goto unmap_sg_src;
+ goto free_hmac_virt;
if (!req_ctx->hmac_virt) {
crypt->icv_rev_aes = buf->phys_addr + buf->buf_len - authsize;
}
+
crypt->ctl_flags |= CTL_FLAG_PERFORM_AEAD;
qmgr_put_entry(SEND_QID, crypt_virt2phys(crypt));
BUG_ON(qmgr_stat_overflow(SEND_QID));
return -EINPROGRESS;
-unmap_sg_src:
- dma_unmap_sg(dev, req->src, req_ctx->src_nents, DMA_BIDIRECTIONAL);
+free_hmac_virt:
if (req_ctx->hmac_virt) {
dma_pool_free(buffer_pool, req_ctx->hmac_virt,
crypt->icv_rev_aes);
}
-unmap_sg_iv:
- dma_unmap_sg(dev, &req_ctx->ivlist, 1, DMA_BIDIRECTIONAL);
-unmap_sg_assoc:
- dma_unmap_sg(dev, req->assoc, req_ctx->assoc_nents, DMA_TO_DEVICE);
- free_buf_chain(req_ctx->buffer, crypt->src_buf);
+free_chain:
+ free_buf_chain(dev, req_ctx->buffer, crypt->src_buf);
out:
crypt->ctl_flags = CTL_FLAG_UNUSED;
- return ret;
+ return -ENOMEM;
}

static int aead_setup(struct crypto_aead *tfm, unsigned int authsize)
--
1.6.0.3

2009-03-02 20:43:03

by Russell King - ARM Linux

[permalink] [raw]

Subject: Re: [PATCH] crypto: fix handling of sg buffers in ixp4xx driver

On Mon, Mar 02, 2009 at 12:45:12PM +0100, Christian Hohnstaedt wrote:
>
> - keep dma functions away from chained scatterlists.
> Use the existing scatterlist iteration inside the driver
> to call dma_map_single() for each chunk and avoid dma_map_sg().

Hmm, interesting. So crypto has its own scatterlist chaining stuff which
is different and incompatible from what is supported by the rest of the
kernel.

That's really confusing. There's know way to know whether a struct scatterlist
should be walked by using the scatterwalk_* stuff or the generic stuff.
Excellent for reviewing.

So, sorry, I'm not qualified to review this. Please find someone else
who knows about crypto stuff.

2009-03-03 03:02:34

[permalink] [raw]

Subject: Re: [PATCH] ixp4xx_crypto panic with fragmented packets in scatterlist

On Thu, Feb 26, 2009 at 11:20:26PM +0000, Russell King - ARM Linux wrote:
>
> I don't think you can use chained scatterlists unless the architecture
> supports it. It's not a case that you have to flatten the chaining
> before passing it over to the arch - it seems you're not allowed to
> create a chained scatterlist in the first place:
>
> static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
> struct scatterlist *sgl)
> {
> #ifndef ARCH_HAS_SG_CHAIN
> BUG();
> #endif

That's why we don't use sg_chain. The chaining is completely
internal to the crypto API (or at least it's supposed to be).

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-03-03 03:02:58

[permalink] [raw]

Subject: Re: [PATCH] crypto: fix handling of sg buffers in ixp4xx driver

On Mon, Mar 02, 2009 at 08:42:39PM +0000, Russell King - ARM Linux wrote:
>
> So, sorry, I'm not qualified to review this. Please find someone else
> who knows about crypto stuff.

No worries, I'm onto it.

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2009-03-27 07:09:57