* Crypto causes panic in scatterwalk_done with large/multiple buffers @ 2012-11-15 9:03 Jorgen Lundman 2012-11-17 0:42 ` Jussi Kivilinna 0 siblings, 1 reply; 7+ messages in thread From: Jorgen Lundman @ 2012-11-15 9:03 UTC (permalink / raw) To: linux-crypto I have a situation where I setup scatterlists as: input scatterlist of 1, address ffffc90003627000 len 0x20000. output scatterlist of 2, address 0 ffffc90002d45000 len 0x20000 address 1 ffff88003b079d98 len 0x000c When I call crypto_aead_encrypt(req); it will die with: kernel: [ 925.151113] BUG: unable to handle kernel paging request at ffffeb04000b5140 kernel: [ 925.151253] IP: [<ffffffff812f4880>] scatterwalk_done+0x50/0x60 kernel: [ 925.151325] PGD 0 kernel: [ 925.151381] Oops: 0000 [#1] SMP kernel: [ 925.151442] CPU 1 kernel: [ 925.154255] [<ffffffff812f7640>] blkcipher_walk_done+0xb0/0x230 kernel: [ 925.154255] [<ffffffffa02e9169>] crypto_ctr_crypt+0x129/0x2b0 [ctr] kernel: [ 925.154255] [<ffffffff812fe580>] ? crypto_aes_set_key+0x40/0x40 kernel: [ 925.154255] [<ffffffff812f6cbd>] async_encrypt+0x3d/0x40 kernel: [ 925.154255] [<ffffffffa0149326>] crypto_ccm_encrypt+0x246/0x290 [ccm] kernel: [ 925.154255] [<ffffffffa01633bd>] crypto_encrypt+0x26d/0x2d0 What is interesting about that is, if I allocate a linear buffer instead: dst = kmalloc(cryptlen, GFP_KERNEL); // 0x20000 + 0x000c sg_init_table(sg, 1 ); sg_set_buf(&sg[0], dst, cryptlen); crypto_aead_encrypt(req); will no longer panic. However, when I try to copy the linear buffer back to scatterlist; scatterwalk_map_and_copy(dst, sg, 0, cryptlen, 1); then it will panic there instead. However, if I replace it with the call: sg_copy_from_buffer(sg, sg_nents(sg), dst, cryptlen); everything works! <- So, what am I doing wrong that makes scatterwalk_map_and_copy() fail, and sg_copy_from_buffer() work fine? It would be nice if I could fix it, so I did not need to copy to a temporary buffer. Lund -- Jorgen Lundman | <lundman@lundman.net> Unix Administrator | +81 (0)3 -5456-2687 ext 1017 (work) Shibuya-ku, Tokyo | +81 (0)90-5578-8500 (cell) Japan | +81 (0)3 -3375-1767 (home) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crypto causes panic in scatterwalk_done with large/multiple buffers 2012-11-15 9:03 Crypto causes panic in scatterwalk_done with large/multiple buffers Jorgen Lundman @ 2012-11-17 0:42 ` Jussi Kivilinna 2012-11-17 8:39 ` Jussi Kivilinna 0 siblings, 1 reply; 7+ messages in thread From: Jussi Kivilinna @ 2012-11-17 0:42 UTC (permalink / raw) To: Jorgen Lundman; +Cc: linux-crypto [-- Attachment #1: Type: text/plain, Size: 2484 bytes --] Hello, I managed to reproduce something similiar with small buffers... Does attached patch help in your case? -Jussi Quoting Jorgen Lundman <lundman@lundman.net>: > > I have a situation where I setup scatterlists as: > > input scatterlist of 1, address ffffc90003627000 len 0x20000. > > output scatterlist of 2, address 0 ffffc90002d45000 len 0x20000 > address 1 ffff88003b079d98 len 0x000c > > When I call crypto_aead_encrypt(req); it will die with: > > kernel: [ 925.151113] BUG: unable to handle kernel paging request at > ffffeb04000b5140 > kernel: [ 925.151253] IP: [<ffffffff812f4880>] scatterwalk_done+0x50/0x60 > kernel: [ 925.151325] PGD 0 > kernel: [ 925.151381] Oops: 0000 [#1] SMP > kernel: [ 925.151442] CPU 1 > kernel: [ 925.154255] [<ffffffff812f7640>] blkcipher_walk_done+0xb0/0x230 > kernel: [ 925.154255] [<ffffffffa02e9169>] > crypto_ctr_crypt+0x129/0x2b0 [ctr] > kernel: [ 925.154255] [<ffffffff812fe580>] ? crypto_aes_set_key+0x40/0x40 > kernel: [ 925.154255] [<ffffffff812f6cbd>] async_encrypt+0x3d/0x40 > kernel: [ 925.154255] [<ffffffffa0149326>] crypto_ccm_encrypt+0x246/0x290 > [ccm] > kernel: [ 925.154255] [<ffffffffa01633bd>] crypto_encrypt+0x26d/0x2d0 > > > > What is interesting about that is, if I allocate a linear buffer instead: > > dst = kmalloc(cryptlen, GFP_KERNEL); // 0x20000 + 0x000c > sg_init_table(sg, 1 ); > sg_set_buf(&sg[0], dst, cryptlen); > > crypto_aead_encrypt(req); > > will no longer panic. However, when I try to copy the linear buffer back to > scatterlist; > > > scatterwalk_map_and_copy(dst, sg, 0, cryptlen, 1); > > > then it will panic there instead. > > > However, if I replace it with the call: > > sg_copy_from_buffer(sg, sg_nents(sg), > dst, cryptlen); > > everything works! <- > > So, what am I doing wrong that makes scatterwalk_map_and_copy() fail, and > sg_copy_from_buffer() work fine? It would be nice if I could fix it, so I > did not need to copy to a temporary buffer. > > Lund > > -- > Jorgen Lundman | <lundman@lundman.net> > Unix Administrator | +81 (0)3 -5456-2687 ext 1017 (work) > Shibuya-ku, Tokyo | +81 (0)90-5578-8500 (cell) > Japan | +81 (0)3 -3375-1767 (home) > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > [-- Attachment #2: 05-fix-broken-scatterwalk_sg_chain.patch --] [-- Type: text/x-patch, Size: 1271 bytes --] crypto: scatterwalk - fix broken scatterlist manipulation From: Jussi Kivilinna <jussi.kivilinna@mbnet.fi> scatterlist_sg_chain() manipulates scatterlist structures directly in wrong way, chaining without marking 'chain' bit 0x01. This can in some cases lead to problems, such as triggering BUG_ON(!sg->length) in scatterwalk_start(). So instead of reinventing wheel, change scatterwalk to use existing functions from scatterlist API. --- include/crypto/scatterwalk.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/include/crypto/scatterwalk.h b/include/crypto/scatterwalk.h index 3744d2a..d31870c 100644 --- a/include/crypto/scatterwalk.h +++ b/include/crypto/scatterwalk.h @@ -34,16 +34,12 @@ static inline void crypto_yield(u32 flags) static inline void scatterwalk_sg_chain(struct scatterlist *sg1, int num, struct scatterlist *sg2) { - sg_set_page(&sg1[num - 1], (void *)sg2, 0, 0); - sg1[num - 1].page_link &= ~0x02; + sg_chain(sg1, num, sg2); } static inline struct scatterlist *scatterwalk_sg_next(struct scatterlist *sg) { - if (sg_is_last(sg)) - return NULL; - - return (++sg)->length ? sg : (void *)sg_page(sg); + return sg_next(sg); } static inline void scatterwalk_crypto_chain(struct scatterlist *head, ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Crypto causes panic in scatterwalk_done with large/multiple buffers 2012-11-17 0:42 ` Jussi Kivilinna @ 2012-11-17 8:39 ` Jussi Kivilinna 2012-11-18 0:29 ` Jorgen Lundman 0 siblings, 1 reply; 7+ messages in thread From: Jussi Kivilinna @ 2012-11-17 8:39 UTC (permalink / raw) To: Jussi Kivilinna; +Cc: Jorgen Lundman, linux-crypto Quoting Jussi Kivilinna <jussi.kivilinna@mbnet.fi>: > Hello, > > I managed to reproduce something similiar with small buffers... > > Does attached patch help in your case? > Appearently this patch only fixed my debug printk loop that used sg_next from scatterlist API instead of scatterwalk_sg_next from scatterwalk API. Sorry for the noise. -Jussi ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crypto causes panic in scatterwalk_done with large/multiple buffers 2012-11-17 8:39 ` Jussi Kivilinna @ 2012-11-18 0:29 ` Jorgen Lundman 2012-11-18 2:10 ` Jorgen Lundman 2012-11-18 17:40 ` Jussi Kivilinna 0 siblings, 2 replies; 7+ messages in thread From: Jorgen Lundman @ 2012-11-18 0:29 UTC (permalink / raw) To: linux-crypto; +Cc: Jussi Kivilinna > > Appearently this patch only fixed my debug printk loop that used sg_next > from scatterlist API instead of scatterwalk_sg_next from scatterwalk API. > Sorry for the noise. > Thanks for looking at this. I think I am dealing with 2 problems, one is that occasionally my buffers are from vmalloc, and needs to have some logic using vmalloc_to_page(). But I don't know if ciphers should handle that internally, blkcipher.c certainly seems to have several modes, although I do not see how to *set* them. Second problem is most likely what you were looking at. It is quite easy to make the crypto code die. For example, if I use "ccm(aes)" which can take the dst buffer, plus a hmac buffer; cipher = kmalloc( ciphersize, ... hmac = kmalloc( 16, ... sg_set_buf( &sg[0], cipher, ciphersize); sg_set_buf( &sg[1], hmac, 16); aead_encrypt()... and all is well, but if you shift hmac address away from PAGE boundary, like: hmac = kmalloc( 16 + 32, ... hmac += 32; sg_set_buf( &sg[1], hmac, 16); ie, allocate a larger buffer, and put the pointer into the page a bit. And it will die in scatterwalk very often. +32 isnt magical, any non-zero number works. Lund -- Jorgen Lundman | <lundman@lundman.net> Unix Administrator | +81 (0)3 -5456-2687 ext 1017 (work) Shibuya-ku, Tokyo | +81 (0)90-5578-8500 (cell) Japan | +81 (0)3 -3375-1767 (home) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crypto causes panic in scatterwalk_done with large/multiple buffers 2012-11-18 0:29 ` Jorgen Lundman @ 2012-11-18 2:10 ` Jorgen Lundman 2012-11-18 17:40 ` Jussi Kivilinna 1 sibling, 0 replies; 7+ messages in thread From: Jorgen Lundman @ 2012-11-18 2:10 UTC (permalink / raw) To: linux-crypto Specifically, the crash I get is finally in here: static void Xscatterwalk_pagedone(struct scatter_walk *walk, int out, unsigned int more) { if (out) { struct page *page; page = sg_page(walk->sg) + ((walk->offset - 1) >> PAGE_SHIFT); printk("**** PageSlab: page %p offset 0x%04lx\n", page, walk->offset); if (!PageSlab(page)) { // <===== here kernel: [ 62.650590] **** PageSlab: page ffffeb04000b7e40 offset 0x1000 Commenting out the call to PageSlab and flush_dcache_page(page); surprisingly makes it work a little better. But presumably it comes with a cost that I can not yet see. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crypto causes panic in scatterwalk_done with large/multiple buffers 2012-11-18 0:29 ` Jorgen Lundman 2012-11-18 2:10 ` Jorgen Lundman @ 2012-11-18 17:40 ` Jussi Kivilinna 2012-11-19 0:29 ` Jorgen Lundman 1 sibling, 1 reply; 7+ messages in thread From: Jussi Kivilinna @ 2012-11-18 17:40 UTC (permalink / raw) To: Jorgen Lundman; +Cc: linux-crypto Quoting Jorgen Lundman <lundman@lundman.net>: >> >> Appearently this patch only fixed my debug printk loop that used sg_next >> from scatterlist API instead of scatterwalk_sg_next from scatterwalk API. >> Sorry for the noise. >> > > Thanks for looking at this. I think I am dealing with 2 problems, one is > that occasionally my buffers are from vmalloc, and needs to have some logic > using vmalloc_to_page(). But I don't know if ciphers should handle that > internally, blkcipher.c certainly seems to have several modes, although I > do not see how to *set* them. From what I now researched, you must not pass vmalloc'd memory to sg_set_buf() as it internally uses virt_to_page() to get page of buffer address. You most likely need to walk through your vmalloc'd buffer and pass all individual pages to scatterlist with sg_set_page(). > > Second problem is most likely what you were looking at. It is quite easy to > make the crypto code die. > > For example, if I use "ccm(aes)" which can take the dst buffer, plus a hmac > buffer; > > cipher = kmalloc( ciphersize, ... > hmac = kmalloc( 16, ... > sg_set_buf( &sg[0], cipher, ciphersize); > sg_set_buf( &sg[1], hmac, 16); > aead_encrypt()... > > and all is well, but if you shift hmac address away from PAGE boundary, like: > > hmac = kmalloc( 16 + 32, ... > hmac += 32; > sg_set_buf( &sg[1], hmac, 16); > > ie, allocate a larger buffer, and put the pointer into the page a bit. And > it will die in scatterwalk very often. +32 isnt magical, any non-zero > number works. This is strange as crypto subsystem's internal test mechanism uses such offsetted buffers. -Jussi ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crypto causes panic in scatterwalk_done with large/multiple buffers 2012-11-18 17:40 ` Jussi Kivilinna @ 2012-11-19 0:29 ` Jorgen Lundman 0 siblings, 0 replies; 7+ messages in thread From: Jorgen Lundman @ 2012-11-19 0:29 UTC (permalink / raw) Cc: Jussi Kivilinna, linux-crypto > > From what I now researched, you must not pass vmalloc'd memory to > sg_set_buf() as it internally uses virt_to_page() to get page of buffer > address. You most likely need to walk through your vmalloc'd buffer and > pass all individual pages to scatterlist with sg_set_page(). I was told the same, so I wrote something that called vmalloc_to_page() for each PAGE_SIZE of the vmalloced memory, with offsets, and created large scatterlists (well, around ~120 or so). Made no difference at all, still dies in scatterwalk_done. Looking at blkcipher (ccm->ctr->blkcipher) and I see it has methods for _phys(), _virt(), _virt_block(), _fast(), _slow() and _copy(). I was quite interested in the copy() method, since that is what I am doing at the top. No idea how I affect those types of use. phys and virt seem to be handled internally, and virt_block() is what ctr.c uses. This made me think the vmalloc thing isn't my problem. Then I copied ccm.c, ctr.c and blkcipher.c to my own module, just so that I could commend out: static void Xscatterwalk_pagedone(struct scatter_walk *walk, int out, unsigned int more) { if (out) { struct page *page; page = sg_page(walk->sg) + ((walk->offset - 1) >> PAGE_SHIFT); #if 0 /* * This call causes panic, so all these sources are just to * avoid this call. * * Once this issue has been fixed, we can remove the entire * "sun-ccm(aes)" module. * */ if (!PageSlab(page)) flush_dcache_page(page); #endif } if (more) { And making the two functions, Xscatterwalk_done() and Xscatterwalk_copychunks(), to call my version instead. Wouldn't you know, all panics are gone. The misaligned buffer panic, gone. Reverted back to just passing vmalloc() memory, no problems. I have been running multi-gig bonnie++ for 24 hours (file system related crypto). I am trusting that it is working now. Tested with 3.2.0-0.bpo.3-amd64 and 3.5.0-17-generic. > This is strange as crypto subsystem's internal test mechanism uses such > offsetted buffers. > I'm sure it does. I run my tests about 100 times and it usually dies around ~40th. I appreciate your help, Lund -- Jorgen Lundman | <lundman@lundman.net> Unix Administrator | +81 (0)3 -5456-2687 ext 1017 (work) Shibuya-ku, Tokyo | +81 (0)90-5578-8500 (cell) Japan | +81 (0)3 -3375-1767 (home) ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-11-19 0:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-11-15 9:03 Crypto causes panic in scatterwalk_done with large/multiple buffers Jorgen Lundman 2012-11-17 0:42 ` Jussi Kivilinna 2012-11-17 8:39 ` Jussi Kivilinna 2012-11-18 0:29 ` Jorgen Lundman 2012-11-18 2:10 ` Jorgen Lundman 2012-11-18 17:40 ` Jussi Kivilinna 2012-11-19 0:29 ` Jorgen Lundman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.