Crypto digests and kmapping sg entries larger than a page, with[PATCH]

From: Clay Haapala
Date: Fri Jun 04 2004 - 13:56:27 EST


N.C.Krishna Murthy asked me to forward this to the crypto maintainers.

While doing work on the iSCSI driver and 2.6.6 to use the crypto
CRC32C routines for digests, he saw that the scatterlist entries had
entries larger than one page. We've had some discussion in
linux-iscsi about making sure we kmap() each page in such entries, and
Krishna saw that the crypto digest code may also need to do the same.

He supplies a patch, below.

If you agree with the implementation, I'll re-diff and test against a
recent BitKeeper extract and submit a patch.

--- Begin Message --- Hi Clay,
I was testing iSCSI driver code from 4.0 branch against 2.6.6 kernel.
I had the in kernel crypto CRC32 feature enabled. Initial testing resulted
in CRC errors. This was due to bugs in iSCSI driver and it has been reported
on sourceforge([965626 ] CRC errors seen when crypto APIs are used).

I fixed the bugs in iSCSI driver and ran some file I/O.
This resulted in an OOPs.
-----------------------------------------------------------------------------
---------- Process iscsi-tx (pid: 3344, threadinfo=f63e4000 task=f5c9f2a0)
Stack: f64d82b0 00000000 c0215af5 ffffffff fffe2000 00002000 fffe2000
c0214c32 f64d82b0 fffe2000 00002000 f64d82b0 f63e4000 f63e4000 00002000
f63e5e94 f7f609a0 f5c74000 f8b0dc80 f64d8284 f63e5e94 00000001 f63e5e78
f63e5e7c Call Trace:
[<c0215af5>] chksum_update+0x25/0x30
[<c0214c32>] update+0x92/0x100
[<f8b0dc80>] iscsi_xmit_data+0x460/0xb60 [iscsi_sfnet]
[<c0119ff3>] scheduler_tick+0x43/0x630
[<c036ea7f>] schedule+0x36f/0x6a0
[<f8b0e52a>] iscsi_xmit_r2t_data+0xca/0x1b0 [iscsi_sfnet]
[<f8af9025>] process_tx_requests+0x1f5/0x320 [iscsi_sfnet]
[<c011a5e0>] default_wake_function+0x0/0x20
[<f8af92e5>] iscsi_tx_thread+0x195/0x1d0 [iscsi_sfnet]
[<c011a5e0>] default_wake_function+0x0/0x20
[<f8af9150>] iscsi_tx_thread+0x0/0x1d0 [iscsi_sfnet]
[<c0103f95>] kernel_thread_helper+0x5/0x10
-----------------------------------------------------------------------------
-----------

Upon looking into the crypto/digest.c, I found that "update()" function
does crypto_kmap() only the first page in the sg. A "struct scatterlist "
can point to a single page or physically contiguous pages. The oops
seen is a result of "update()" not handling contiguous pages.
The patch I have attached solves the bug. I was able to test it and did
not see any oops after that. File I/O went through fine without any digest
errors. Could you please forward the patch to the appropriate forum?

Thanx
N.C.Krishna Murthy

-------------------------------------------------------

--- /usr/src/linux-2.6.6/crypto/digest.c 2004-06-03 14:36:09.000000000 +0530
+++ /usr/src/linux-2.6.6/crypto.new/digest.c 2004-06-03 15:12:10.683261696 +0530
@@ -27,13 +27,28 @@
struct scatterlist *sg, unsigned int nsg)
{
unsigned int i;
-
+
for (i = 0; i < nsg; i++) {
- char *p = crypto_kmap(sg[i].page, 0) + sg[i].offset;
- tfm->__crt_alg->cra_digest.dia_update(crypto_tfm_ctx(tfm),
- p, sg[i].length);
- crypto_kunmap(p, 0);
- crypto_yield(tfm);
+
+ struct page *pg = sg[i].page;
+ unsigned int offset = sg[i].offset;
+ unsigned int l = sg[i].length;
+
+ do {
+ unsigned int bytes_from_page = min(l, ((unsigned int)
+ (PAGE_SIZE)) -
+ offset);
+ char *p = crypto_kmap(pg, 0) + offset;
+
+ tfm->__crt_alg->cra_digest.dia_update
+ (crypto_tfm_ctx(tfm), p,
+ bytes_from_page);
+ crypto_kunmap(p, 0);
+ crypto_yield(tfm);
+ offset = 0;
+ pg++;
+ l -= bytes_from_page;
+ } while (l > 0);
}
}


--- End Message ---


--
Clay Haapala (chaapala@xxxxxxxxx) Cisco Systems SRBU +1 763-398-1056
6450 Wedgwood Rd, Suite 130 Maple Grove MN 55311 PGP: C89240AD
"We're serious about this campaign now! The training wheels are coming off!"
-- a high White Horse Souse