| Message ID | 20141121171406.1147.31491.stgit@tstruk-mobl1 (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On Fri, Nov 21, 2014 at 09:14:06AM -0800, Tadeusz Struk wrote: > > diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c > index 85e3bdb..e393c71 100644 > --- a/crypto/algif_skcipher.c > +++ b/crypto/algif_skcipher.c > @@ -359,8 +359,6 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock, > err = 0; > > ctx->more = msg->msg_flags & MSG_MORE; > - if (!ctx->more && !list_empty(&ctx->tsgl)) > - sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list); > > unlock: > skcipher_data_wakeup(sk); > @@ -408,9 +406,6 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page, > > done: > ctx->more = flags & MSG_MORE; > - if (!ctx->more && !list_empty(&ctx->tsgl)) > - sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list); > - > unlock: > skcipher_data_wakeup(sk); > release_sock(sk); Please put these clean-ups in a separate patch. > @@ -469,6 +464,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock, > if (!used) > goto free; > > + sg_mark_end(&sg[sgl->cur - 1]); I don't think this will work as if we only partially use up the SGs and MSG_MORE is set then bad things will happen to the next send call on the socket. Cheers,
Hi Herbert, On 11/25/2014 06:42 AM, Herbert Xu wrote: > Please put these clean-ups in a separate patch. Ok, will do. > >> > @@ -469,6 +464,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock, >> > if (!used) >> > goto free; >> > >> > + sg_mark_end(&sg[sgl->cur - 1]); > I don't think this will work as if we only partially use up the > SGs and MSG_MORE is set then bad things will happen to the next > send call on the socket. Yes, I see now. I assumed that the user would want to read the same len that was first sent and thus the skcipher_pull_sgl() would clean the whole ctx->tsgl and then skcipher_alloc_sgl() would create a new one. I'll see if something else can be done to mark the end of data. Thanks, Tadeusz -- 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
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 85e3bdb..e393c71 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -359,8 +359,6 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock, err = 0; ctx->more = msg->msg_flags & MSG_MORE; - if (!ctx->more && !list_empty(&ctx->tsgl)) - sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list); unlock: skcipher_data_wakeup(sk); @@ -408,9 +406,6 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page, done: ctx->more = flags & MSG_MORE; - if (!ctx->more && !list_empty(&ctx->tsgl)) - sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list); - unlock: skcipher_data_wakeup(sk); release_sock(sk); @@ -469,6 +464,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock, if (!used) goto free; + sg_mark_end(&sg[sgl->cur - 1]); ablkcipher_request_set_crypt(&ctx->req, sg, ctx->rsgl.sg, used, ctx->iv);
Hi, algif_skcipher sends 127 sgl buffers for encryption regardless of how many buffers acctually have data to process, where the few first with valid len and the rest with zero len. This is not very eficient and may cause problems when algs do something like this without checking the buff lenght: for_each_sg(sgl, sg, sg_nents, i) sg_virt(sg) This patch marks the last one with data as the last one to process. Also removed some unneeded code. Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com> --- crypto/algif_skcipher.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) -- 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